Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
CountBleck wants to merge3 commits intoAssemblyScript:main
base:main
Choose a base branch
Loading
fromCountBleck:labeled-statements

Conversation

CountBleck
Copy link
Member

@CountBleckCountBleck commentedDec 16, 2024
edited
Loading

Fixes#2889.

Changes proposed in this pull request:
⯈ Support labeled statements in the parser/AST
⯈ Support labeledbreak/continue.

This change is breaking solely because it modifies theNode.createXXX APIs, which are used by transforms.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

SebastienGllmt reacted with thumbs up emoji
@CountBleckCountBleck changed the titleAdd support for labeled statements, breaks, and continuesBREAKING CHANGE: Add support for labeled statements, breaks, and continuesDec 16, 2024
@CountBleckCountBleck changed the titleBREAKING CHANGE: Add support for labeled statements, breaks, and continuesbreak: Add support for labeled statements, breaks, and continuesDec 16, 2024
@CountBleckCountBleckforce-pushed thelabeled-statements branch 3 times, most recently from60e57da to3a29149CompareDecember 16, 2024 23:05
@CountBleckCountBleck marked this pull request as ready for reviewDecember 16, 2024 23:05
@CountBleck
Copy link
MemberAuthor

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?

@CountBleckCountBleck marked this pull request as draftDecember 17, 2024 00:40
@CountBleck
Copy link
MemberAuthor

It looks like I forgot aboutif...

@CountBleck
Copy link
MemberAuthor

CountBleck commentedDec 17, 2024
edited
Loading

Okay, it appears that my code breaks something related to Flow flags, so more work on this PR is needed.

One issue is thatbreaking out of a labeled block sets theBreaks flag on the outer flow and messes with the function not appearing to always terminate.

Another issue is that acontinue (and likelybreak as well) from inside an inner loop to an outer loop won't set theContinues/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. Clearlyunusable 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-definedstatement labels to the internal Binaryen labels passed to module.br().Thanks to the existing logic to handle unlabeled break/continue, addingsupport for labeled break/continue is a breeze.FixesAssemblyScript#2889.
@HerrCai0907
Copy link
Member

In TS, there are new ASTNodeLabeledStatement to identifier this labeled statement, could we use the same concept?

@CountBleck
Copy link
MemberAuthor

@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 properFlowFlags for acontinue to an outer loop...maybe the class I use to keep track of labels should contain the flow, and I can use something likedo flow.set(...) while ((flow = flow.parent) != targetFlow). (Of course, the actual code would look nicer than that.)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support labeled statements
2 participants
@CountBleck@HerrCai0907

[8]ページ先頭

©2009-2025 Movatter.jp