Repository navigation
A labelled break or continue disposes and releases up to the statement it names - #591
Merged
Merged
Conversation
…t it names The scope-exit walk handed each enclosing scope an empty label, and every loop's context held an empty one, so a labelled jump stopped at the first loop body it met: `continue outer` in an `if` in an inner loop never disposed the outer body's `using`. A jump to a labelled block matched nothing and ran on to the function's scope, disposing the function's `using` there and then and, under rc, releasing its locals (a string built in the block was freed before it was returned). The walk now carries the label. A loop's body scope records the loop's label in `loopBodyLabel`, apart from `loopLabel`: a loop's context is a copy of the scope around it and stands in for that scope in the walk. A labelled statement that is not a loop gets a scope of its own, `labelledScopeLabel`, where its jumps stop. An unlabelled break in a labelled block left only the block: LabelOp lowering claimed unlabelled jumps, which in JS belong to the loop or switch around it. And a loop body written without braces had no body scope, so a break or continue in it disposed the scopes around the loop; it is now made one as a brace-less if branch is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #590: the scope-exit bugs found there, all in the walk a
break/continuemakes outwards to disposeusingdeclarations and release owned locals.A labelled jump stopped at the wrong scope.
scopeExitContinuesOutwardshanded each enclosing scope an empty label, and every loop's context held an empty one. So a labelled jump stopped at the first loop body it met:continue outerwritten inside anifin an inner loop never disposed the outer body'susing.a: { ... break a; }) matched nothing and ran on to the function's scope. It disposed the function'susingthere and then (and never at the function's end). Under-mm=rcit released the function's locals: a string built in the block was freed before it was returned. This is the failure Linux CI showed oncontinue aina: b: forgoes on to the next iteration; labelled for-of no longer crashes #590.Now the walk carries the label all the way out:
loopBodyLabel.loopLabelalone can't do this, because a loop's context is a copy of the scope around it and stands in for that scope in the walk.labelledScopeLabelmarks where its jumps stop.labelledScopeLabelso they don't inherit it.An unlabelled
breakin a labelled block only left the block. TheLabelOplowering claimed unlabelled jumps; in JS they belong to the loop or switch around the block.LabelOpnow claims only its own label (LowerToAffineLoops.cpp).A loop body without braces had no body scope. In
for (...) if (c) continue;, the jump disposed the scopes around the loop, here the function'susing. Such a body is now made a scope throughmlirGenBranchStatement, as a brace-lessifbranch already was.New test
00labelled_scope_exit.ts(compile + JIT, and inTSLANG_CORPUS) checks dispose order for each case, plus the rc string cases. It passes under gc, rc and none, and--verify-ownershipis quiet on it.Found, not fixed here
Any
usingin acaseclause crashes the compiler, even with nobreak. Case clauses are generated in the enclosing scope's context, so theusinglands in the function's dispose list, but its symbol is gone by the function's end. A switch needs a scope of its own, and an unlabelledbreakneeds to stop at it. That's a different change, which I'll propose separately.Full
ctest -C Release -j 8 --timeout 300: 4112/4112 passed.🤖 Generated with Claude Code