Skip to content

Fixing print-profile.wast test #4939

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

Merged
merged 1 commit into from
Aug 19, 2022
Merged

Fixing print-profile.wast test #4939

merged 1 commit into from
Aug 19, 2022

Conversation

ashleynh
Copy link
Collaborator

In a multi-memories world, memory must be explicitly defined before being exported. Fix CI for 6b3f3af.

/cc @sps-gold

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

I’m surprised this works without giving the memory a name. A name (or at least index) on the export statement would say which memory is being exported. Maybe one of the 0s in the export declaration is being interpreted as an index? In that case, the other 0 seems like it shouldn’t be there.

But anyway lgtm to fix CI. Parser issues will eventually be resolved by switching to the new parser.

@kripken
Copy link
Member

kripken commented Aug 19, 2022

@tlively - good points, it seems like something is wrong in the s-parser logic there... the export declaration should only take a name or an index, and error on having an extra param.

@kripken
Copy link
Member

kripken commented Aug 19, 2022

Landing this to unbreak CI and PRs.

@kripken kripken merged commit 5b64cb7 into main Aug 19, 2022
@kripken kripken deleted the test_fix branch August 19, 2022 16:25
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.

3 participants