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 0e738b468..bf41fff56 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 @@ -4329,3 +4329,45 @@ Not here: shows a use-after-free that only those two tests prevent. - The benchmark keeps its two source edits; the unedited `rtbench.ts` does not compile under own (a non-goal of the plan). + +## 35. A merge of locals made before a loop, bound in it, 2026-10-23 + +`const each: C = i % 2 == 0 ? h : h2` inside a loop, with `h` and `h2` made before it, was +`'this value' is moved inside a loop but was made outside it` for every type (found by #671, whose +`00interface_cast_unrelated` loop case was rewritten around it). The ternary lowers to an +`arith.select` of the two values (or, when a side has a call, to a `cf` branch passing each as a +block argument), and neither was a read: the merge took the value, and `takerLoops` saw the taker +come round without the value made again. + +- **A merge whose value is only read is a borrow of each value it chooses** (`borrowsThroughMerge`, + OwnershipInferenceMoves). `movesOf` walks what the select's result or the block argument leads to + (`walkBorrowed`, which now passes through a select as it does through a branch); when nothing + stores, returns, captures or retains it, and none of it runs after one of the value's releases, + the merge is a read, and so is each use the walk found: a use of the merged value after a move of + `h` is a use after that move. A `let` declared from a merge of values the function made (`const h = + new C()`) borrows it as `let b = a` does: its + reads are walked, its `ts.RetainSlot` and `ts.ReleaseSlot`s go (`mergeBorrowRetains`, which + `decideRetains` then skips). The verdict-2 path (`checkValueMoves`'s borrowing `let`s) leaves such + a merge out of the takers too. +- **Not widened:** a merge whose value is kept is still a taker, decided as before: pushed, stored + into a field, returned or captured by a closure that escapes inside the loop it is still `moved + inside a loop`, and a merged value assigned to an outer `let` and read after one side's release is + still rejected (`takes a second reference`). A `let each` that is assigned again is not a borrower. + +`own_ternary_outer_loop` binds `c ? h : h2` in a loop as a `const` and a `let`, of a class and of an +interface, as a branch (`i % 4 == 0 ? h : (odd(i) ? h2 : h)`), and with one side made in the loop, +churns, and reads `h` and `h2` after the loop; it prints `done.` under own, rc, gc and none, JIT and +AOT, with no counting under own. `00interface_cast_unrelated` gets its loop case back. Negatives +`own_err_ternary_loop_stored` (pushed), `own_err_ternary_loop_returned` (a `let` returned) and +`own_err_ternary_loop_captured` (captured by an escaping closure). Tooth: built without the change, +each of the run test's six functions fails (`moved inside a loop`, and for the `let`s `takes a +second reference`), and so does the interface test's loop. + +Not here: +- `let each = c ? h : h2` where `h` and `h2` are themselves `let`s: the merge is of their reads, which + are not values `movesOf` checks, so `each`'s `ts.RetainSlot` is still `merged from several + branches` (and `moved inside a loop`). A `const each` of the same reads compiled before this section + and still does. +- Under rc a folded `const` that a ternary yields is never given back (`releaseDiscardedTemporaries` + skips a value a `ts.Result` uses), so the class cases leak `h` and `h2` under rc, and under own + too. The interface cases do not: their `ts.New` is used by the view, not by the branch. diff --git a/tslang/lib/TypeScript/OwnershipInferenceBorrowers.cpp b/tslang/lib/TypeScript/OwnershipInferenceBorrowers.cpp index e93ff2f03..0b2d8324b 100644 --- a/tslang/lib/TypeScript/OwnershipInferenceBorrowers.cpp +++ b/tslang/lib/TypeScript/OwnershipInferenceBorrowers.cpp @@ -267,6 +267,13 @@ namespace own_inference return true; } + // one of the values a select chooses between, not its condition + if (auto selectOp = mlir::dyn_cast(user); selectOp && use.get() != selectOp.getCondition()) + { + values.push_back(selectOp.getResult()); + return true; + } + if (auto branchOp = mlir::dyn_cast(user)) { if (auto argument = branchOp.getSuccessorBlockArgument(use.getOperandNumber())) diff --git a/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp b/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp index 7bfcbde41..8f4e74c56 100644 --- a/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp +++ b/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp @@ -503,7 +503,7 @@ namespace own_inference // Not a move. Every taker a `let` that can borrow, and the temporary's own release the // owner that bounds them (spec 2.2, verdict 2) - else the move's error stands. - llvm::SmallVector takers; + llvm::SmallVector> notReads; llvm::SmallVector releases; forEachUse(value, [&](mlir::Operation *user, mlir::Value used) { if (mlir::isa(user)) @@ -517,10 +517,21 @@ namespace own_inference } else if (!isBorrow(user, used)) { - takers.push_back(user); + notReads.push_back({user, used}); } }); + // a merge that only borrows it (movesOf) takes nothing + llvm::SmallVector takers; + for (auto [user, used] : notReads) + { + llvm::SmallVector uses; + if (!borrowsThroughMerge(user, used, value.getDefiningOp(), releases, uses, toErase)) + { + takers.push_back(user); + } + } + if (!releases.empty() && !takers.empty() && llvm::all_of(takers, [](mlir::Operation *taker) { return !!borrowerOf(taker); })) { @@ -565,12 +576,123 @@ namespace own_inference return true; } + llvm::SmallVector OwnershipInferencePass::mergedValuesOf(mlir::Operation *user, mlir::Value used) + { + llvm::SmallVector merged; + if (auto selectOp = mlir::dyn_cast(user)) + { + if (used != selectOp.getCondition()) + { + merged.push_back(selectOp.getResult()); + } + + return merged; + } + + auto branchOp = mlir::dyn_cast(user); + if (!branchOp) + { + return merged; + } + + for (auto &operand : user->getOpOperands()) + { + if (operand.get() != used) + { + continue; + } + + auto argument = branchOp.getSuccessorBlockArgument(operand.getOperandNumber()); + if (!argument) + { + return {}; // the branch's own operand - its condition - is not a merge + } + + merged.push_back(*argument); + } + + return merged; + } + + bool OwnershipInferencePass::borrowsThroughMerge(mlir::Operation *user, mlir::Value used, mlir::Operation *definition, + llvm::ArrayRef releases, + llvm::SmallVectorImpl &uses, + llvm::SetVector &toErase) + { + auto merged = mergedValuesOf(user, used); + if (merged.empty()) + { + return false; + } + + llvm::SmallVector made; + if (definition) + { + made.push_back(definition); + } + + llvm::SmallVector borrowers; + auto isLet = [&](mlir::Operation *varUser, llvm::ArrayRef) { + auto borrower = mlir::isa(varUser) ? borrowerOf(varUser) : mlir_ts::VariableOp(); + if (borrower) + { + borrowers.push_back(borrower); + } + + return !!borrower; + }; + + llvm::SmallVector found; + for (auto value : merged) + { + // a store, a return, a capture or a retain of the merged value keeps it: the merge is a + // move, decided as any other taker + if (walkBorrowed(value, made, found, isLet)) + { + return false; + } + } + + llvm::SmallVector bookkeeping; + for (auto borrower : borrowers) + { + if (collectBorrowerUses(borrower, borrower.getOperation(), found, bookkeeping)) + { + return false; + } + } + + for (auto *release : releases) + { + for (auto &use : found) + { + if (reachableAfter(release, use.op, use.kills)) + { + return false; + } + } + } + + uses.append(found.begin(), found.end()); + for (auto *op : bookkeeping) + { + toErase.insert(op); + if (mlir::isa(op)) + { + mergeBorrowRetains.insert(op); + } + } + + return true; + } + bool OwnershipInferencePass::movesOf(mlir::Value value, llvm::SetVector &toErase) { llvm::SmallVector takers; llvm::SmallVector releases; llvm::SmallVector reads; llvm::DenseMap readOf; + llvm::SmallVector> merges; forEachUse(value, [&](mlir::Operation *user, mlir::Value used) { if (mlir::isa(user)) { @@ -586,6 +708,10 @@ namespace own_inference reads.push_back(user); readOf.try_emplace(user, used); } + else if (!mergedValuesOf(user, used).empty()) + { + merges.push_back({user, used}); + } else { takers.push_back(user); @@ -593,6 +719,26 @@ namespace own_inference }); auto *definition = value.getDefiningOp(); + + // `c ? h : h2` that is only read borrows `h` and `h2`, and in a loop moves neither. What + // reads the merged value reads this one, so each such read can be a use after a move of + // it like any other. + for (auto [merge, used] : merges) + { + llvm::SmallVector uses; + if (!borrowsThroughMerge(merge, used, definition, releases, uses, toErase)) + { + takers.push_back(merge); + continue; + } + + reads.push_back(merge); + for (auto &use : uses) + { + reads.push_back(use.op); + } + } + auto name = describe(value); llvm::SmallVector moved; for (auto *taker : takers) diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index b08000cd3..eb8685567 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -138,6 +138,7 @@ namespace own_inference stateBoxes.clear(); fieldBorrowRetains.clear(); fieldBorrowReleases.clear(); + mergeBorrowRetains.clear(); captureStores.clear(); captureRetains.clear(); returnedParams.clear(); @@ -320,6 +321,11 @@ namespace own_inference continue; } + if (mergeBorrowRetains.contains(op)) + { + continue; // a `let` that borrows a merge: borrowsThroughMerge erased it + } + // a retain of what holds no block takes nothing: a union's number payload made into // another union is a view of the union it came from, but a number all the way if (auto retainOp = mlir::dyn_cast(op); retainOp && holdsNoBlock(retainOp.getReference())) diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.h b/tslang/lib/TypeScript/OwnershipInferencePass.h index 31fd373bb..58f0986a9 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.h +++ b/tslang/lib/TypeScript/OwnershipInferencePass.h @@ -7,6 +7,7 @@ #include "mlir/Pass/Pass.h" #include "mlir/IR/Dominance.h" #include "mlir/Dialect/LLVMIR/LLVMAttrs.h" +#include "mlir/Dialect/Arith/IR/Arith.h" #include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h" #include "mlir/Interfaces/ControlFlowInterfaces.h" #include "mlir/Interfaces/FunctionInterfaces.h" @@ -465,7 +466,8 @@ class OwnershipInferencePass static bool isAssignmentOf(mlir::Operation *op, mlir_ts::VariableOp varOp); // What `use` hands the very value it uses on to, as that value: a cast or a view of it, the - // argument of the block a branch passes it to (a merge: `a ?? b`), or the reads of a local + // argument of the block a branch passes it to (a merge: `a ?? b`) or the `arith.select` that + // chooses it (the merge folded: `c ? h : h2`), or the reads of a local // that owns nothing and is assigned it (`for (x of arr)` into an outer `let x`). Answers // whether the use is one of those, even where nothing comes out of it. Where the value lands // somewhere that is replaced - a block argument, a local - `renewedAt` gets what replaces it: @@ -931,6 +933,23 @@ class OwnershipInferencePass // The move verdict for an SSA value (phase 1). Reports and returns false when it is not one. bool movesOf(mlir::Value value, llvm::SetVector &toErase); + // What a merge of branches makes of `used` (`c ? h : h2`): the result of an `arith.select` it + // is a choice of, or each block argument a branch passes it as. Empty for any other use. + static llvm::SmallVector mergedValuesOf(mlir::Operation *user, mlir::Value used); + + // Is the merge `user` makes of `used` only a borrow of it - everything the merged value leads + // to reads it (walkBorrowed), a `let` declared from it included (`let each = c ? h : h2`: its + // `ts.RetainSlot` and `ts.ReleaseSlot`s go, as a borrowing `let`'s do), and none of that runs + // after one of `releases`, the value's own ends? Each such read joins `uses`. False when + // nothing merges it: the merge then takes the value, as any other use that is not a read does. + bool borrowsThroughMerge(mlir::Operation *user, mlir::Value used, mlir::Operation *definition, + llvm::ArrayRef releases, llvm::SmallVectorImpl &uses, + llvm::SetVector &toErase); + + // the `ts.RetainSlot`s of the `let`s that borrow a merge (borrowsThroughMerge): erased, and + // decided there + llvm::DenseSet mergeBorrowRetains; + // A call that keeps an argument and is given the same value again through another operand - the // value itself, a view of it, something read out of it, another read of its slot (`f(n, n)`, // `f(h, n, n.v)` with only the first `n` kept): the callee takes the value while it borrows it diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index d8aa2aa7f..a495241a4 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -3597,7 +3597,7 @@ foreach(own_test own_fresh_string own_fresh_array own_return_new own_try_local o own_field_chain_let own_read_after_move own_conditional_move own_interface_facts own_declared_scalar_no_drops own_borrowing_record own_borrowing_record_arg - own_reduce own_fresh_call_result) + own_reduce own_fresh_call_result own_ternary_outer_loop) tslang_add_test(NAME test-jit-own-${own_test} COMMAND test-runner -jit -mm=own "${PROJECT_SOURCE_DIR}/test/tester/own/${own_test}.ts") tslang_add_test(NAME test-compile-own-${own_test} COMMAND test-runner -mm=own "${PROJECT_SOURCE_DIR}/test/tester/own/${own_test}.ts") # The promise: no counting at all in what own emits. @@ -3676,6 +3676,13 @@ foreach(conditional_move_mm rc gc none) tslang_add_test(NAME test-compile-${conditional_move_mm}-own_conditional_move COMMAND test-runner -mm=${conditional_move_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/own_conditional_move.ts") endforeach() +# `c ? h : h2` bound in a loop, of locals made before it, borrows the one it chooses (own); the +# same program under every other model. +foreach(ternary_loop_mm rc gc none) + tslang_add_test(NAME test-jit-${ternary_loop_mm}-own_ternary_outer_loop COMMAND test-runner -jit -mm=${ternary_loop_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/own_ternary_outer_loop.ts") + tslang_add_test(NAME test-compile-${ternary_loop_mm}-own_ternary_outer_loop COMMAND test-runner -mm=${ternary_loop_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/own_ternary_outer_loop.ts") +endforeach() + # A call through an interface closed in its module gets its implementations' facts (spec 31); the # same program under every other model. foreach(interface_facts_mm rc gc none) @@ -4165,6 +4172,9 @@ set(own_error_cases "own_err_field_let_global_overwritten|:42:11: error: '[^']*' borrows a field but is used here after it may be released or overwritten" "own_err_conditional_move_read|:20:11: error: '[^']*' is used here after its value was moved" "own_err_conditional_move_outer|:19:13: error: '[^']*' is moved inside a loop but was made outside it" + "own_err_ternary_loop_stored|:12:25: error: '[^']*' is moved inside a loop but was made outside it" + "own_err_ternary_loop_returned|:11:23: error: '[^']*' is moved inside a loop but was made outside it" + "own_err_ternary_loop_captured|:17:31: error: '[^']*' is moved inside a loop but was made outside it" "own_err_conditional_move_read_after_join|:18:9: error: '[^']*' is moved here on some paths only" "own_err_conditional_move_field_borrow|:22:9: error: '[^']*' is moved here on some paths only" "own_err_conditional_move_catch|:24:13: error: '[^']*' is moved here on some paths only" diff --git a/tslang/test/tester/own/own_err_ternary_loop_captured.ts b/tslang/test/tester/own/own_err_ternary_loop_captured.ts new file mode 100644 index 000000000..cd90cc065 --- /dev/null +++ b/tslang/test/tester/own/own_err_ternary_loop_captured.ts @@ -0,0 +1,25 @@ +// -mm=own rejects: `c ? h : h2` inside a loop is a borrow of `h` or `h2` only while it is read; +// captured by a closure that escapes, it would move one of them into another closure each turn. +interface HasDist { + dist: number; +} + +class C implements HasDist { + constructor(public dist: number) {} +} + +let readers: (() => number)[] = []; + +function register() { + const h: HasDist = new C(3); + const h2: HasDist = new C(30); + for (let i = 0; i < 4; i++) { + const each: HasDist = i % 2 == 0 ? h : h2; + readers.push(() => each.dist); + } +} + +function main() { + register(); + print(readers.length); +} diff --git a/tslang/test/tester/own/own_err_ternary_loop_returned.ts b/tslang/test/tester/own/own_err_ternary_loop_returned.ts new file mode 100644 index 000000000..6c075fe61 --- /dev/null +++ b/tslang/test/tester/own/own_err_ternary_loop_returned.ts @@ -0,0 +1,22 @@ +// -mm=own rejects: `c ? h : h2` inside a loop is a borrow of `h` or `h2` only while it is read; +// returned, it would hand the caller a local the function still owns. +class C { + constructor(public dist: number) {} +} + +function pick(n: number) { + const h = new C(3); + const h2 = new C(30); + for (let i = 0; i < 4; i++) { + let each: C = i % 2 == 0 ? h : h2; + if (i == n) { + return each; + } + } + + return new C(0); +} + +function main() { + print(pick(1).dist); +} diff --git a/tslang/test/tester/own/own_err_ternary_loop_stored.ts b/tslang/test/tester/own/own_err_ternary_loop_stored.ts new file mode 100644 index 000000000..a42ff2f53 --- /dev/null +++ b/tslang/test/tester/own/own_err_ternary_loop_stored.ts @@ -0,0 +1,17 @@ +// -mm=own rejects: `c ? h : h2` inside a loop is a borrow of `h` or `h2` only while it is read; +// pushed, it would give one of them a new owner each iteration. +class C { + constructor(public dist: number) {} +} + +function main() { + const h = new C(3); + const h2 = new C(30); + const kept: C[] = []; + for (let i = 0; i < 4; i++) { + const each: C = i % 2 == 0 ? h : h2; + kept.push(each); + } + + print(kept.length); +} diff --git a/tslang/test/tester/own/own_ternary_outer_loop.ts b/tslang/test/tester/own/own_ternary_outer_loop.ts new file mode 100644 index 000000000..d43aefe3a --- /dev/null +++ b/tslang/test/tester/own/own_ternary_outer_loop.ts @@ -0,0 +1,117 @@ +// -mm=own: `c ? h : h2` bound inside a loop, of locals made before it, borrows whichever it +// chooses and moves neither - a `const` or a `let`, of a class or an interface, folded to a select +// or kept as a branch, and with one side made inside the loop. Both stay the function's after the +// loop. +interface HasDist { + dist: number; +} + +class C implements HasDist { + v: number[] = []; + constructor(public dist: number) {} +} + +function churn() { + const junk: C[] = []; + for (let i = 0; i < 64; i++) { + const c = new C(999); + c.v.push(99); + junk.push(c); + } +} + +function constClass() { + const h = new C(3); + const h2 = new C(30); + let sum: number = 0; + for (let i = 0; i < 10; i++) { + const each: C = i % 2 == 0 ? h : h2; + sum += each.dist; + } + + churn(); + return sum + h.dist + h2.dist; +} + +function letClass() { + const h = new C(3); + const h2 = new C(30); + let sum: number = 0; + for (let i = 0; i < 10; i++) { + let each: C = i % 2 == 0 ? h : h2; + sum += each.dist + each.v.length; + } + + churn(); + return sum + h.dist + h2.dist; +} + +function constInterface() { + const h: HasDist = new C(3); + const h2: HasDist = new C(30); + let sum: number = 0; + for (let i = 0; i < 10; i++) { + const each: HasDist = i % 2 == 0 ? h : h2; + sum += each.dist; + } + + churn(); + return sum + h.dist + h2.dist; +} + +function letInterface() { + const h: HasDist = new C(3); + const h2: HasDist = new C(30); + let sum: number = 0; + for (let i = 0; i < 10; i++) { + let each: HasDist = i % 2 == 0 ? h : h2; + sum += each.dist; + } + + churn(); + return sum + h.dist + h2.dist; +} + +let calls: number = 0; +function odd(i: number) { + calls++; + return i % 2 != 0; +} + +// a call on one side keeps the merge a branch +function branched() { + const h = new C(3); + const h2 = new C(30); + let sum: number = 0; + for (let i = 0; i < 10; i++) { + const each: C = i % 4 == 0 ? h : (odd(i) ? h2 : h); + sum += each.dist; + } + + churn(); + return sum + h.dist + h2.dist; +} + +function madeInside() { + const h2: HasDist = new C(30); + let sum: number = 0; + for (let i = 0; i < 10; i++) { + const h: HasDist = new C(i); + const each = i % 2 == 0 ? h : h2; + sum += each.dist; + } + + churn(); + return sum + h2.dist; +} + +function main() { + assert(constClass() == 5 * 3 + 5 * 30 + 33, "a const of two classes"); + assert(letClass() == 5 * 3 + 5 * 30 + 33, "a let of two classes"); + assert(constInterface() == 5 * 3 + 5 * 30 + 33, "a const of two interfaces"); + assert(letInterface() == 5 * 3 + 5 * 30 + 33, "a let of two interfaces"); + assert(branched() == 5 * 3 + 5 * 30 + 33, "a merge kept as a branch"); + assert(calls == 7, "the call on one side"); + assert(madeInside() == (0 + 2 + 4 + 6 + 8) + 5 * 30 + 30, "one side made in the loop"); + print("done."); +} diff --git a/tslang/test/tester/tests/00interface_cast_unrelated.ts b/tslang/test/tester/tests/00interface_cast_unrelated.ts index 12db6b245..58f52c2ba 100644 --- a/tslang/test/tester/tests/00interface_cast_unrelated.ts +++ b/tslang/test/tester/tests/00interface_cast_unrelated.ts @@ -122,9 +122,8 @@ function main() { // casts made again and again share one vtable per content and keep reading right let sum: number = 0; for (let i = 0; i < 1000; i++) { - const even: HasDist = h; - const odd: HasDist = hc; - sum += i % 2 == 0 ? even.dist : odd.dist; + const each: HasDist = i % 2 == 0 ? h : hc; + sum += each.dist; } assert(sum == 500 * 3 + 500 * 30, "casts in a loop");