Repository navigation
A using in a switch case is disposed where the switch ends; break in an if in a case compiles - #592
Merged
Merged
Conversation
… in an `if` in a case compiles
The cases of a switch were generated with a copy of the enclosing context that shared its
dispose and release lists, while their names lived only in the switch's symbol table scope.
Any `using` in a case crashed the compiler at the end of the enclosing scope, which looked
the name up and did not find it.
The switch now has a scope of its own (GenContext::isSwitchScope, its own lists), exited at
the start of the merge block: a `break` at the end of a case, the last case falling off, and
no case matching all come through there. A `using` in a case the path did not run has a
dispose guard that is still off, and an owned local there is still null.
- `break` and `continue` walk outwards differently now (DisposeDepth::BreakScope): an
unlabelled `break` stops at the switch; a `continue` passes through it to the loop.
- SwitchOpLowering gives a `break` written inside a case (in an `if`, say) a jump target.
It had none, and lowering it crashed.
- A case's lone `{ }` block that declares a `using` keeps its scope, so it is disposed
before a case falling through runs the next one.
- A switch whose cases declare a `using` (or hold a typed catch, on Itanium) is wrapped in a
catch-less TryOp as a block is, so an exception after a case's `using` runs its cleanup.
- The safe-cast narrowing of a switch on a property cleared insertIntoParentScope on the
switch's context instead of its own copy, so later cases' variables were gone by the merge.
- The ownership verifier takes a hoisted owned slot (no initializer) as acquired at its
stores rather than at the hoisted declaration, where the slot is still null. This also
stops it reporting a `break` ahead of a declaration in a try body.
Test: 00switch_case_scope_exit.ts (compile + JIT, and in the rc/none corpus).
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.
Builds on the scope-exit walk from #591.
Problem
switch (i) { case 0: { using c = new D("case"); } }(orcase 0: using c = ...; break;) crashed MLIRGen with 0xC0000005. The cases were generated with a copy of the enclosing context that shared itsusingVars/ownedVars, while their names lived only in the switch's symbol table scope, so the dispose at the end of the enclosing scope lookedcup and didn't find it.While testing, a second crash turned up: a
breakinside anifin a case (case 0: if (x) break;) had no jump target inSwitchOpLowering, so lowering it crashed.Fix
GenContext::isSwitchScope, its own lists,parentBlockContext). It is exited at the start of the merge block, which every normal way out reaches: abreakat the end of a case, the last case falling off its end, and no case matching. A hoistedusingfrom a case that didn't run has a dispose guard that is still off. A hoisted owned local there is still null.breakvscontinue: newDisposeDepth::BreakScope. An unlabelledbreakstops at the switch scope, while acontinuepasses through it to the loop body.SwitchOpLoweringgives an unlabelledBreakOpnested in a case a jump target, the continuation. That jump skips the merge block, whose cleanup thebreakalready did on its way out.{ }block that declares ausingis no longer inlined, so it is disposed before a case falling through runs the next one, as in JS.using(or hold a typed catch, on Itanium) is wrapped in a catch-less TryOp, the same waymlirGenBlockWithUnwindCleanupwraps a block. The switch's storage is hoisted in front of that TryOp, so an exception after a case'susingis stored runs the switch's cleanup. Without this the verifier flagged the unwind leak inside ausingloop body.switch (o.prop)clearedinsertIntoParentScopeon the shared switch context instead of on its own copy. Every later case then declared its variables in a per-case scope, and they were gone by the merge.VariableOpwith no initializer) is now treated as acquired at its stores, not at the hoisted declaration, where the slot is null (VariableOpLowering). This also removes a false report for abreakthat comes before a declaration in a try body.Verified
using+ break, falling through, no case matched, braced cases falling through,breakfrom anif, a switch in a loop whose body has ausing(continue/break/default), and a switch on a property.--emit=mlir-affine --verify-ownershipis quiet under gc/rc/own.00switch_case_scope_exit.ts: compile + JIT, and added toTSLANG_CORPUS.ctest -C Release -j 8 --timeout 300: 100% passed, 4118/4118 (Windows, VS 2026 Release).Not fixed here (pre-existing)
usings in declaration order, but JS disposes them in reverse. The test avoids twousings in one scope.let s = ...; { using a = new D(); return s; }: the verifier reportssas unreleased on the unwind path, with a plain block as well as with a switch.🤖 Generated with Claude Code