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.
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
formal spec overview for the 3rd exception handling proposal #143
formal spec overview for the 3rd exception handling proposal #143
Changes from 16 commits
9c0c1a8
2dc31f8
57c41e5
51c360b
5a988d5
aafc4b1
4bb9f84
1afd21a
4ff1c38
fbe87b0
fa5a237
83872ed
4552306
8cfa1b6
7d122dd
d874fcd
c98ff33
72bf4f3
4606660
662343c
b12eec9
9b02deb
bb0a623
5457b7a
6f681ff
315fe15
219d92e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what purpose the throw contexts serve. Is it something like block contexts? I might be mistaken but I thought block contexts are a way to concisely describe multiple levels of nested labels... But not sure if that applies to this too. Why do reduction rules need throw contexts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC block contexts do not automatically contain other nested constructs (these are the delimited administrative
catch...end
,delegate...end
, andcaught...end
). I think we have to explicitly add these to the definition of block contexts (lines 120-122 above).On the other hand, throw contexts exclude the nested constructs
catch_m { a^? instr* }* [_] end
anddelegate{l} [_] end
, so any throw would have to stop when we encounter the first (innest)catch_m {...}* [_] end
ordelegate{l} [_] end
.The prose we used in the 2nd proposal might be clearer:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a short explanation above the definition of throw contexts. Does that and my comment above clarify their purpose, @aheejin ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means rethrowing an exception is semantically the same as throwing the same values, which is true for the core spec.. It may carry auxiliary info like stack traces for JS API spec, in which case they are not identical, but I guess it is fine because this is only for core spec..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this doesn't affect the JS API, WDYT @Ms2ger ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be fine at the moment because this is the core spec... By the way why is it still
B^l
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean that both
B^l
should beT
? The intention is that thel
inB^l
is matching the one inrethrow l
, otherwise how will we know whichcaught
to target?`Did I miss a spec change on this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we consider this resolved, or am I missing the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
l+1
? It would help if we have a brief description on what the administrative instructiondelegate{}
is (not the Wasm instructiondelegate
). The same for the other administrative instructions tooThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
l
becomesl+1
because we are adding a label around it, so we can break to it from insideinstr*
.The administrative instructions
catch_n ... end
,caught_n ... end
anddelegate ... end
are label-like, in that they (plus a surrounding label), delimit an instruction sequence, while also adding information on the special behaviour of these label/label-like combinations.Off the top of my head perhaps it's possible to merge each of these with its label, and instead define new labels. So in particular redefine labels to be:
WDYT?
(cc. @rossberg)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is another indication that the hack I suggested for typing catch labels – which is to put the block label outside the catch – is not the greatest idea (see also this comment).
I'd much prefer to keep the label statement separate. If you treated all of these as labels, all of them would need to carry a continuation. And likewise future block-constructs we might add. That conflates concerns, increasing complexity.
Unfortunately, there is some inherent conflation due to
rethrow
addressing its sourcecatch
via a label. (If we hadn't done that, i.e., only count catch clauses, we wouldn't have this problem.) That's why we need the ad-hoc annotation on the label in the context, and that's where the reduction hack comes from.I'll try to come up with some replacement for this hack – dealing with a typing corner case shouldn't bleed into reduction rules in counter-intuitive ways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aheejin, @rossberg, thinking about it, is the +1 really necessary? If we look at
try-delegate
as:exn
,l
th surrounding block,exn
from there,then I think we don't need the
+1
when reducing to the administrativedelegate
. In fact I think the reduction of the administrativedelegate
is wrong to throw after (outside of) thel+1
'th surrounding block. We should be breaking after thel
th surrounding block (including the label added bytry-delegate
's reduction), and then doing a throw from there. Only then can we be redirected to a try-block's landing pad, if the destination ofl
is a try-block. WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, you may be right that there is an off-by-1 bug that could work in our favour. Should be easy to check by working through an example. And of course with the interpreter, provided it matches the rules faithfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the long delay!
Please never mind my previous explanation. I looked at this again and realised that the reduction rule of the administrative
delegate{ l }
should resemble the reduction ofbr l
:This way, there is a unique
B^l
that matches the reduction rule.I modified Example 4 and added Example 5 in
Exceptions-formal-examples.md
which show the reduction steps of the administrativedelegate
s, to check that this works, and modified (or rather simplified) the interpreter accordingly, adding a couple of extra tests there.I think that now it matches the idea of break-and-throw better. Note that the interpreter part that was removed didn't previously have a corresponding reduction rule in this formal-overview.