This repository was archived by the owner on Apr 25, 2025. It is now read-only.
[test] Add tests for return_call(_indirect) in try #275
Conversation
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
This adds tests of `return_call(_indirect)`s within `try`s. Because this repo's interpreter doesn't have the tail call support, this requires WebAssembly#274 to be merged before merging. But currently even with WebAssembly#275 the tests here don't run. When a `return_call(_indirect)` run within a `try`, it errors out with ``` runtime crash: undefined frame ``` I think we need to fix the interpreter first before merging this. Closes WebAssembly#249.
aheejin
force-pushed
the
return_call_tests
branch
from
0e7ccc5 to
2c722f7
Compare
thibaudmichaud
approved these changes
Apr 21, 2023
|
I would also suggest adding a test that return_calls an import, and maybe one that return_calls a host function in the JS tests. |
aheejin
added a commit
that referenced
this pull request
Apr 23, 2023
This cherry-picks interpreter changes from https://github.com/WebAssembly/tail-call to run spec tests that mix `try`s with `return_call(_indirect)`s (#275). When tail-call is merged to the main spec repo, we can revert this change and merge the upstream spec. This also adds missing handling for `ReturningInvoke` in the new `Catch`/`Caught`/`Delegate` instructions in the evaluator.
aheejin
commented
Apr 23, 2023
| @@ -31,4 +31,4 @@ jobs: | |||
| - name: Build interpreter | |||
| run: cd interpreter && opam exec make | |||
| - name: Run tests | |||
| run: cd interpreter && opam exec make JS=node ci | |||
| run: cd interpreter && opam exec make JS='node --experimental-wasm-return_call' ci | |||
There was a problem hiding this comment.
This was necessary to make the interpreter tests pass, because we not only run them with ocaml but also with JS. I think we can revert this later once the tail call is merged to the main spec and is enabled without a flag.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 adds tests of
return_call(_indirect)s withintrys. Because this repo's interpreter doesn't have the tail call support, this requires #274 to be merged before merging. But currently even with #274 thetests here don't run. When a
return_call(_indirect)runs within atry, it errors out withI think we need to fix the interpreter first before merging this.
Closes #249.