From 3d42a3ebeabec8ee81355f546d33cb445251fee6 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Tue, 29 Sep 2026 10:55:52 +0100 Subject: [PATCH 1/3] A folded const's reference is taken over by no receiver that stores it `const a = new C()` has no storage: the name is the `new`'s own result, marked __owned_result, so every later mention offered that one reference to its receiver, and each took it. An inner `let` gave it back while the name still read it; two `let`s, or a push then a `let`, gave one count two holders; a push or a field store in a loop took it on every iteration. `let b = h.c = new C()` took one `new` twice. Under rc these double-freed or read freed memory (00owned_named_consts.ts: eight cases, each failing alone under -mm=rc before this change). - MLIROwnedReference.h: carriesUnclaimedReference (owned, not yet consumed) and mayTakeOverReference (also not a folded const), used by every receiver. - MLIRGen marks a folded const's producer __owned_result_named. Storing receivers retain it; a return or a delete still takes it over. - OwnedReturnConsumptionPass keeps a named call result's receiver retain unless it is the one a return took. - -mm=own: a taker plus the temporary's later release in the same block is a move when rc gave the taker its own reference, so `const a = new C(); let b = a;` stays accepted. NewInterface, which rc gives no reference, is still a second owner. Docs: reference-counting-evaluation.md 9.79; own spec 11.1 and 11.4. Co-Authored-By: Claude Opus 5.5 --- tslang/docs/reference-counting-evaluation.md | 40 ++++ .../2026-09-24-own-memory-model-design.md | 10 + tslang/include/TypeScript/Defines.h | 8 + .../TypeScript/MLIRLogic/MLIRCodeLogic.h | 11 +- .../TypeScript/MLIRLogic/MLIROwnedReference.h | 44 ++++ tslang/lib/TypeScript/MLIRGenExpressions.cpp | 2 +- tslang/lib/TypeScript/MLIRGenImpl.h | 24 ++- tslang/lib/TypeScript/MLIRGenStatements.cpp | 4 +- tslang/lib/TypeScript/MLIRGenVariables.cpp | 13 ++ .../TypeScript/OwnedReturnConsumptionPass.cpp | 39 +++- .../lib/TypeScript/OwnershipInferencePass.cpp | 55 ++++- tslang/test/tester/CMakeLists.txt | 7 + .../test/tester/tests/00owned_named_consts.ts | 197 ++++++++++++++++++ 13 files changed, 427 insertions(+), 27 deletions(-) create mode 100644 tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h create mode 100644 tslang/test/tester/tests/00owned_named_consts.ts diff --git a/tslang/docs/reference-counting-evaluation.md b/tslang/docs/reference-counting-evaluation.md index 7ca8b6922..6600019a3 100644 --- a/tslang/docs/reference-counting-evaluation.md +++ b/tslang/docs/reference-counting-evaluation.md @@ -5903,3 +5903,43 @@ its characters; a C-owned `char *` has none. Under `-mm=gc` that is harmless - n header. Under `-mm=rc` a release of such a value reads a count that is not there, which is undefined. Not fixed: tsbindgen v1 targets `gc`. Revisit if rc becomes a supported model for bindings - the likely shape is a distinct C-string type in the compiler that is never released. + +### 9.79 A folded `const` was taken over by every receiver that met it + +Found by the review of `-mm=own` phase 0, which crashed on these programs and so asked why `rc` +did not. `rc` did: the same programs double-free or read freed memory under `rc` today, and +only luck in what the allocator reused next had hidden it. `00owned_named_consts.ts` collects +eight of them. Under `rc` each failed on its own - four read a block `churn()` had since +claimed (999, 1006 where 5 and 7 belong, a string compare gone false), two corrupted the heap +(JIT exit 127, AOT `0xC0000374`). + +**The cause.** `const a = new C()` gets no storage: the name is folded into the `new`'s own +result. Every later mention of `a` is that one SSA value, marked `__owned_result`, and every +receiver of §9.25 asked only that question. So each one took the reference over: + +- `{ let b = a; } ... a.v` - the inner `let` took it and gave it back at its scope exit, and `a` + read freed memory. +- `let b = a; let c = a;`, and `arr.push(a); let d = a;` - two holders, one count. +- `for (...) { arr.push(a) }`, `for (...) { h.s = s }` - one reference taken on every iteration; + the field store's overwrite released the very string it was storing. +- `const r = make(); { let b = r; }` - the same through §9.27's pass, which took the first + receiver's retain for the call's +1. + +And one without a name: `let b = h.c = new C()` offers one `new` to the field and then the local. +Only the array push (§9.74) asked whether the reference had been taken already; no other +receiver did. + +**The fix.** One question for every receiver, `mayTakeOverReference` in `MLIROwnedReference.h`: +the producer carries a reference, no receiver took it yet, and it is not a folded `const`. +MLIRGen marks a folded `const`'s producer `__owned_result_named` where the name is bound. A named +value is retained by each receiver instead, and its own reference is given back at the end of +its block like any unclaimed temporary (§9.30). A `return` and a `delete` still take it over - +nothing reads the name after either - through `carriesUnclaimedReference`, which leaves out only +the naming check. §9.27's pass keeps a named result's retain unless it is the one a `return` +took. + +Cost: a retain and a release for `const a = new C(); arr.push(a)`, which used to be free. + +Under `-mm=own` the extra pair is a move, not a second owner: one taker plus the temporary's +release later in the same block is one owner, and the inference pass erases both. A million +`const p = make(); let q = p;` iterations peak at 4.2 MB under both `rc` and `own` (`none` 50.3). diff --git a/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md b/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md index e3038b216..3f836d3eb 100644 --- a/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md +++ b/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md @@ -410,6 +410,14 @@ The rule as built: non-owning declaration, `--di`'s `ts.DebugVariable`, and the retain/release bookkeeping. - **Owners** of V are its taking uses, plus one if V is released as a temporary. There must be at most one owner. Zero is a leak, as under rc. +- **A move out of a temporary.** One taking use and one temporary release count as one owner + when the release comes after the taker in the same block and rc gave the taker a reference of + its own (a `ts.Retain` of V in front of it, or the `ts.RetainSlot` of an owning `let`). The + value moves into the taker, and the release is erased with the retains. This is the shape of + a folded `const` handed on, `const a = new C(); let b = a;`, since rc stopped letting a + receiver take a named `const`'s reference over (that let one reference have several holders + under rc: `00owned_named_consts.ts`). A + taker rc gives no reference, such as `ts.NewInterface`, is still a second owner. - **Locality**: the taking use is in V's defining block, and nothing uses V after it. That one check rejects a move inside a loop (§2.6), a move on one branch, and use after move, with no liveness analysis. Phase 1 relaxes it. @@ -477,3 +485,5 @@ Tested both ways: value the callee does not own; spec §2.4's owned-by-callee parameter is phase 4), and a move out of the value's own block are all rejected. - Results of runtime helpers and `declare`d functions are rejected. +- A field initialized from an array literal of eight numbers is rejected ("takes a second + reference" at the literal); four are accepted. Not yet diagnosed. diff --git a/tslang/include/TypeScript/Defines.h b/tslang/include/TypeScript/Defines.h index 3d4047278..85d390ad7 100644 --- a/tslang/include/TypeScript/Defines.h +++ b/tslang/include/TypeScript/Defines.h @@ -47,6 +47,14 @@ // and then dropped, which is what expression-shaped code is made of. See §9.30. #define OWNED_RESULT_CONSUMED_ATTR_NAME "__owned_result_consumed" +// Marks the producer of a `const` that was folded into its value rather than given storage - +// `const a = new C()`. The name and the value are one SSA value, so every later mention of `a` +// is the operation's own result, and a receiver cannot tell whether this mention is the last. +// None that stores it may take the reference over; each retains, and the reference the producer +// carried is given back where §9.30 gives back any other unclaimed one. A `return` or a `delete` +// still takes it over: nothing after either reads the name. See MLIROwnedReference.h. +#define OWNED_RESULT_NAMED_ATTR_NAME "__owned_result_named" + // Marks a `ts.CreateBoundFunction` whose `this` is a capture box built for it a moment earlier, // rather than a receiver that belongs to somebody else. Only such a closure owns its `this`, and // only it gets the type tag at CLOSURE_TYPE_INDEX - a bound method must not take ownership of diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRCodeLogic.h b/tslang/include/TypeScript/MLIRLogic/MLIRCodeLogic.h index 4e68b5d88..da48336c3 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRCodeLogic.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRCodeLogic.h @@ -7,6 +7,7 @@ #include "TypeScript/Defines.h" #include "TypeScript/MLIRLogic/MLIRDefines.h" #include "TypeScript/MLIRLogic/MLIRGenContext.h" +#include "TypeScript/MLIRLogic/MLIROwnedReference.h" #include "TypeScript/MLIRLogic/MLIRTypeHelper.h" #include "llvm/ADT/APSInt.h" @@ -761,11 +762,13 @@ class MLIRCustomMethods // first freed an element the other still held. Nothing to do with `splice` - it // reproduces with no splice anywhere - but §9.74's release is what made it visible, // because until then an array that outlived its elements never gave them back. - auto *definingOp = value.getDefiningOp(); - if (definingOp && definingOp->hasAttr(OWNED_RESULT_ATTR_NAME) && - !definingOp->hasAttr(OWNED_RESULT_CONSUMED_ATTR_NAME)) + // + // And the same one array receiving it again in a loop, from a `const` made once + // before the loop, is the same mistake repeated: mayTakeOverReference turns away a + // folded `const` for that reason (see OWNED_RESULT_NAMED_ATTR_NAME). + if (mayTakeOverReference(value)) { - definingOp->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME, builder.getUnitAttr()); + value.getDefiningOp()->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME, builder.getUnitAttr()); continue; } diff --git a/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h b/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h new file mode 100644 index 000000000..9b2a1cd31 --- /dev/null +++ b/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h @@ -0,0 +1,44 @@ +#ifndef MLIR_TYPESCRIPT_COMMONGENLOGIC_MLIROWNEDREFERENCE_H_ +#define MLIR_TYPESCRIPT_COMMONGENLOGIC_MLIROWNEDREFERENCE_H_ + +#include "TypeScript/Defines.h" + +#include "mlir/IR/Operation.h" +#include "mlir/IR/Value.h" + +namespace typescript +{ + +// Does this value still carry a reference nobody has taken over? +// +// The producer has to carry one at all (OWNED_RESULT_ATTR_NAME), and no receiver can have taken +// it already (OWNED_RESULT_CONSUMED_ATTR_NAME). There is only ever one to take, and one value can +// reach two receivers: `let b = h.c = new C()` offers the same `new` to the field and then to the +// local. +// +// This is the whole question for a receiver after which nothing reads the value - a `return`, a +// `delete`. Every other receiver asks mayTakeOverReference. +inline bool carriesUnclaimedReference(mlir::Value value) +{ + auto *definingOp = value ? value.getDefiningOp() : nullptr; + return definingOp && definingOp->hasAttr(OWNED_RESULT_ATTR_NAME) && + !definingOp->hasAttr(OWNED_RESULT_CONSUMED_ATTR_NAME); +} + +// May a receiver that stores this value - a local, a field or element, an array, a literal - take +// its reference over instead of retaining? +// +// Not when the value is a folded `const` (OWNED_RESULT_NAMED_ATTR_NAME). Every mention of the +// name is this very value, so the receiver cannot tell the last mention from the first: one in an +// inner scope releases the reference while the name still reads it, and one in a loop takes the +// same reference on every iteration. +// +// Answering no costs a retain and a release. Answering yes wrongly frees live memory. +inline bool mayTakeOverReference(mlir::Value value) +{ + return carriesUnclaimedReference(value) && !value.getDefiningOp()->hasAttr(OWNED_RESULT_NAMED_ATTR_NAME); +} + +} // namespace typescript + +#endif // MLIR_TYPESCRIPT_COMMONGENLOGIC_MLIROWNEDREFERENCE_H_ diff --git a/tslang/lib/TypeScript/MLIRGenExpressions.cpp b/tslang/lib/TypeScript/MLIRGenExpressions.cpp index b96cfd679..42bc2c6a8 100644 --- a/tslang/lib/TypeScript/MLIRGenExpressions.cpp +++ b/tslang/lib/TypeScript/MLIRGenExpressions.cpp @@ -1196,7 +1196,7 @@ namespace mlirgen // `delete new C()` and `delete c`, where `c` is a `const` the compiler kept as a // value rather than storage: the reference is one nobody has claimed, and §9.30 // releases those at the end of the block. This is the claim. - if (producesOwnedReference(expr)) + if (producesOwnedReferenceAtLastUse(expr)) { consumeOwnedReference(expr); } diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index 0b50f9439..c564dd90b 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -986,15 +986,20 @@ class MLIRGenImpl // runtime helper, or a function from a module built before returns retained their result, // hands back a heap value with no retain behind it, and consuming one of those would skip a // retain nobody performed. Answering "no" for something that was in fact owned only leaks. + // + // Asked by a receiver that stores the value, so a reference some other receiver already + // took, or one a folded `const` still needs for its later mentions, is not there to take - + // see mayTakeOverReference. bool producesOwnedReference(mlir::Value value) { - if (!value) - { - return false; - } + return mayTakeOverReference(value); + } - auto *definingOp = value.getDefiningOp(); - return definingOp && definingOp->hasAttr(OWNED_RESULT_ATTR_NAME); + // The same question from a receiver after which nothing reads the value - a `return`, a + // `delete` - which may take over a folded `const`'s reference too. + bool producesOwnedReferenceAtLastUse(mlir::Value value) + { + return carriesUnclaimedReference(value); } // Records that a receiver has taken over the reference this value carried, so nothing later @@ -1023,7 +1028,10 @@ class MLIRGenImpl // // A record-shaped value retains through its own routine, which walks its owning fields, so // the boxed-literal case needs one of these on the whole tuple rather than one per field. - void mlirGenRetainCaptured(mlir::Location location, mlir::ValueRange values) + // + // `atLastUse` is for a `return`: nothing after it reads the value, so a folded `const`'s + // reference can go out with it rather than being retained here and given back as a temporary. + void mlirGenRetainCaptured(mlir::Location location, mlir::ValueRange values, bool atLastUse = false) { for (auto value : values) { @@ -1034,7 +1042,7 @@ class MLIRGenImpl // a value that already carries a reference for its receiver is taken over rather // than retained again (§9.25) - `[new C()]`, and `return new C()` alike - if (producesOwnedReference(value)) + if (atLastUse ? producesOwnedReferenceAtLastUse(value) : producesOwnedReference(value)) { consumeOwnedReference(value); } diff --git a/tslang/lib/TypeScript/MLIRGenStatements.cpp b/tslang/lib/TypeScript/MLIRGenStatements.cpp index cb181ef26..105b419ee 100644 --- a/tslang/lib/TypeScript/MLIRGenStatements.cpp +++ b/tslang/lib/TypeScript/MLIRGenStatements.cpp @@ -50,7 +50,7 @@ namespace mlirgen // Cast first, for the reason spelled out at the return statement below. EXIT_IF_FAILED(castToDeclaredReturnType(loc(body), resultValue, genContext)) - mlirGenRetainCaptured(loc(body), mlir::ValueRange{resultValue}); + mlirGenRetainCaptured(loc(body), mlir::ValueRange{resultValue}, /*atLastUse=*/true); return mlirGenReturnValue(loc(body), resultValue, false, genContext); } @@ -479,7 +479,7 @@ namespace mlirgen // interface builds a heap block the tuple's own reference says nothing about. EXIT_IF_FAILED(castToDeclaredReturnType(location, expressionValue, genContext)) - mlirGenRetainCaptured(location, mlir::ValueRange{expressionValue}); + mlirGenRetainCaptured(location, mlir::ValueRange{expressionValue}, /*atLastUse=*/true); EXIT_IF_FAILED(mlirGenScopeExit(location, DisposeDepth::FullStack, {}, &genContext)); diff --git a/tslang/lib/TypeScript/MLIRGenVariables.cpp b/tslang/lib/TypeScript/MLIRGenVariables.cpp index 8a780bcf0..ac71bc435 100644 --- a/tslang/lib/TypeScript/MLIRGenVariables.cpp +++ b/tslang/lib/TypeScript/MLIRGenVariables.cpp @@ -211,6 +211,19 @@ namespace mlirgen if (variableDeclarationInfo.needsIdentityStorage && mlir::failed(createLocalVariable(location, variableDeclarationInfo, genContext))) return mlir::Type(); + + // Folded: from here the name *is* this value, so a receiver meeting it later + // cannot know whether it is the last mention - see OWNED_RESULT_NAMED_ATTR_NAME. + // A ConstRef's initial is a reference to storage and owns nothing of its own. + if (!variableDeclarationInfo.needsIdentityStorage && compileOptions.tracksOwnership() + && variableDeclarationInfo.initial + && mth.ownsHeapMemory(location, variableDeclarationInfo.initial.getType())) + { + if (auto *definingOp = variableDeclarationInfo.initial.getDefiningOp()) + { + definingOp->setAttr(OWNED_RESULT_NAMED_ATTR_NAME, builder.getUnitAttr()); + } + } } else if (mlir::failed(createLocalVariable(location, variableDeclarationInfo, genContext))) return mlir::Type(); } diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index ed5760913..740c29955 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -5,6 +5,7 @@ #include "TypeScript/Passes.h" #include "TypeScript/Defines.h" #include "TypeScript/MLIRLogic/MLIRTypeHelper.h" +#include "TypeScript/MLIRLogic/MLIROwnedReference.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -263,8 +264,7 @@ class OwnedReturnConsumptionPass // branch borrows, the receiver's retain is already the right and only answer. auto carriesOwned = [&](mlir_ts::ResultOp resultOp) { auto *definingOp = resultOp->getOperand(0).getDefiningOp(); - return definingOp && definingOp->hasAttr(OWNED_RESULT_ATTR_NAME) && - !definingOp->hasAttr(OWNED_RESULT_CONSUMED_ATTR_NAME) && + return ::typescript::mayTakeOverReference(resultOp->getOperand(0)) && definingOp->getParentRegion() == resultOp->getParentRegion(); }; @@ -891,6 +891,26 @@ class OwnedReturnConsumptionPass return false; } + // Is this retain the one a `return` of `result` took? Scope exit may release other locals + // between the two, but nothing else may use the value before it is returned. + static bool retainFeedsReturn(mlir_ts::RetainOp retainOp, mlir::Value result) + { + for (auto *op = retainOp->getNextNode(); op; op = op->getNextNode()) + { + if (mlir::isa(op)) + { + return op->getNumOperands() > 0 && op->getOperand(0) == result; + } + + if (llvm::is_contained(op->getOperands(), result)) + { + return false; + } + } + + return false; + } + // The retain a receiver put on this call's result, if it took one. Two shapes, matching the // two ways §9.25's receivers acquire: a `ts.Retain` on the value itself (a field or element // store, a literal capturing it, a return passing it on), and a `ts.RetainSlot` on the @@ -899,19 +919,32 @@ class OwnedReturnConsumptionPass // Returning null is the ordinary answer for a result nobody took - `f();` on its own, or // `f().n` - and it is left exactly as it is. Consuming a reference no receiver balances // would free the value while the expression is still using it. + // + // A folded `const` (OWNED_RESULT_NAMED_ATTR_NAME) is the exception: its name is this very + // value, so the one receiver that may take its reference over is a `return`, after which + // nothing reads the name. Any other receiver keeps its retain, and the call's +1 is given + // back as a temporary like any other nobody took. static mlir::Operation *findReceiverRetain(mlir::Value result) { + auto *definingOp = result.getDefiningOp(); + auto named = definingOp && definingOp->hasAttr(OWNED_RESULT_NAMED_ATTR_NAME); + for (auto *user : result.getUsers()) { if (auto retainOp = mlir::dyn_cast(user)) { - if (retainOp.getReference() == result) + if (retainOp.getReference() == result && (!named || retainFeedsReturn(retainOp, result))) { return retainOp.getOperation(); } } } + if (named) + { + return nullptr; + } + for (auto *user : result.getUsers()) { auto varOp = mlir::dyn_cast(user); diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index d78c00770..3625a2858 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -90,18 +90,23 @@ class OwnershipInferencePass : public mlir::PassWrapper proven; + llvm::SmallVector toErase; for (auto value : candidates) { - if (hasOneOwner(value) && isFresh(value)) + mlir::Operation *movedRelease = nullptr; + if (hasOneOwner(value, movedRelease) && isFresh(value)) { proven.insert(value); + if (movedRelease) + { + toErase.push_back(movedRelease); + } } } // What is proven loses its retains; any other retain is a second reference nobody // proved safe - of a loaded value, a parameter, an unknown producer, or of a fresh value // whose own check was reported above. - llvm::SmallVector toErase; for (auto *op : retains) { auto value = retainedValue(op); @@ -236,12 +241,19 @@ class OwnershipInferencePass : public mlir::PassWrapper releases; for (auto *user : value.getUsers()) { if (mlir::isa(user)) @@ -251,7 +263,7 @@ class OwnershipInferencePass : public mlir::PassWrapper(user)) { - released = true; + releases.push_back(user); continue; } @@ -282,17 +294,23 @@ class OwnershipInferencePass : public mlir::PassWrappergetBlock() != home || + !taker->isBeforeInBlock(releases.front()) || !takerAcquires(taker, value)) + { + reportSecondReference(taker); + return false; + } + + movedRelease = releases.front(); } // Every other use must come first. A use in another block runs after the whole of this // one, and so after the move. for (auto *user : value.getUsers()) { - if (user == taker || mlir::isa(user)) + if (user == taker || user == movedRelease || mlir::isa(user)) { continue; } @@ -308,6 +326,25 @@ class OwnershipInferencePass : public mlir::PassWrapper(taker)) + { + return llvm::any_of(varOp.getResult().getUsers(), + [](mlir::Operation *user) { return mlir::isa(user); }); + } + + return llvm::any_of(value.getUsers(), [&](mlir::Operation *user) { + return mlir::isa(user) && user->getBlock() == taker->getBlock() && + user->isBeforeInBlock(taker); + }); + } + // The declaration's name survives only as debug metadata (`--di`), the way LowerToLLVM's // VariableOp lowering reads it. Without it the error still points at the right place. static llvm::StringRef varName(mlir_ts::VariableOp varOp) diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index f9625e479..6d4ceb2cb 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -423,6 +423,7 @@ tslang_add_test(NAME test-compile-00-owned-array-ops COMMAND test-runner "${PROJ tslang_add_test(NAME test-compile-00-owned-inline-records COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_inline_records.ts") tslang_add_test(NAME test-compile-00-owned-transfer COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_transfer.ts") tslang_add_test(NAME test-compile-00-owned-call-results COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_call_results.ts") +tslang_add_test(NAME test-compile-00-owned-named-consts COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_named_consts.ts") tslang_add_test(NAME test-compile-00-owned-temporaries COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_temporaries.ts") tslang_add_test(NAME test-compile-00-owned-interfaces COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_interfaces.ts") tslang_add_test(NAME test-compile-00-owned-closures COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_closures.ts") @@ -876,6 +877,7 @@ tslang_add_test(NAME test-jit-00-owned-array-ops COMMAND test-runner -jit "${PRO tslang_add_test(NAME test-jit-00-owned-inline-records COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_inline_records.ts") tslang_add_test(NAME test-jit-00-owned-transfer COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_transfer.ts") tslang_add_test(NAME test-jit-00-owned-call-results COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_call_results.ts") +tslang_add_test(NAME test-jit-00-owned-named-consts COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_named_consts.ts") tslang_add_test(NAME test-jit-00-owned-temporaries COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_temporaries.ts") tslang_add_test(NAME test-jit-00-owned-interfaces COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_interfaces.ts") tslang_add_test(NAME test-jit-00-owned-closures COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_closures.ts") @@ -1468,6 +1470,8 @@ tslang_add_test(NAME test-jit-rc-owned-transfer COMMAND test-runner -jit -mm=rc tslang_add_test(NAME test-jit-none-owned-transfer COMMAND test-runner -jit -mm=none "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_transfer.ts") tslang_add_test(NAME test-jit-rc-owned-call-results COMMAND test-runner -jit -mm=rc "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_call_results.ts") tslang_add_test(NAME test-jit-none-owned-call-results COMMAND test-runner -jit -mm=none "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_call_results.ts") +tslang_add_test(NAME test-jit-rc-owned-named-consts COMMAND test-runner -jit -mm=rc "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_named_consts.ts") +tslang_add_test(NAME test-jit-none-owned-named-consts COMMAND test-runner -jit -mm=none "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_named_consts.ts") tslang_add_test(NAME test-jit-rc-owned-temporaries COMMAND test-runner -jit -mm=rc "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_temporaries.ts") tslang_add_test(NAME test-jit-none-owned-temporaries COMMAND test-runner -jit -mm=none "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_temporaries.ts") tslang_add_test(NAME test-jit-rc-owned-interfaces COMMAND test-runner -jit -mm=rc "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_interfaces.ts") @@ -1774,6 +1778,7 @@ set(TSLANG_CORPUS 00owned_iteration.ts 00owned_literals.ts 00owned_locals.ts + 00owned_named_consts.ts 00owned_nested_captures.ts 00owned_strings.ts 00owned_temporaries.ts @@ -2032,6 +2037,7 @@ set(TSLANG_CORPUS_RC_NAMED 00owned_iteration.ts 00owned_literals.ts 00owned_locals.ts + 00owned_named_consts.ts 00owned_nested_captures.ts 00owned_strings.ts 00owned_temporaries.ts @@ -2074,6 +2080,7 @@ set(TSLANG_CORPUS_NONE_NAMED 00owned_interfaces.ts 00owned_iteration.ts 00owned_literals.ts + 00owned_named_consts.ts 00owned_nested_captures.ts 00owned_strings.ts 00owned_temporaries.ts diff --git a/tslang/test/tester/tests/00owned_named_consts.ts b/tslang/test/tester/tests/00owned_named_consts.ts new file mode 100644 index 000000000..c043d89ea --- /dev/null +++ b/tslang/test/tester/tests/00owned_named_consts.ts @@ -0,0 +1,197 @@ +// A `const` whose initializer carries a reference - `const a = new C()`, `const s = "a" + k`, +// `const r = make()` - has no storage of its own: the name is folded into the value itself. So +// every later mention of the name is the very value that carried the reference, and a receiver +// that asks "does this carry a reference I can take over?" gets yes at every mention, not just +// the first. Taking it over is only right for a value that is used once, right there. +// +// Each case below had a receiver take over a reference the name still needed: +// - a `let` in an inner scope took it, released it at its scope exit, and the name went on +// reading freed memory; +// - two receivers both took the one reference, so the value had two holders and one count; +// - a receiver inside a loop took it on every iteration, from a value made once. +// And one without a name: `let b = h.c = new C()` hands the same `new` to the field and then to +// the local, and both took it. +// +// Every case calls `churn()` between the suspect release and the read, so that a freed block is +// claimed by something else and a use-after-free reads a wrong answer instead of a lucky one. +// See 00owned_transfer.ts for why that matters. + +class C { + x: number; + v: number[] = []; + + constructor(x: number) { + this.x = x; + } +} + +class H { + c: C = new C(0); + s: string = ""; +} + +function churn() { + const keep: C[] = []; + for (let i = 0; i < 64; i++) { + const c = new C(999); + c.v.push(99); + c.v.push(99); + c.v.push(99); + c.v.push(99); + c.v.push(99); + c.v.push(99); + c.v.push(99); + keep.push(c); + } +} + +function churnStrings() { + const keep: string[] = []; + for (let i = 0; i < 64; i++) { + keep.push("b" + (i % 10)); + } +} + +function filled(n: number) { + const c = new C(n); + for (let i = 0; i < n; i++) { + c.v.push(i); + } + + return c; +} + +// a `let` in an inner scope, then the name again after that scope has ended +function innerLetThenName() { + const a = new C(3); + a.v.push(1); + a.v.push(2); + a.v.push(3); + { + let b = a; + b.x = 4; + } + + churn(); + return a.v.length + a.x; +} + +// the same `let` inside a loop: taken over on every iteration +function innerLetInLoop() { + const a = new C(0); + for (let i = 0; i < 5; i++) { + a.v.push(i); + } + + for (let i = 0; i < 3; i++) { + let b = a; + b.x += 1; + } + + churn(); + return a.v.length + a.x; +} + +// two `let`s of one name +function twoLets() { + const a = new C(3); + a.v.push(1); + a.v.push(2); + a.v.push(3); + let r = 0; + { + let b = a; + let c = a; + r = b.v.length + c.v.length; + } + + churn(); + return r + a.v.length; +} + +// a push, then a `let` of the same name +function pushThenLet() { + const arr: C[] = []; + const c = new C(3); + c.v.push(1); + c.v.push(2); + c.v.push(3); + arr.push(c); + { + let d = c; + d.x = 4; + } + + churn(); + return arr[0].v.length + arr[0].x; +} + +// a push inside a loop: one value, three elements +function pushInLoop() { + let keep = new C(0); + { + const arr: C[] = []; + const c = new C(5); + for (let i = 0; i < 3; i++) { + arr.push(c); + } + + keep = arr[0]; + } + + churn(); + return keep.x; +} + +// a fresh string stored into a field inside a loop: each overwrite released the one before it, +// and the one before it was the same string +function fieldStoreInLoop() { + let k: number = 3; + const h = new H(); + const s = "a" + k; + for (let i = 0; i < 3; i++) { + h.s = s; + } + + churnStrings(); + return h.s == "a3" && s == "a3" ? 1 : 0; +} + +// a call's result, named, then a `let` in an inner scope +function callResultThenName() { + const a = filled(3); + { + let b = a; + b.x = 4; + } + + churn(); + return a.v.length + a.x; +} + +// no name at all: an assignment's value handed on to a second receiver +function chainedAssignment() { + const h = new H(); + { + let b = h.c = new C(3); + b.v.push(1); + b.v.push(2); + b.v.push(3); + } + + churn(); + return h.c.v.length + h.c.x; +} + +function main() { + assert(innerLetThenName() == 7, "a name outlives an inner `let` of it"); + assert(innerLetInLoop() == 8, "a name outlives a `let` of it in a loop"); + assert(twoLets() == 9, "two `let`s of one name are two holders"); + assert(pushThenLet() == 7, "a pushed name outlives a `let` of it"); + assert(pushInLoop() == 5, "a name pushed in a loop is three references"); + assert(fieldStoreInLoop() == 1, "a string stored into a field in a loop survives the overwrites"); + assert(callResultThenName() == 7, "a named call result outlives an inner `let` of it"); + assert(chainedAssignment() == 6, "a field and a local assigned one `new` are two holders"); + + print("done."); +} From daba73378a0b33b357318fdc5da7507ae1444712 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Tue, 29 Sep 2026 11:02:32 +0100 Subject: [PATCH 2/3] Every return of a folded const takes its reference over, not only the first The first return marked the producer consumed, so a second return on another path (`if (x) return c; return c;`) retained instead and handed its caller two references: 1M calls grew from 4.7 MB to 20.1 MB under -mm=rc. A return or delete may take a named value over even when it is marked consumed, since no storing receiver takes a named value and returns on different paths exclude each other. OwnedReturnConsumptionPass likewise takes over every retain that feeds a return of a named call result, where it took only the first (that half predates this). Records as open: a named const still leaks on the path that does not return. Co-Authored-By: Claude Opus 5.5 --- tslang/docs/reference-counting-evaluation.md | 12 +++-- tslang/include/TypeScript/Defines.h | 3 +- .../TypeScript/MLIRLogic/MLIROwnedReference.h | 18 +++++++- tslang/lib/TypeScript/MLIRGenImpl.h | 4 +- .../TypeScript/OwnedReturnConsumptionPass.cpp | 46 ++++++++++++------- .../test/tester/tests/00owned_named_consts.ts | 28 +++++++++++ 6 files changed, 86 insertions(+), 25 deletions(-) diff --git a/tslang/docs/reference-counting-evaluation.md b/tslang/docs/reference-counting-evaluation.md index 6600019a3..51f609b0b 100644 --- a/tslang/docs/reference-counting-evaluation.md +++ b/tslang/docs/reference-counting-evaluation.md @@ -5934,12 +5934,18 @@ the producer carries a reference, no receiver took it yet, and it is not a folde MLIRGen marks a folded `const`'s producer `__owned_result_named` where the name is bound. A named value is retained by each receiver instead, and its own reference is given back at the end of its block like any unclaimed temporary (§9.30). A `return` and a `delete` still take it over - -nothing reads the name after either - through `carriesUnclaimedReference`, which leaves out only -the naming check. §9.27's pass keeps a named result's retain unless it is the one a `return` -took. +nothing reads the name after either - through `mayTakeOverReferenceAtLastUse`, on every path: +`if (x) return c; return c;` has two, and whichever runs is the last use. (The first version of +this fix let only the first `return` take it over and made the second retain, which leaked on +that path: 1M calls went from 4.7 MB to 20.1 MB.) §9.27's pass likewise keeps a named result's +retains except those a `return` took, and now takes every one of those rather than the first. Cost: a retain and a release for `const a = new C(); arr.push(a)`, which used to be free. Under `-mm=own` the extra pair is a move, not a second owner: one taker plus the temporary's release later in the same block is one owner, and the inference pass erases both. A million `const p = make(); let q = p;` iterations peak at 4.2 MB under both `rc` and `own` (`none` 50.3). + +**Open, and older than this:** `const c = new C(); if (x) return c; print(c.x);` leaks `c` on the +path that does not return. The `return` marks the producer consumed, which is what stops §9.30 +releasing it, and that mark is per operation, not per path. diff --git a/tslang/include/TypeScript/Defines.h b/tslang/include/TypeScript/Defines.h index 85d390ad7..137578aa7 100644 --- a/tslang/include/TypeScript/Defines.h +++ b/tslang/include/TypeScript/Defines.h @@ -52,7 +52,8 @@ // is the operation's own result, and a receiver cannot tell whether this mention is the last. // None that stores it may take the reference over; each retains, and the reference the producer // carried is given back where §9.30 gives back any other unclaimed one. A `return` or a `delete` -// still takes it over: nothing after either reads the name. See MLIROwnedReference.h. +// still takes it over, on every path it appears on: nothing after either reads the name. See +// MLIROwnedReference.h. #define OWNED_RESULT_NAMED_ATTR_NAME "__owned_result_named" // Marks a `ts.CreateBoundFunction` whose `this` is a capture box built for it a moment earlier, diff --git a/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h b/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h index 9b2a1cd31..d07e61f0c 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIROwnedReference.h @@ -16,8 +16,8 @@ namespace typescript // reach two receivers: `let b = h.c = new C()` offers the same `new` to the field and then to the // local. // -// This is the whole question for a receiver after which nothing reads the value - a `return`, a -// `delete`. Every other receiver asks mayTakeOverReference. +// A receiver that stores the value asks mayTakeOverReference, and one after which nothing reads +// it - a `return`, a `delete` - asks mayTakeOverReferenceAtLastUse. inline bool carriesUnclaimedReference(mlir::Value value) { auto *definingOp = value ? value.getDefiningOp() : nullptr; @@ -39,6 +39,20 @@ inline bool mayTakeOverReference(mlir::Value value) return carriesUnclaimedReference(value) && !value.getDefiningOp()->hasAttr(OWNED_RESULT_NAMED_ATTR_NAME); } +// May a `return` or a `delete` take the reference over? Nothing reads the value after either. +// +// A folded `const` may be taken over even when already marked consumed: no storing receiver +// takes a named value, so what consumed it can only be another `return` or `delete`, on a path +// that excludes this one - `if (x) return c; return c;`. Retaining here instead would hand the +// caller two references on the second path. +inline bool mayTakeOverReferenceAtLastUse(mlir::Value value) +{ + auto *definingOp = value ? value.getDefiningOp() : nullptr; + return definingOp && definingOp->hasAttr(OWNED_RESULT_ATTR_NAME) && + (definingOp->hasAttr(OWNED_RESULT_NAMED_ATTR_NAME) || + !definingOp->hasAttr(OWNED_RESULT_CONSUMED_ATTR_NAME)); +} + } // namespace typescript #endif // MLIR_TYPESCRIPT_COMMONGENLOGIC_MLIROWNEDREFERENCE_H_ diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index c564dd90b..30124a2d1 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -996,10 +996,10 @@ class MLIRGenImpl } // The same question from a receiver after which nothing reads the value - a `return`, a - // `delete` - which may take over a folded `const`'s reference too. + // `delete` - which may take over a folded `const`'s reference too, on every path. bool producesOwnedReferenceAtLastUse(mlir::Value value) { - return carriesUnclaimedReference(value); + return mayTakeOverReferenceAtLastUse(value); } // Records that a receiver has taken over the reference this value carried, so nothing later diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 740c29955..1ef530a74 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -169,11 +169,12 @@ class OwnedReturnConsumptionPass return; } - if (auto *retain = findReceiverRetain(result)) + auto retains = findReceiverRetains(result); + if (!retains.empty()) { callOp->setAttr(OWNED_RESULT_ATTR_NAME, mlir::UnitAttr::get(&getContext())); callOp->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME, mlir::UnitAttr::get(&getContext())); - toErase.push_back(retain); + toErase.append(retains.begin(), retains.end()); return; } @@ -290,10 +291,11 @@ class OwnedReturnConsumptionPass ifOp->setAttr(OWNED_RESULT_ATTR_NAME, mlir::UnitAttr::get(&getContext())); - if (auto *retain = findReceiverRetain(result)) + auto retains = findReceiverRetains(result); + if (!retains.empty()) { ifOp->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME, mlir::UnitAttr::get(&getContext())); - toErase.push_back(retain); + toErase.append(retains.begin(), retains.end()); } }); @@ -916,33 +918,43 @@ class OwnedReturnConsumptionPass // store, a literal capturing it, a return passing it on), and a `ts.RetainSlot` on the // storage of a local declared from it. // - // Returning null is the ordinary answer for a result nobody took - `f();` on its own, or + // Returning none is the ordinary answer for a result nobody took - `f();` on its own, or // `f().n` - and it is left exactly as it is. Consuming a reference no receiver balances // would free the value while the expression is still using it. // // A folded `const` (OWNED_RESULT_NAMED_ATTR_NAME) is the exception: its name is this very // value, so the one receiver that may take its reference over is a `return`, after which // nothing reads the name. Any other receiver keeps its retain, and the call's +1 is given - // back as a temporary like any other nobody took. - static mlir::Operation *findReceiverRetain(mlir::Value result) + // back as a temporary like any other nobody took. Every `return` of it takes it over, since + // each is on a path of its own - `if (x) return r; return r;` - and one left retaining would + // hand its caller two references. + static llvm::SmallVector findReceiverRetains(mlir::Value result) { auto *definingOp = result.getDefiningOp(); - auto named = definingOp && definingOp->hasAttr(OWNED_RESULT_NAMED_ATTR_NAME); - - for (auto *user : result.getUsers()) + if (definingOp && definingOp->hasAttr(OWNED_RESULT_NAMED_ATTR_NAME)) { - if (auto retainOp = mlir::dyn_cast(user)) + llvm::SmallVector returned; + for (auto *user : result.getUsers()) { - if (retainOp.getReference() == result && (!named || retainFeedsReturn(retainOp, result))) + auto retainOp = mlir::dyn_cast(user); + if (retainOp && retainOp.getReference() == result && retainFeedsReturn(retainOp, result)) { - return retainOp.getOperation(); + returned.push_back(user); } } + + return returned; } - if (named) + for (auto *user : result.getUsers()) { - return nullptr; + if (auto retainOp = mlir::dyn_cast(user)) + { + if (retainOp.getReference() == result) + { + return {retainOp.getOperation()}; + } + } } for (auto *user : result.getUsers()) @@ -961,12 +973,12 @@ class OwnedReturnConsumptionPass // the declaration becomes the acquisition, which is what keeps the // ownership verifier able to pair the release still to come varOp->setAttr(OWNED_LOCAL_CONSUMED_ATTR_NAME, mlir::UnitAttr::get(varOp.getContext())); - return retainSlotOp.getOperation(); + return {retainSlotOp.getOperation()}; } } } - return nullptr; + return {}; } }; diff --git a/tslang/test/tester/tests/00owned_named_consts.ts b/tslang/test/tester/tests/00owned_named_consts.ts index c043d89ea..077895fed 100644 --- a/tslang/test/tester/tests/00owned_named_consts.ts +++ b/tslang/test/tester/tests/00owned_named_consts.ts @@ -183,6 +183,33 @@ function chainedAssignment() { return h.c.v.length + h.c.x; } +// A name returned on two paths. Each `return` is the last use on its own path, so each hands the +// reference over; one that retained instead would give its caller a second reference, a leak no +// assertion here can see - `-mm=own` and the ownership verifier are what notice that. This pins +// that both paths still hand back a live value. +function returnedOnTwoPaths(x: boolean) { + const c = new C(3); + c.v.push(1); + if (x) return c; + return c; +} + +// the same for a named call result, which the pass after MLIRGen settles +function callResultReturnedOnTwoPaths(x: boolean) { + const r = filled(2); + if (x) return r; + return r; +} + +function returns() { + const a = returnedOnTwoPaths(true); + const b = returnedOnTwoPaths(false); + const c = callResultReturnedOnTwoPaths(true); + const d = callResultReturnedOnTwoPaths(false); + churn(); + return a.x + b.x + c.v.length + d.v.length; +} + function main() { assert(innerLetThenName() == 7, "a name outlives an inner `let` of it"); assert(innerLetInLoop() == 8, "a name outlives a `let` of it in a loop"); @@ -192,6 +219,7 @@ function main() { assert(fieldStoreInLoop() == 1, "a string stored into a field in a loop survives the overwrites"); assert(callResultThenName() == 7, "a named call result outlives an inner `let` of it"); assert(chainedAssignment() == 6, "a field and a local assigned one `new` are two holders"); + assert(returns() == 10, "a name returned on either of two paths is handed back alive"); print("done."); } From 99cb62b32cc9004557dee71e6b23448b1462b7f9 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Tue, 29 Sep 2026 11:07:33 +0100 Subject: [PATCH 3/3] A temporary used inside an if, loop or switch body is still released at its block end Section 9.30 released an unclaimed reference only when every use sat in the producer's own block, and a statement body is a region of an op in that block. With named consts now retained by their receivers, `const a = new C(); if (x) { let b = a; }` never gave `a`'s own reference back: 1M calls reached 89.2 MB at -O1 under rc (none 89.2; -O3 hid it by removing the allocation). A use nested in an if, loop, switch, label or try body now counts as in the block, since those bodies finish before the next op runs; a nested function or an async.execute body still does not. After: 4.7 MB. Co-Authored-By: Claude Opus 5.5 --- tslang/docs/reference-counting-evaluation.md | 9 +++++ .../TypeScript/OwnedReturnConsumptionPass.cpp | 36 ++++++++++++++++--- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/tslang/docs/reference-counting-evaluation.md b/tslang/docs/reference-counting-evaluation.md index 51f609b0b..c9351df21 100644 --- a/tslang/docs/reference-counting-evaluation.md +++ b/tslang/docs/reference-counting-evaluation.md @@ -5946,6 +5946,15 @@ Under `-mm=own` the extra pair is a move, not a second owner: one taker plus the release later in the same block is one owner, and the inference pass erases both. A million `const p = make(); let q = p;` iterations peak at 4.2 MB under both `rc` and `own` (`none` 50.3). +**And a leak the fix exposed.** §9.30 gave a temporary back only if every use was in its own +block, and the body of an `if` or a loop is a region of an op in that block, not the block. Once +the `let` in `const a = new C(); if (x) { let b = a; }` retained instead of taking over, nothing +gave `a`'s own reference back: 1M calls of such a function reached 89.2 MB at `-O1` (`none` 89.2, +and `-O3` hid it by removing the allocation). A use nested in an `if`, loop, `switch`, label or +`try` body now counts as in the block (`runsWithinBlock`), since those bodies finish before the +next op runs; a nested function or an `async.execute` body still does not. After: 4.7 MB. It +also releases a `const` that is only *read* in a branch, which leaked before this arc. + **Open, and older than this:** `const c = new C(); if (x) return c; print(c.x);` leaks `c` on the path that does not return. The `return` marks the producer consumed, which is what stops §9.30 releasing it, and that mark is per operation, not per path. diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 1ef530a74..db9c161cd 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -327,7 +327,8 @@ class OwnedReturnConsumptionPass // Two things disqualify a value, and both simply leave it leaking as before: // // - a user outside the producer's block, so the value outlives the block or is used on a - // path this cannot see; + // path this cannot see - the body of a statement written in the block is not outside it + // (runsWithinBlock); // - a user that is a terminator, since a value handed to a successor as a block argument is // still live after the point this would release it; // - a `ts.StateLabel` after the definition, which is a generator's resume point: the state @@ -399,9 +400,10 @@ class OwnedReturnConsumptionPass // the path that falls through (section 9.60). // // Only exits that come after the definition in this block are covered, because only - // those are dominated by it. Nothing else has to be checked: a temporary whose value - // is used from another block is excluded by allUsesReleasableInOwnBlock above, so a - // nested exit cannot be returning this value or reading it. + // those are dominated by it. A nested exit may read the value - a branch that reads a + // `const` and returns - but it cannot return it: a `return` of the value takes it over + // and marks it consumed, which keeps it out of this set. The release goes in front of + // the exit, after everything the branch read. // // A `return` leaves for good wherever it is written. A `break` or `continue` only // sometimes does, and which it is decides between a leak and a double release: @@ -512,14 +514,38 @@ class OwnedReturnConsumptionPass return false; } + // Does `user` run before control reaches the op that follows its ancestor in `block`? True + // for `block`'s own ops, and for ops nested in structured statements only. + static bool runsWithinBlock(mlir::Operation *user, mlir::Block *block) + { + for (auto *op = user; op->getBlock() != block; op = op->getParentOp()) + { + auto *parent = op->getParentOp(); + if (!parent || !mlir::isa(parent)) + { + return false; + } + } + + return true; + } + // Can a release at the end of this value's own block give its reference back safely? See // releaseDiscardedTemporaries for what disqualifies a use and why. + // + // A user inside the body of an `if`, a loop, a `switch`, a label or a `try` written in this + // block is not outside it: those regions run to completion before the op after them does, so + // the end of the block still follows every use. A folded `const` read inside a branch - + // `const a = new C(); if (x) { let b = a; }` - is exactly that, and leaked before this. Any + // other region is: a nested function or an `async.execute` body runs later, from somewhere + // else. static bool allUsesReleasableInOwnBlock(mlir::Operation *op) { auto *block = op->getBlock(); for (auto *user : op->getResult(0).getUsers()) { - if (user->getBlock() != block || user->hasTrait()) + if (user->hasTrait() || !runsWithinBlock(user, block)) { return false; }