Share reading/validation code between elem exprs & other const exprs #2288
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
keithw
force-pushed
the
unify-elem-exprs
branch
2 times, most recently
from
3fca756 to
7d0fd5e
Compare
Merged
keithw
force-pushed
the
unify-elem-exprs
branch
from
7d0fd5e to
d56e9e7
Compare
sbc100
approved these changes
Sep 6, 2023
src/binary-reader-objdump.cc
Outdated
| } | ||
|
|
||
| if (expr.insts.empty()) { | ||
| PrintDetails("malformed\n"); |
There was a problem hiding this comment.
Do we do this kind of things elsewhere in this file? How about <MALFORMED> to be extra clear?
There was a problem hiding this comment.
Sounds good. I went with <EMPTY> to be safe -- on further review I don't think an empty const expression is malformed, just invalid.
keithw
force-pushed
the
unify-elem-exprs
branch
from
fdde555 to
a947ebc
Compare
Merged
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.
This continues the work from #1783 and reduces special handling of elem exprs, by treating them the same as other const expressions (init expressions).
Fixes #2201 (now allows global.get in an elem expr)
This is a prerequisite for updating the testsuite (spun out from #2287). It seemed cleaner to do it this way than to add a
OnElemSegmentElemExpr_GlobalGetto all the readers and validators, given that the validation rules are the same as when global.get appears in any other const expression.