This repository was archived by the owner on Jan 15, 2025. It is now read-only.
Update embedding and execution specs for memory64#74
Merged
sbc100 merged 3 commits intoWebAssembly:mainfrom Sep 23, 2024
Merged
Update embedding and execution specs for memory64#74sbc100 merged 3 commits intoWebAssembly:mainfrom
sbc100 merged 3 commits intoWebAssembly:mainfrom
Conversation
rossberg
reviewed
Aug 2, 2024
Vectors in the spec are syntactically limited to a maximum size of 2^32. This is incompatible with memory64, so the storage has been changed to use sequences instead of vectors. The execution semantics are unaffected.
Memory and table operations in the embedding section have been updated to use u64 instead of u32, reflecting the changes to `limits`. Various operations in the execution section were also updated to reflect `idxtype`'s inclusion in `memtype` and `tabletype`, as well as the supporting validation rules.
42b7239 to
4e06b9a
Compare
bvisness
commented
Aug 5, 2024
rossberg
reviewed
Aug 8, 2024
Member
rossberg
left a comment
There was a problem hiding this comment.
LGTM except for the -1 you removed, see comment.
Btw, please never force-push to a branch under review. That breaks the GH review workflow, such as relative diffs.
Collaborator
Author
Member
|
Sorry, I missed that you were waiting on me here. I added you has a collaborator so you can land these kind of thing yourself going forward. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Memory and table operations in the embedding section have been updated to use u64 instead of u32, reflecting the changes to
limits. Various operations in the execution section were also updated to reflectidxtype's inclusion inmemtypeandtabletype.Additionally, I have switched the storage in memory and table instances to sequences instead of vectors. This is because vectors are restricted to a maximum length of 2^32, which is incompatible with i64 storage.
Finally, I have decided not to try and address #67 in this PR or the one to follow. I'll submit a separate PR to rename "index type" to "offset type" across the board after all other spec changes are done, and then we can decide if we actually want to do it or not.
See also #75, which updates the JS API.