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

Share validation code between constant expressions and function bodie… #1783

Merged
merged 1 commit into from
Dec 10, 2021

Conversation

sbc100
Copy link
Member

@sbc100 sbc100 commented Dec 10, 2021

…s. NFC

Previously we has special cases for initializer expressions (constant
expressions). This change paves the way for adding support for
extended constant expressions that support a wider range of
instructions.

This change removes twice as many lines as it adds which shows that
this simplification is probably worthwhile even without the pending
extensions.

@sbc100 sbc100 force-pushed the validate_init_expr branch 2 times, most recently from 8ddf70c to 73c8d3f Compare December 10, 2021 01:26
Copy link
Member

@ngzhian ngzhian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failure messages are less clear i think (previously it mentions global initializer expression, now it's constant expression), but with the line and column it should be easy to pinpoint the errors.

@sbc100
Copy link
Member Author

sbc100 commented Dec 10, 2021

The failure messages are less clear i think (previously it mentions global initializer expression, now it's constant expression), but with the line and column it should be easy to pinpoint the errors.

I agree, it could be seen a less clear, but its also more consistent which I like. I can take a look and see if we can be more specific perhaps.

…s. NFC

Previously we has special cases for initializer expressions (constant
expressions).  This change paves the way for adding support for
extended constant expressions that support a wider range of
instructions.

This change removes twice as many lines as it adds which shows that
this simplification is probably worthwhile even without the pending
extensions.
@sbc100 sbc100 enabled auto-merge (squash) December 10, 2021 16:34
@sbc100 sbc100 merged commit 1d64588 into main Dec 10, 2021
@sbc100 sbc100 deleted the validate_init_expr branch December 10, 2021 16:47
keithw added a commit that referenced this pull request Aug 30, 2023
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)
keithw added a commit that referenced this pull request Aug 30, 2023
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)
keithw added a commit that referenced this pull request Aug 30, 2023
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)
keithw added a commit that referenced this pull request Aug 31, 2023
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)
keithw added a commit that referenced this pull request Sep 6, 2023
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)
keithw added a commit that referenced this pull request Sep 6, 2023
…2288)

This continues the work from #1783 and reduces special handling of elem
exprs, by treating them the same as other const expressions (init
expressions).
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.

2 participants