Skip to content

Provide backward compatible support for assert_return_arithmetic_nan #1371

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

Closed
binji opened this issue Mar 23, 2020 · 6 comments
Closed

Provide backward compatible support for assert_return_arithmetic_nan #1371

binji opened this issue Mar 23, 2020 · 6 comments

Comments

@binji
Copy link
Member

binji commented Mar 23, 2020

See #1275 where these were removed. The upstream spec was changed to use a more generic syntax:

(assert_return_canonical_nan (invoke "..."))

becomes

(assert_return (invoke "...") (nan:canonical))

(And similarly for assert_return_arithmetic_nan and nan:arithmetic).

But in general, wabt tries to be backward compatible, so it could continue to support this old syntax too.

See WebAssembly/bulk-memory-operations#144.

@sbc100
Copy link
Member

sbc100 commented Mar 23, 2020

I don't think we should do this. I think wabt should support all the testsuite features that are present in the amalgamated testsuite repo (https://github.com/WebAssembly/testsuite) and no others.

If we do decide we need this behaviour then I think the way to achieve it would be to first include all tests from all proposals in the amalgamated repo rather then limiting the mirroring to only tests with upstream changes. i.e. we should let the testsuite repo drive out testing/coverage.

@binji
Copy link
Member Author

binji commented Mar 23, 2020

Fair point. My main concern is that there are existing .wast files that we may want to continue to support, especially those that have been copied from the spec testsuite at an earlier date, which seems to be common. I don't know that we want to say everything should work, but it would be nice to avoid breaking code if it's not too much work (which I think it wouldn't be, in this case).

@rianhunter
Copy link
Contributor

rianhunter commented Mar 23, 2020

I don't have a strong opinion on this but the reason I suggested keeping backwards compatible behavior was that my spectests suddenly stopped working when i upgraded the version of wabt that comes with my distro, and it was an annoying experience.

@chfast
Copy link
Contributor

chfast commented Aug 7, 2020

Can the wasm 1.0 tests from WebAssembly/spec, branch wg_v1 be updated to the need syntax? It looks there are only 4 cases of the old syntax:

find ../wasm-spec/test/core -name '*.wast' -exec wast2json {} \;                                                                                                                   ✔  14:13:49  
../wasm-spec/test/core/float_misc.wast:572:1: error: unexpected token (, expected EOF.
(assert_return_canonical_nan (invoke "f64.sqrt" (f64.const -0x1.360e8d0032adp...
^
../wasm-spec/test/core/f64.wast:51:1: error: unexpected token (, expected EOF.
(assert_return_canonical_nan (invoke "add" (f64.const -0x0p+0) (f64.const -na...
^
../wasm-spec/test/core/f32.wast:51:1: error: unexpected token (, expected EOF.
(assert_return_canonical_nan (invoke "add" (f32.const -0x0p+0) (f32.const -na...
^
../wasm-spec/test/core/float_exprs.wast:49:1: error: unexpected token (, expected EOF.
(assert_return_arithmetic_nan (invoke "f32.no_fold_add_zero" (f32.const nan:0...
^
../wasm-spec/test/core/conversions.wast:357:1: error: unexpected token (, expected EOF.
(assert_return_canonical_nan (invoke "f64.promote_f32" (f32.const nan)))
^

@rossberg
Copy link
Member

Upgrading the branch seems plausible, but we'd need to make sure that there are no other clients of the 1.0 test suite that we'd break with that.

@keithw
Copy link
Member

keithw commented Mar 15, 2023

Closing as stale -- it doesn't appear that a lot of people need WABT to support this syntax (even if it was in the spec testsuite at the time of 1.0?).

@keithw keithw closed this as completed Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants