Update load/store to match design repo changes #86
Conversation
This file contains 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
|
LGTM, I like it. (The one odd case that is not prevented in the AST by construction is Does it still make sense to call it |
I know and it bugged me too :) I was thinking we could have some new variant that enumerated the valid
|
|
Something could probably be done with polymorphic variants and GADTs, but I
don't think it's worth going there.
|
lukewagner
added a commit
that referenced
this pull request
Sep 25, 2015
Update load/store to match design repo changes
eqrion
pushed a commit
to eqrion/wasm-spec
that referenced
this pull request
Jul 18, 2019
…embly#86) * Fix expected trap messages. The spec interpreter says "element segment dropped", rather than "elements segment dropped". * Fix "zero len, but dst offset out of bounds" test. Fix this test to test what it's comment says it's testing. * Add more tests for zero-length operations. * Update the Overview text to reflect the zero-length at-the-end semantics.
eqrion
pushed a commit
to eqrion/wasm-spec
that referenced
this pull request
Jul 18, 2019
… (WebAssembly#87) Change the test generators to use `ref.func` and remove `passive`. At some point we'll want to remove the generators, but for let's try to maintain them.
rossberg
pushed a commit
that referenced
this pull request
Feb 11, 2021
dhil
pushed a commit
to dhil/webassembly-spec
that referenced
this pull request
Oct 21, 2024
* Revert "Allow cont ref to be non-null in switch (WebAssembly#85)" This reverts commit 92cbcc5. * Remove other unnecessary `null?` in instruction typing
rossberg
pushed a commit
that referenced
this pull request
Nov 6, 2024
* Rework IndexValueToU64 to allow more inputs to BigInts * Address #86 feedback
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Updating the spec repo to re-include ops re-added by design/#366.
In the process, I applied the idea we discussed in the other issue of splitting
LoadintoLoad/LoadExtendandStoreintoStore/StoreTrunc. Given that the extend/trunc are really doing semantically different things and take additional immediate arguments, this feels like the right thing to do and makes theLoad/Storecases really short and sweet.Patch also contains minor fix of source info in
Constfor better error reporting.