Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to append to abi.DynamicArray #604

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jasonpaulos
Copy link
Contributor

This is a draft PR that partially implements support for appending to dynamic arrays.

This has not been tested and isn't fully functional.

@barnjamin
Copy link
Contributor

Would it make sense to have some other types that were better for this type of thing that could be initialized from/encoded into an ABI type?

ex:

StringArray:
  positions: uint64[] (itob'd/concat'd list of string positions, stored in a scratch var)
  elements: string[] (concat'd list of elements with just the byte contents stored in scratch)

str_arr[3] => Extract(ExtractUint64(positions.load(), 8*3), ExtractUint64(positions.load(), 8*(3+1)))

or maybe storing lengths would be better in case some string is replaced, but would need to sum along the way to find the right position.

This would alleviate the need to update each element in the header describing the offset any time you append to the array but I've not tried to write it to see how expensive it is in opcode terms

@FrankSzendzielarz
Copy link

There could be a couple of representations of complex types depending on if the element is declared as readonly. Alternatively we could have a representation that works for both scenarios (eg a box that treats all complex type elements as references) and decode/encode only on entry/exit to/from ABI methods. Arbitrary representations might complicate debugger experiences.

In general I feel that the AVM would probably benefit from supporting types like nested tuples and arrays directly, using some kind of element access syntax, and having the AVM take care of the encoding/decoding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants