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

Behavior for try-delegate targeting catch-less try? #164

Closed
takikawa opened this issue Jun 24, 2021 · 9 comments · Fixed by #168
Closed

Behavior for try-delegate targeting catch-less try? #164

takikawa opened this issue Jun 24, 2021 · 9 comments · Fixed by #168

Comments

@takikawa
Copy link
Collaborator

takikawa commented Jun 24, 2021

Something I overlooked when reviewing #157 was what the behavior of try-delegate should be when it targets a catchless try block. For example:

try $l
  try
  delegate $l
end

I think I didn't see discussion of this particular case anywhere, sorry if I missed it. A semantics that might make sense is that this will just delegate to the next enclosing catch block (effectively rethrows from try $l). Any thoughts on how this should be specified?

@ioannad
Copy link
Collaborator

ioannad commented Jun 24, 2021

The semantics you propose are in line with the formalism in #143, so with that interpretation it should be fine for try-delegate to target a catchless try, as that would be to take the exception coming from try-delegate and rethrow it from the catchless try block. This also matches the idea that exceptions not caught by a try block's handlers are rethrown.

I suppose it will be good to describe a catchless try as one that rethrows any exception occurring in its try block instead of it being "effectively a regular block". As a "regular block" it can't be the target of try-delegate but that seems complicated, because in a one-pass validation run we don't know how to distinguish between a catchless and catchfull try until we read catch , catch_all, or end.

@rossberg
Copy link
Member

What @ioannad said. This is really just a case of delegating to a try that does not handle the current exception.

Btw, is it allowed to delegate to another try-delegate?

@ioannad
Copy link
Collaborator

ioannad commented Jun 24, 2021

@rossberg, yes, delegate can target any try block (so any try label).

Btw I noticed the proposal overview says

The delegate instruction can only delegate to catch/catch_all blocks in a try or to another delegate below the delegate itself.

Given that catchless trys are valid, perhaps it would be simpler conceptually if we talk about the try block being the destination that the try-delegate instruction delegates to, and not its catches (which might not exist). So the quote above could be instead something like the following.

The delegate instruction can only delegate an exception to be thrown from a surrounding try block.

What do you think, @aheejin?


Btw, if we do look at it like that, what is the reason for the restriction on delegate's label? (Disclaimer: I don't mind having the restriction, just wondering.)

@aheejin
Copy link
Member

aheejin commented Jun 25, 2021

For catchless try, I agree with @ioannad and @rossberg's interpretation. A catchless try can be considered in the same way as a try with catches that cannot catch the current exception. Also agree w/ @ioannad that describing catchless try as a block is confusing that a block cannot be targeted by a rethrow or a delegate. Actually I noticed we don't have any examples for catchless try in our explainer. I'll add a few.


And yes, as @ioannad said, delegate can target any try. But its destination can be different depending on whether the delegate is within try-catch part or catch-end part. (See #146 for examples). The same for rethrow, which also can target any try but its meaning can be different depending on whether it is within try-catch or catch-end. I'll revise the text to be less confusing. Also the sentence does not account for the fact that delegate can target catchless trys, so it is technically incorrect now.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 26, 2021
See WebAssembly/exception-handling#164
for a discussion of the intended behavior of this case.

Simplifies try note handling by using the existing mechanism and
just installing a no-handler rethrow pad.

Differential Revision: https://phabricator.services.mozilla.com/D118789
@aheejin
Copy link
Member

aheejin commented Jun 27, 2021

Sorry I think what I said above was confusing. The destination doesn't change depending on whether delegate is within try-catch part or catch-end part, but it can either a validation failure in one and not other. #146 is still what I was trying to summarize, but I think I did it in a confusing way.

Examples in these comments reflect my understanding correctly:
#146 (comment)
#146 (comment)

@aheejin
Copy link
Member

aheejin commented Jun 27, 2021

Sorry for flip-flopping, but I started to think that it might be more consistent to make try-delegate targetting catchless try a validation failure.

As I described in #146 (comment), the current rule is as follows:

try $label0
  try
  delegate $label0  ;; delegates to catch ($label0)
catch ($label0)
  try
  delegate $label0  ;; validation failure because we cannot delegate upwards
end

Note that we consider the second try-delegate a validation failure, not delegating to the enclosing try. I think the case of catchless try is similar to this second case. The reason we made it a validation failure is, we wanted to make sure the same label (or an immediate) targets the same catch or delegate and didn't want to make their meanings different, and we wanted to make it consistent with branch rules. Now we allow slipping to the enclosing try for catchless trys, I think it is inconsistent to the current rules.

Also, rethrow targetting catchless try should be a validation failure, by definition. rethrow's label is not a destination but it is the exception caught by a catch associated with the try, so rethrow targetting catchless try or try-delegate should be a validation failure.

@rossberg
Copy link
Member

As I described in #146 (comment), the current rule is as follows:

try $label0
  try
  delegate $label0  ;; delegates to catch ($label0)
catch ($label0)
  try
  delegate $label0  ;; validation failure because we cannot delegate upwards
end

Note that we consider the second try-delegate a validation failure, not delegating to the enclosing try. I think the case of catchless try is similar to this second case.

But this is unrelated. In a try with no catches, the second case cannot even occur, by construction, since this is a case that can only arise within a catch. So there is nothing to rule out.

Similarly, there cannot be, by construction, a rethrow targetting a "catchless try", because that would have to occur within a catch clause hat doesn't exist in the first place. So again, there is nothing new to be ruled out.

@thibaudmichaud
Copy link
Collaborator

I agree with @rossberg .

Similarly, there cannot be, by construction, a rethrow targetting a "catchless try", because that would have to occur within a catch clause hat doesn't exist in the first place. So again, there is nothing new to be ruled out.

I believe @aheejin is talking about this case:

try
  rethrow 0
end

which everyone seems to agree is a validation failure. This is not a special case and follows from the try-catch rules.

I also agree that delegating to a try-end block should be allowed, and is just a particular case of delegating to a try-catch-end block that doesn't handle the exception.

@aheejin
Copy link
Member

aheejin commented Jun 28, 2021

OK, I'll update the explainer reflecting those lines.

aheejin added a commit to aheejin/exception-handling that referenced this issue Jun 28, 2021
- Removes the sentence that catchless `try` is the same as a regular
`block`, because it can be targetted by `delegate`.
- Improves a few sentences
- Adds examples for catchless try

Closes WebAssembly#164.
aheejin added a commit to aheejin/exception-handling that referenced this issue Jun 28, 2021
- Removes the sentence that catchless `try` is the same as a regular
`block`, because it can be targetted by `delegate`.
- Improves a few sentences
- Adds examples for catchless try

Closes WebAssembly#164.
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 16, 2021
See WebAssembly/exception-handling#164
for a discussion of the intended behavior of this case.

Simplifies try note handling by using the existing mechanism and
just installing a no-handler rethrow pad.

Differential Revision: https://phabricator.services.mozilla.com/D118789
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 a pull request may close this issue.

5 participants