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

Update rethrow depth handling and catch_all opcode #1608

Merged
merged 2 commits into from
Feb 18, 2021

Conversation

takikawa
Copy link
Contributor

This PR updates exception handling support to match the most recent spec explainer (see WebAssembly/exception-handling#137). In particular, it fixes the handling of rethrow's depth argument and changes the catch_all opcode. Tests are also expanded (with test cases taken from spec issue examples).

Previously `catch_all` shared an opcode with `else`, but
the spec now allocates it the 0x19 opcode.
Previously this had interpreted the rethrow depth argument
as counting only catch blocks, but the spec has clarified that
it should count all blocks (in a similar fashion as `br` and
related instructions).
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

This looks good to me.
@sbc100 can you take a quick look too since you have more familiarity with wabt?

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

lgtm!

@takikawa
Copy link
Contributor Author

Thanks for review! :) I don't have merge permissions so would appreciate someone who does merging this in.

@sbc100 sbc100 merged commit ffb22e3 into WebAssembly:master Feb 18, 2021
@aheejin
Copy link
Member

aheejin commented Feb 18, 2021

@takikawa It seems you does have the merge access though? You are shown as a "member". Last time I merged your PR because you were shown as a "contributor". I guess you joined the WebAssembly org in the meantime?

@takikawa takikawa deleted the exception-fixes branch February 18, 2021 17:33
@takikawa
Copy link
Contributor Author

Thanks for the merge!

@aheejin I did indeed join the org as a member, maybe I also need to request to join a team in the org? I didn't see the merge button in my UI as far as I could tell.

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.

4 participants