Repository navigation
The usings of a scope are disposed in reverse declaration order - #593
Merged
Merged
Conversation
`{ using a = new D("a"); using b = new D("b"); }` disposed `a` then `b`; JS disposes the last
one declared first, so a later resource can still use an earlier one while it is disposed.
mlirGenDisposable walked the scope's usingVars forward; it now walks them in reverse, as
mlirGenReleaseOwned beside it already did for owned locals.
Every scope exit goes through it - the end of a block, `return`, `break`, `continue`, the
unwind cleanup region of a block or try body, and the switch's wrap - so they all change
together. With the dispose guard, a `[Symbol.dispose]()` that throws in the body still leaves
the earlier `using`s for the cleanup to dispose.
New test 00using_dispose_order.ts: two and three `using`s in one scope, through falling off
a block and a function body, a `return` out of a nested block, a throw out of a block and out
of a try body, a `[Symbol.dispose]()` that throws, `continue`/`break` out of a loop body, and `break` out of a switch case.
00switch_case_scope_exit.ts gains a `using` in each of two cases, which share the switch's
scope. No existing test depended on the old order (each had at most one `using` per scope,
and nested scopes were already disposed inner first), so none of their expectations changed.
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.
{ using a = new D("a"); using b = new D("b"); }disposedathenb. JS disposes the last one declared first, so a later resource can still use an earlier one while it is disposed.mlirGenDisposable(MLIRGenImpl.h) walked the scope'susingVarsforward. It now walks them in reverse, asmlirGenReleaseOwnedbeside it already does for owned locals. It is the only place that iterates the list, and every scope exit goes through it: the end of a block,return,break,continue, the unwind cleanup region of a block or try body, and the switch's TryOp wrap from #592. So they all changed together. The order across scopes was already right (inner scope first).Because of the dispose guard, a
[Symbol.dispose]()that throws still leaves the earlierusings for the cleanup to dispose:body;b;a;caught;, as in JS.Tests
00using_dispose_order.ts: two and threeusings in one scope, through falling off a block and a function body, areturnout of a nested block, a throw out of a block and out of a try body, a throwing dispose,continue/breakout of a loop body, andbreakout of a switch case. It is registered for compile and JIT, and through the corpus for rc and none.00switch_case_scope_exit.tsgets ausingin each of two cases. The cases share the switch's scope, and Ausingin a switch case is disposed where the switch ends;breakin anifin a case compiles #592 had avoided this shape because of this bug.usingper scope.Verification
--emit=mlir-affine --verify-ownership(rc, own) reports nothing on either file.ctest -C Release -j 8 --timeout 300: 100% passed, 0 failed out of 4124. The two test files were edited after that run; I reran them and the ownership-verifier shards afterwards, and all 20 passed.Not in scope (pre-existing, still open)
let s = ...; { using a = new D(); return s; }: a function-level owned local has no unwind release when a nested using-block's cleanup unwinds out of the function.🤖 Generated with Claude Code