Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

Could the different try variants be combined? #131

Closed
tlively opened this issue Oct 2, 2020 · 4 comments · Fixed by #157
Closed

Could the different try variants be combined? #131

tlively opened this issue Oct 2, 2020 · 4 comments · Fixed by #157

Comments

@tlively
Copy link
Member

tlively commented Oct 2, 2020

Kudos to @ioannad for the great write up in #87!

One thing that stood out to me was the large number of try variants, and I wanted to ask about them in a new issue to avoid cluttering up that other thread. We currently have the following:

instr ::= ... | throw x | rethrow
        | try bt instr* (catch exnidx instr*)+ end
        | try bt instr* (catch exnidx instr*)* catch_all instr* end
        | try bt instr* catch_br l
        | try bt instr* unwind instr* end

It's clear that try-catch_br needs to be its own separate variant, but I wonder if it would be simpler to combine the other variants:

instr ::= ... | throw x | rethrow
        | try bt instr* (catch exnidx instr*)* (catch_all instr*)? (unwind instr*)? end
        | try bt instr* catch_br l

One quirk of this formulation is that it would allow for try bt intr* end, which is completely useless. I imagine it would just have the semantics of a normal block. The other obvious change is that this formulation would allow a try to have both catches and an unwind. I imagine the semantics of that would be similar to the semantics of this:

try
  try
    ...
  catch
    ...
  end
unwind
  ...
end

Specifically, if the catch body rethrows, the unwind body is still run afterwards. So this would just allow for saving a level of try nesting.

I don't feel strongly about this at all since it is just a surface syntax change, but I think it might be a nice simplification. I'm curious to hear what other people think and whether this change would complicate anything I've overlooked.

@ioannad
Copy link
Collaborator

ioannad commented Oct 5, 2020

Thank you so much @tlively!

I am in favour of your suggestion. I think it would simplify the formal writeup a lot, and, at least at first glance I can't see any complications it could create.

@aheejin
Copy link
Member

aheejin commented Oct 6, 2020

I'd like to keep try-catch and try-unwind separate. I don't have a very strong argument to back it, but I think it is generally clearer; when both of them are present, it may not be very obvious which has to be visited first. But yeah, it can be well defined and it may not be a problem.

The other reason is that our LLVM toolchain will not use it anyway. Current our LLVM toolchain uses a variant of Windows IR that uses catchpad and cleanuppad system, so they are separately generated. If we want to make use of this feature, we should post-scan the code for opportunities for merging, which I'm probably not gonna do, because code size saving will likely to be negligible. (Because we generate them separately, I don't think there will be many cases in which we can merge some of them in the first place.) But yeah, "I'm not gonna use it" is not a strong argument either. This is just my preferences.

@rossberg
Copy link
Member

The new explainer has unified catch and catch_all, but not unwind. That seems reasonable to me. We should still simplify the currently described grammar to

try bt instr* (catch exnidx instr*)* (catch_all instr*)? end

and avoid making a special-case restriction for the empty case.

One quirk of this formulation is that it would allow for try bt intr* end, which is completely useless. I imagine it would just have the semantics of a normal block.

That is actually fine, and consistent with all other constructs in Wasm: everywhere else, lists can be empty just fine.

The closest parallel probably is br_table, which can also have an empty table (up to the default label). When it has, it is equivalent to a plain br -- in the same way, an empty try is equivalent to a plain block. So nothing new or quirky about having corner cases like this.

@aheejin
Copy link
Member

aheejin commented Jun 4, 2021

Sorry for catching up on this late. I agree that the restriction that a try must have at least a single catch/catch_all/delegate is not necessary. Do other people have any other opinions or concerns on the effect of removing this restriction?

aheejin added a commit to aheejin/exception-handling that referenced this issue Jun 7, 2021
aheejin added a commit that referenced this issue Jun 11, 2021
Ms2ger pushed a commit to Ms2ger/exception-handling that referenced this issue Jun 24, 2021
* [interpreter] Simplify zero-len and drop semantics

* Update overview

* [spec] Change drop semantics

* [spec] Forgot to adjust prose for *.init ops

* [test] Update generated tests for OOBs and dropping changes (WebAssembly#131)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants