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.

Sign up for GitHub

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

Jump to bottom

wast: does not support recently added atypical condition test #1213

Closed
ia0 opened this issue Sep 23, 2023 · 1 comment · Fixed by #1215
Closed

wast: does not support recently added atypical condition test #1213

ia0 opened this issue Sep 23, 2023 · 1 comment · Fixed by #1215

Comments

@ia0
Copy link

ia0 commented Sep 23, 2023

Parsing the if.wast file of the WebAssembly/spec test suite fails after WebAssembly/spec#1682.

https://github.com/WebAssembly/spec/blob/bfb21e96f99820e0cfb0c41a264bd0d42795bfe2/test/core/if.wast#L530-L534

  (func (export "atypical-condition")
    i32.const 0
    (if (then) (else))
    (if (i32.const 1) (i32.eqz) (then) (else))  ; it fails on the "then"
  )
@ia0 ia0 mentioned this issue Sep 23, 2023
Merged
ia0 added a commit to google/wasefire that referenced this issue Sep 23, 2023
@ia0
Upgrade all dependencies (#227)
18f2e85 
Kept the following a bit behind:
- `third_party/WebAssembly/spec` because of
bytecodealliance/wasm-tools#1213
- `generic-array` because RustCrypto don't use the 1.0.0 yet
alexcrichton added a commit to alexcrichton/wasm-tools that referenced this issue Sep 25, 2023
@alexcrichton
Clean up parsing (if ...)
29109b7 
Much of this code dates back to when this crate was trying to parse all
of WABT's tests as well as the upstream test suite. WABT at the time had
syntactical forms that weren't technically supposed to be valid given
the upstream test suite, so at the time more was accepted than should be.

This commit fixes how `(if ...)` is parsed to require `(then ..)` and
`(else ..)` wrappers. Previously these were mistakenly only optional.
Additionally this fixes bytecodealliance#1213 by allowing multiple condition-related
folded expressions before `(then ..)`.

This updates a few of the golden outputs for the spec test suites,
notably those that use `(if (foo) (then) (else))` where previously
`wast` would erroneously not emit an `else` instruction but now it does.

Closes bytecodealliance#1213
@alexcrichton alexcrichton mentioned this issue Sep 25, 2023
Merged
@alexcrichton
Copy link
Member

Thanks for the report! This should be fixed in #1215

alexcrichton added a commit that referenced this issue Sep 25, 2023
@alexcrichton
Clean up parsing (if ...) (#1215)
585a0bd 
* Clean up parsing `(if ...)`

Much of this code dates back to when this crate was trying to parse all
of WABT's tests as well as the upstream test suite. WABT at the time had
syntactical forms that weren't technically supposed to be valid given
the upstream test suite, so at the time more was accepted than should be.

This commit fixes how `(if ...)` is parsed to require `(then ..)` and
`(else ..)` wrappers. Previously these were mistakenly only optional.
Additionally this fixes #1213 by allowing multiple condition-related
folded expressions before `(then ..)`.

This updates a few of the golden outputs for the spec test suites,
notably those that use `(if (foo) (then) (else))` where previously
`wast` would erroneously not emit an `else` instruction but now it does.

Closes #1213

* Fix some test syntax
@alexcrichton alexcrichton mentioned this issue Sep 28, 2023
Closed
kofls pushed a commit to kofls/wasefire that referenced this issue Nov 27, 2023
@ia0
Upgrade all dependencies (google#227)
9b424e5 
Kept the following a bit behind:
- `third_party/WebAssembly/spec` because of
bytecodealliance/wasm-tools#1213
- `generic-array` because RustCrypto don't use the 1.0.0 yet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Clean up parsing (if ...) alexcrichton/wasm-tools
2 participants
@alexcrichton @ia0