Share reading/validation code between elem exprs & other const exprs#2288
Merged
Conversation
3fca756 to
7d0fd5e
Compare
Merged
7d0fd5e to
d56e9e7
Compare
sbc100
approved these changes
Sep 6, 2023
Member
sbc100
left a comment
There was a problem hiding this comment.
Nice!
I've long thought this would be good idea but wan't sure how to make it happen. Thanks.
| } | ||
|
|
||
| if (expr.insts.empty()) { | ||
| PrintDetails("malformed\n"); |
Member
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?
Member
Author
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.
fdde555 to
a947ebc
Compare
Merged
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
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.