Fix return representation in Ast
#256
Conversation
|
Ah, nice, I hadn't even noticed that |
Adjust `if` to match post-order format.
|
@lukewagner, is that an LGTM? :) Btw, I screwed up my branching, so that #257 is included in this branch. Sorry about that, just look at the first commit. |
|
oops, yes :) |
Fix `return` representation in Ast
|
Thanks :) |
| @@ -95,7 +95,7 @@ | |||
| (loop | |||
| (if | |||
| (i32.eq (get_local 0) (i32.const 0)) | |||
| (br 1) | |||
| (br 2) | |||
There was a problem hiding this comment.
This pull is for return - why did these brs change?
And where does 2 go? the only blocks or loops above this br are the one loop, which would take indexes 0 and 1?
There was a problem hiding this comment.
if has an implicit label now too. Change was here: 0568800. Not sure why it didn't break then.
edit: oh, it got added in to this merge accidentally.
There was a problem hiding this comment.
Sorry for this, I messed up the branching, and accidentally merged the if changes into the return branch instead of master. This line is part of the if changes (if now has an implicit block).
There was a problem hiding this comment.
is it the if that has an implicit block, or the then and else branches that do? because I see they can have a label name on them now? must it be the same for both of them if there is?
There was a problem hiding this comment.
The branches have implicit blocks, so the labels go there (also, that makes it clear that they are not in scope in the condition). They can be named differently, see the examples in test/labels.wast.
This makes some cosmetic changes to the identity-preserving `rethrow` test added in WebAssembly#253. This is to prepare for adding the same set of tests for a Wasm-defined-and-exported tag, as requrested in WebAssembly/exception-handling#253 (comment).
Ast.Returnimproperly had a label argument to support its desugaring. That is now removed, and the logic properly encapsulated in desugaring.