-
-
Notifications
You must be signed in to change notification settings - Fork 674
break: Add support for labeled statements, breaks, and continues #2891
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
Conversation
60e57da to
3a29149
Compare
I just need to add some tests; then, this will be ready to merge.
@JairusSW Heads up: this change definitely breaks transforms. Do you have anything to comment on this PR?
It looks like I forgot about if...
Okay, it appears that my code breaks something related to Flow flags, so more work on this PR is needed.
One issue is that breaking out of a labeled block sets the Breaks flag on the outer flow and messes with the function not appearing to always terminate.
Another issue is that a continue (and likely break as well) from inside an inner loop to an outer loop won't set the Continues/ConditionallyContinues flow flag on the outer loop body's flow. This is a much more significant issue (since nested loops are the most common use of labels) and seems to be more difficult to fix...
This is a prerequisite for supporting labeled breaks/continues. Clearly unusable labels, such as `x: let foo = 1;` report an error by default, similar to TS's behavior.
This requires an additional field to Flow that maps user-defined statement labels to the internal Binaryen labels passed to module.br(). Thanks to the existing logic to handle unlabeled break/continue, adding support for labeled break/continue is a breeze. Fixes AssemblyScript#2889.
3a29149 to
692b388
Compare
In TS, there are new ASTNode LabeledStatement to identifier this labeled statement, could we use the same concept?
@HerrCai0907 I considered that before, and it might be a pretty good idea. It would be better for transform users for sure.
Still, there's the issue of setting the proper FlowFlags for a continue to an outer loop...maybe the class I use to keep track of labels should contain the flow, and I can use something like do flow.set(...) while ((flow = flow.parent) != targetFlow). (Of course, the actual code would look nicer than that.)
Uh oh!
There was an error while loading. Please reload this page.
Fixes #2889.
Changes proposed in this pull request:
⯈ Support labeled statements in the parser/AST
⯈ Support labeled
break/continue.This change is breaking solely because it modifies the
Node.createXXXAPIs, which are used by transforms.