diff --git a/docs/own-memory-model.md b/docs/own-memory-model.md index 0c000bf3a..db16814b9 100644 --- a/docs/own-memory-model.md +++ b/docs/own-memory-model.md @@ -742,6 +742,9 @@ a reference gives it back on every path out of a function. - Array methods that keep elements of the source in a new array (`filter` over class instances) give each such element a second owner, which is an error by design. Use `Shared` elements, or build the result from copies. +- `sort`, `reverse` and `copyWithin` work on arrays of numbers and strings (a string is copied + where it moves). Over class instances they are an error: a swap reads an element into a local + while its slot is overwritten, which -mm=own cannot follow yet; use `Shared` elements. - Iterator helpers work, the lazy ones (`map`, `filter`, `take`, `drop`) taking their source over as in JavaScript: `const seq = fibonacci().map(f)`. Over an iterator that borrows its container (`s.values()` of a `Map` or a `Set`) a helper is bounded by the container too: 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 708355604..7e98fbe33 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 @@ -1808,6 +1808,9 @@ The error stays where a copy would be wrong: pass told this function's callers it keeps (`__own_params`) or returns a borrow of (`__own_result_borrows`). Facts lost (§15, an exported function) are not such facts. There the callers keep their own, so a copy is right. + A read of the parameter the result borrows that is stored into an element or a field, or that + starts a local which is not returned, is copied all the same (§41): the fact is about what is + returned. - **Not a string** in the sense of §22.1. ### 22.5 What it should accept @@ -4641,3 +4644,34 @@ the statement that made it, churning between steps, under own, rc, gc and none ( DefaultLib: `array_keys`, `array_values` and `array_iterator` pass under own; `iterator(this: T[])` (`[Symbol.iterator]`), which returns `this.values()`, says `@borrows("this")`. Not here: `entries()`, whose `yield [i, this[i]]` puts a borrowed element in a tuple the caller owns. + +## 41. Copies inside a function whose result borrows a parameter, 2026-10-27 + +DefaultLib `array_copyWithin`, `owning_copies`, `array_reverse` and `array_sort`, all over strings or +numbers. + +**`copyWithin` and `reverse`.** Both return `this`, so the signature pass records that the result +borrows `this` (`__own_result_borrows`), and §22.4 kept every string read out of `this` from being +copied: "a fact the callers rely on". A read stored back into an element (`this[t] = this[s]`) then had +no copy, and was "borrows an element and cannot be stored". The fact is about what is returned. So +`copyForRetain` now copies such a read when it goes into an element or a field (a `ts.Store` through +a `ts.ElementRef` or a `ts.PropertyRef`), and starts a `let` from a copy when that local cannot be +what the function returns (`mayBeReturned`: no load of it, nor of a local it is stored into, reaches a +`ts.ReturnInternal`). A read returned - directly, or through a local - stays the borrow. A parameter +the function keeps (`__own_params`) is unchanged. DefaultLib's `reverse` reads the element into a +`let`, as `sort`'s `temp` already did: a `const` borrows the slot the next line overwrites. + +**`sort`.** It assigned its optional `callbackfn` a default comparator. A callback parameter's facts - +what a call of it may release - come from the functions given for it (closed world), and a parameter +that is assigned is one nothing knows: `callbackfn(this[i], this[j])` was a use of the two elements +after a call that may release them. DefaultLib's `sort` now gives a `selectionSort` helper either the +comparator it was given or the default, and never assigns a parameter. No compiler change. + +Not here: `sort` and `reverse` over class instances. A swap holds one element in a local while its +slot is overwritten; for a string the local is a copy, for an object it would be a second owner. That +needs an exchange of two slots the analysis sees as moving nothing. + +`own_copy_within_returned_param`: `this[t] = this[s]` and a swap through a `let` in functions that +return `this`, and a read returned directly and through a local, over strings built at run time, +churning between steps, under own, rc, gc and none; AOT under own, 1000, 200000 and 400000 +iterations peak at 4.0, 4.0 and 4.4 MB (a returned read copied would leak). diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index 635281b7a..013099ca5 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -585,6 +585,56 @@ namespace own_inference return param >= 0 && (llvm::is_contained(ownedParams(getFunction()), param) || param == returnsBorrowOf); } + bool OwnershipInferencePass::mayBeReturned(mlir_ts::VariableOp varOp) + { + llvm::SmallVector slots{varOp.getResult()}; + llvm::SmallPtrSet seen{varOp.getResult()}; + while (!slots.empty()) + { + auto slot = slots.pop_back_val(); + for (auto *user : slot.getUsers()) + { + auto loadOp = mlir::dyn_cast(user); + if (!loadOp) + { + continue; + } + + auto returned = false; + forEachUse(loadOp.getResult(), [&](mlir::Operation *loadUser, mlir::Value used) { + if (mlir::isa(loadUser)) + { + returned = true; + return; + } + + mlir::Value other; + if (auto otherVar = mlir::dyn_cast(loadUser)) + { + other = otherVar.getResult(); + } + else if (auto storeOp = mlir::dyn_cast(loadUser); + storeOp && storeOp.getValue() == used && storeOp.getReference().getDefiningOp()) + { + other = storeOp.getReference(); + } + + if (other && seen.insert(other).second) + { + slots.push_back(other); + } + }); + + if (returned) + { + return true; + } + } + } + + return false; + } + bool OwnershipInferencePass::copyForRetain(mlir::Operation *retain) { // `let x = v`: the local starts from a copy @@ -592,7 +642,16 @@ namespace own_inference { auto varOp = retainSlotOp.getSlot().getDefiningOp(); auto init = varOp ? varOp.getInitializer() : mlir::Value(); - if (!init || !isCopyableString(init.getType()) || borrowsKnownParam(init) || isCellVariable(varOp.getResult())) + if (!init || !isCopyableString(init.getType()) || isCellVariable(varOp.getResult())) + { + return false; + } + + // a read of the parameter the result borrows, into a local that is not returned (a swap's + // `let v = this[i]` in a function that returns `this`): a copy, as any read is + if (borrowsKnownParam(init) && + !(borrowedParam(init) == returnsBorrowOf && + !llvm::is_contained(ownedParams(getFunction()), returnsBorrowOf) && !mayBeReturned(varOp))) { return false; } @@ -611,7 +670,7 @@ namespace own_inference return true; } - if (!isCopyableString(value.getType()) || borrowsKnownParam(value)) + if (!isCopyableString(value.getType())) { return false; } @@ -640,6 +699,20 @@ namespace own_inference return false; } + // A read of a parameter this function owns, or of the one its result borrows, stays what it + // is: the borrow it returns, or the parameter's own. Stored into an element or a field + // (`this[t] = this[s]` in a function that returns `this`) it is a copy, as any read is; not + // into a local, which may be what is returned. + auto storeOp = mlir::dyn_cast(taking->getOwner()); + auto intoPlace = storeOp && storeOp.getValue() == taking->get() && + mlir::isa_and_nonnull(storeOp.getReference().getDefiningOp()); + if (borrowsKnownParam(value) && + !(intoPlace && borrowedParam(value) == returnsBorrowOf && + !llvm::is_contained(ownedParams(getFunction()), returnsBorrowOf))) + { + return false; + } + // only a retain in the use's block: a copy made elsewhere (rc's birth retain, after the // producer) has no owner on a path that does not reach the use; rc retains for the use // beside it too diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.h b/tslang/lib/TypeScript/OwnershipInferencePass.h index 80f43c43d..ec4fb2f27 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.h +++ b/tslang/lib/TypeScript/OwnershipInferencePass.h @@ -172,6 +172,9 @@ class OwnershipInferencePass // without the twinReadAfter fallback: a retain whose value has no use of its own is not copied, // so its error stays. bool copyForRetain(mlir::Operation *retain); + // May what this local holds be what the function returns - read and returned, or handed to + // another local that may be? + bool mayBeReturned(mlir_ts::VariableOp varOp); // A retain of a tuple `ts.CreateTuple` made from borrowed strings and handles (`[s, shared]`): the // strings are copied into it instead, each handle is retained alone, and the retain goes. bool copyIntoTuple(mlir_ts::RetainOp retainOp); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 417995afc..51234eeeb 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -3938,7 +3938,7 @@ foreach(own_test own_fresh_string own_fresh_array own_return_new own_try_local o own_generator_method_this own_string_copy_call own_param_kept_some_paths own_method_callbacks own_loop_read_again own_length_drops_elements own_tuple_param_kept own_for_of_borrowed_iterator own_generator_keeps own_generator_keeps_bounded - own_generator_this_param) + own_generator_this_param own_copy_within_returned_param) 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. @@ -3966,9 +3966,11 @@ foreach(for_of_mm rc gc none) endforeach() # A generator whose `@keeps` makes its state own its arguments (spec 38), and one given a generator -# that borrows, and one over its `this` parameter; the same programs under every other model. +# that borrows, and one over its `this` parameter; strings copied within a parameter the result borrows; +# the same programs under every other model. foreach(keeps_mm rc gc none) - foreach(keeps_test own_generator_keeps own_generator_keeps_bounded own_generator_this_param) + foreach(keeps_test own_generator_keeps own_generator_keeps_bounded own_generator_this_param + own_copy_within_returned_param) tslang_add_test(NAME test-jit-${keeps_mm}-${keeps_test} COMMAND test-runner -jit -mm=${keeps_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/${keeps_test}.ts") tslang_add_test(NAME test-compile-${keeps_mm}-${keeps_test} COMMAND test-runner -mm=${keeps_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/${keeps_test}.ts") endforeach() diff --git a/tslang/test/tester/own/own_copy_within_returned_param.ts b/tslang/test/tester/own/own_copy_within_returned_param.ts new file mode 100644 index 000000000..abe102aef --- /dev/null +++ b/tslang/test/tester/own/own_copy_within_returned_param.ts @@ -0,0 +1,70 @@ +// A function whose result borrows its parameter (`return this`) still copies a string it reads out +// of that parameter when it stores it into an element, as any function does (spec 41): +// `this[t] = this[s]`, and a swap's `let v = this[i]`. What it returns stays the borrow - read and +// returned, directly or through a local - so the caller does not free it twice, and a copy of it +// would leak. Every string is built at run time (a literal is immortal and would hide a missing copy), +// and each case churns between steps. +class C { + v: number[] = []; + constructor(public x: 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 made(n: int): string[] { + const r: string[] = []; + for (let i = 0; i < n; i++) r.push("s" + i); + return r; +} + +function copyWithin(this: string[], target: int, start: int, count: int) { + for (let i = 0; i < count; i++) this[target + i] = this[start + i]; + return this; +} + +function swapEnds(this: string[]) { + let v = this[0]; + this[0] = this[this.length - 1]; + this[this.length - 1] = v; + return this; +} + +function first(this: string[]) { + return this[0]; +} + +function firstOrSecond(this: string[]) { + let x = this[0]; + if (this.length > 9) x = this[1]; + return x; +} + +function main() { + let ok = 0; + for (let i = 0; i < 1000; i++) { + const a = made(4); + const same = a.copyWithin(0, 2, 2); + churn(); + if (same[0] == "s2" && a[1] == "s3" && a[2] == "s2" && a[3] == "s3") ok++; + + const b = made(3); + b.swapEnds(); + churn(); + if (b[0] == "s2" && b[2] == "s0") ok++; + + const f = b.first(); + const g = b.firstOrSecond(); + churn(); + if (f == "s2" && g == "s2") ok++; + } + + assert(ok == 1000 * (3), "ok"); + print("done."); +}