From daa6162b1784c5695f0a45fd9eb68bcaec446aab Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Fri, 2 Oct 2026 19:45:08 +0100 Subject: [PATCH] -mm=own phase 6: named messages for a second reference; nullable views, spreads, declaration retains The catch-all ("takes a second reference; -mm=own cannot prove a move or a borrow here yet") was the first error of 100 corpus files. reportSecondReference now names the shape it is given, from the retained value's root, for all four places that report it: a function returning a borrow of its argument on some paths and another value on others (___unbox, ___cast, user code), a parameter returned or kept where its callers cannot learn the fact, a parameter assigned, a local that owns nothing given a value, a value merged from branches, a global's value, a captured variable's value, and an object literal holding a value it owns. Accepted on the way: - a string or an array widened to a union of it and null or undefined, or narrowed back, is a view; - a view of a value whose type owns no block holds none (a union's number payload made into another union), and a retain of it is erased; - a local's declaration retain of constant data is erased wherever the slot still holds its initializer, whatever is stored later (MLIRGen emits it only at the declaration); - a field of constant data is constant data; - an array a spread builds in a local of its own moves into the owning local it is read into; - an interface's `this` bound for a call of a function-typed field is a borrow (#410's shape). Corpus: 402 -> 416 of 592 (415 with --opt), none lost; the catch-all 100 -> 25. Spec section 21. Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-02-own-phase-6.md | 11 +- .../2026-09-24-own-memory-model-design.md | 145 +++++++++ tslang/lib/TypeScript/OwnershipFacts.h | 70 +++- .../lib/TypeScript/OwnershipInferencePass.cpp | 302 ++++++++++++++++-- tslang/test/tester/CMakeLists.txt | 16 +- .../test/tester/own/own_declared_constant.ts | 31 ++ .../tester/own/own_err_captured_returned.ts | 10 + .../tester/own/own_err_global_let_call.ts | 11 + .../tester/own/own_err_global_returned.ts | 12 + .../tester/own/own_err_local_owns_nothing.ts | 10 + .../test/tester/own/own_err_merged_result.ts | 8 + .../test/tester/own/own_err_mixed_return.ts | 9 + .../test/tester/own/own_err_param_assigned.ts | 13 + .../tester/own/own_err_param_kept_exported.ts | 12 + .../own/own_err_param_returned_exported.ts | 8 + .../own/own_err_record_literal_given.ts | 14 + .../test/tester/own/own_err_unbox_string.ts | 9 + .../tester/own/own_interface_field_call.ts | 27 ++ tslang/test/tester/own/own_nullable_view.ts | 45 +++ tslang/test/tester/own/own_spread_build.ts | 23 ++ 20 files changed, 744 insertions(+), 42 deletions(-) create mode 100644 tslang/test/tester/own/own_declared_constant.ts create mode 100644 tslang/test/tester/own/own_err_captured_returned.ts create mode 100644 tslang/test/tester/own/own_err_global_let_call.ts create mode 100644 tslang/test/tester/own/own_err_global_returned.ts create mode 100644 tslang/test/tester/own/own_err_local_owns_nothing.ts create mode 100644 tslang/test/tester/own/own_err_merged_result.ts create mode 100644 tslang/test/tester/own/own_err_mixed_return.ts create mode 100644 tslang/test/tester/own/own_err_param_assigned.ts create mode 100644 tslang/test/tester/own/own_err_param_kept_exported.ts create mode 100644 tslang/test/tester/own/own_err_param_returned_exported.ts create mode 100644 tslang/test/tester/own/own_err_record_literal_given.ts create mode 100644 tslang/test/tester/own/own_err_unbox_string.ts create mode 100644 tslang/test/tester/own/own_interface_field_call.ts create mode 100644 tslang/test/tester/own/own_nullable_view.ts create mode 100644 tslang/test/tester/own/own_spread_build.ts diff --git a/tslang/docs/superpowers/plans/2026-10-02-own-phase-6.md b/tslang/docs/superpowers/plans/2026-10-02-own-phase-6.md index 0e7f83100..dc7a908d2 100644 --- a/tslang/docs/superpowers/plans/2026-10-02-own-phase-6.md +++ b/tslang/docs/superpowers/plans/2026-10-02-own-phase-6.md @@ -142,7 +142,10 @@ one: ### Task 5: named messages and the specific-rule buckets (the next PR) -- [ ] Named messages for each recognised, unaccepted bucket left after tasks 1-4. -- [ ] Sample the use-after-move (22) and borrows-a-field (21) files with `--di`. A true positive - stays; a false positive becomes a shape. -- [ ] Re-run the report. Spec §20. +- [x] Named messages for each recognised, unaccepted bucket left after tasks 1-4 (spec §21.2). + Accepted on the way (§21.3): nullable views, views of what owns no block, a declaration + retain of constant data, an array a spread builds, an interface field's function called. +- [x] Sample the use-after-move (22) and borrows-a-field (21) files with `--di`. A true positive + stays; a false positive becomes a shape. One false positive, fixed; two model questions + (§21.5). +- [x] Re-run the report. Spec §21: 402 -> 416 (415 with `--opt`), catch-all 100 -> 25. 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 0331ca121..510832239 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 @@ -1576,3 +1576,148 @@ of the use-after-move and borrows-a-field files for false positives. `let m = n; n = null; use(m)`, with `n: number | null`. - **Nested record literals, and arrays of record literals,** are the catch-all. - **A conditional's result** is the catch-all. + +## 21. Phase 6 results, 2026-10-02 (second PR): named messages + +Plan: `docs/superpowers/plans/2026-10-02-own-phase-6.md`, task 5. Changed: `OwnershipFacts.h`, +`OwnershipInferencePass.cpp`. Nothing outside own's two passes changes, so rc, gc and none emit +what they did. + +### 21.1 The report + +The same tooling as §20.1, rebuilt: the corpus script (now also run with the runner's +`--opt --opt_level=3`), and the `OWN_SHAPES` dump, applied only inside the catch-all branch. + +On cd7c2840 (#445 merged), 402 of 592 compile, plain and with `--opt`. The catch-all is the first +error of 100 files. After this PR, 416 compile plain and 415 with `--opt`, none lost either way: + +| first error | before | after | +|---|---|---| +| `takes a second reference` (the catch-all) | 100 | 25 | +| used after its value was moved | 24 | 23 | +| borrows a field and cannot be stored, returned or captured | 21 | 20 | +| borrows an element and cannot be stored, returned or captured | 10 | 10 | + +The 25 left are a long tail: no shape is the only blocker of more than three files. Phase 6's exit +criterion (every bucket that is the only blocker of six or more files is accepted or named) holds. + +The 13 files gained under `--opt` each pass test-runner `-mm=own`, AOT and JIT, with the runner's +default flags and with `-noopt` (`--di --opt_level=0`). `00interface_object3` compiles plain only +(§21.6), so it is not counted. + +### 21.2 Named messages + +`reportSecondReference` now classifies what it is given, from the retained value's root, in one +place, so all four places that report it get the same names. Each message has a negative test. + +| shape | message | test | +|---|---|---| +| a function returns a borrow of a parameter on one path and something else on another | `'f' returns a borrow of its argument on some paths and another value on others; -mm=own cannot tell its callers whether they own the result`, with a note at the other return | `own_err_mixed_return`, `own_err_unbox_string` | +| a parameter's value, returned, where the callers cannot learn the fact (§15) | `'x' is a parameter, returned here, but -mm=own cannot tell the callers of 'f' that the result borrows it`, with §15's facts-lost note | `own_err_param_returned_exported` | +| a parameter's value, kept (stored, pushed, given away) | `'x' is a parameter, kept here, but -mm=own cannot make the callers of 'f' give it up`, with the note | `own_err_param_kept_exported` | +| a store into a parameter's slot | `'x' is a parameter and cannot be assigned under -mm=own yet` | `own_err_param_assigned` | +| a store into a local that owns nothing | `'x' owns nothing under -mm=own (it is declared without a value, or in a catch or finally clause), so it cannot be given a new value here` | `own_err_local_owns_nothing` | +| a value merged from branches or loop iterations (a conditional, `&&`, `\|\|`, `??`, a loop) | 'x' is merged from several branches (the operators, a loop); -mm=own cannot prove which one owns it yet | `own_err_merged_result` | +| a global's value, stored, returned or boxed | `'x' borrows the global 'g' and cannot be stored, returned or captured` | `own_err_global_returned` | +| a global's value read into a local | `'x' borrows the global 'g', but -mm=own cannot prove the global keeps its value while 'x' is used: a call or an assignment may overwrite it` | `own_err_global_let_call` | +| a captured variable's value, through its cell | `'x' borrows a captured variable and cannot be stored, returned or captured` | `own_err_captured_returned` | +| an object literal holding a value it owns, given anywhere but an owning local or the result | `an object literal that holds a value it owns is moved only into a local or returned as it is; -mm=own cannot move it here yet` | `own_err_record_literal_given` | + +The mixed-return message is the largest bucket (17 files). Most are two synthesized helpers: +- `___unbox` (`` of an `any`) returns the box's own string when it holds one, and a + string made from its number, boolean or bigint otherwise; +- `___cast` of a union does the same with the union's payload. + +The others are user functions such as `return name` on one path and `return "Sorry, " + name` on +another. + +The parameter messages name the parameter from its slot, which carries a name under `--di` only, +like the other messages (`test-own-err-names-the-variable`). + +### 21.3 What this PR accepts + +- **A string or an array widened to a union of it and null or undefined, or narrowed back** + (`isView`). Such a cast is the same block: the lowering passes the pointer through, and an + optional array narrowed out of its optional is the array or an empty one. A union with other + members is not a view, since it can convert on the way out (a number printed into a string). +- **A view of a value that owns no block holds none** (`holdsNoBlock`). A union's number payload + made into another union is a view, by `rootOf`, of the union it came from, but it is a number + all the way. A `value_ref` is not taken as such a type: by type it owns nothing, but one + `ts.New` made is the object (§20.2). A `ts.Retain` of such a value is erased. Without this, the + number path of `widen(u: number | string): boolean | number | string` left its retain behind + for the lowering's guard: a loud failure, not a hole. + - The signature pass reads it too. A function that returns a number widened into a union + (`mkN(v: number): number | P { return v; }`) no longer has a result that "borrows" its + argument, so `00union_global_init` stops at a later error, a global boxed into `any`. +- **A local's declaration retain of constant data, whatever is stored later** + (`slotHoldsInitializerAt`, replacing §20.2's "nothing is ever stored into the slot"). + - MLIRGen emits `ts.RetainSlot` only at a declaration (`takeOwnershipOfLocal`), right after the + initializer. The rule checks that nothing between the two can write the slot: in their block, + no op is given the slot, no call runs, and no op has regions. + - So the retain takes only the initializer, and each later store is decided on its own. + - `let y: number | null = null; y = 0` compiles now. +- **A field of constant data is constant data** (`isConstantData`): a tuple of literals re-made as + a named tuple, `let pair: [name: string, age: number] = ["user", 10]`. +- **An array a spread builds** (`isBuiltArrayRead`). `const b = [...a, ...a]`: + - MLIRGen makes a fresh array in a local of its own that owns nothing, pushes into it, and reads + it once, when it is done, into the owning local it is made for. + - The fresh array moves into the builder and then into that local, whose `ts.RetainSlot` is the + reference. + - The builder may only be filled (`push`, `unshift`), and only before the read. +- **A function-typed field of an interface, called** (`isBoundForCall`). `i.toString()` with + `toString: () => string` binds `ts.ExtractInterfaceThis(i)` to the function read out of the field + and splits the bound function straight back for the call. It is #410's shape with the interface's + `this` in place of a cast to `opaque`: the call borrows the object. + +Two rules were tried and dropped, because no corpus file and no program written for them needed +them: `ts.SafeCast` as a view, and a number cast to a union as holding no block. + +### 21.4 Tests and teeth + +- **Positive:** `own_nullable_view`, `own_declared_constant`, `own_spread_build`, + `own_interface_field_call`. Each passes test-runner under own, rc, none and gc, AOT and JIT, and + under own with `-noopt`. +- **Negative:** the eleven in the table of §21.2. + +Teeth, each a temporary switch: the positive stops compiling, plain, with `--opt` and with `--di`. + +| rule turned off | fails | +|---|---| +| the nullable view | `own_nullable_view` | +| a view of a value that owns no block | `own_nullable_view` | +| the erase of a retain of such a value | `own_nullable_view` | +| a field of constant data | `own_declared_constant` | +| the declaration retain | `own_declared_constant`, `own_constant_data` | +| the spread builder | `own_spread_build` | +| the interface's `this` bound for a call | `own_interface_field_call` | + +### 21.5 Sampled: use-after-move (24) and borrows-a-field (21) + +Both buckets were read with `--di`. + +- **One false positive, fixed:** the interface field call of §21.3 (`00interface_function_typed_field`). +- **The rest are true positives of single ownership.** Two shapes cover most of them, and each is a + question for the model, not a fix: + - **A value moved into a place that still holds it, then read**, in about 14 of the 24 + use-after-move files. Examples: `h.item = kept; return kept.n`, `(x = f()) !== null`, + `(x += "ba") == ...`, `constructor(public what: string) { print(what) }`. The read could be a + borrow of the destination while nothing overwrites or releases it. + - **A string stored or returned as a second owner**, in most of the 21 borrows-a-field files. + Examples: `testrec.str = testrec.str2`, `return held.text`, `caught = e.m`. Strings are + immutable, so a second reference could be a copy. +- **A value given to an `any` parameter moves into its box** (`isA(union)` in `00union_to_any`). The + box owns what it holds (§15), so the use after the call is a true use after a move. A box that + borrows, for a callee that does not keep it, would accept it. + +### 21.6 Known limits + +- **A synthesized helper's error** (`___unbox`, `___cast<...>`) is reported at the + location of the first use that instantiated it, which may be a comment line at the top of the + file. +- **An interface's `this` bound for a call is decided per read.** Under `--opt`, CSE merges the + reads of one interface value, and a read that is also stored (`const mt: () => void = a.print`) + makes the merged one a taker. `00interface_object3` compiles plain and fails with `--opt` and + `--di`. +- **`instanceof` of a class value held in a union or a nullable local** is false under every + model, even narrowed (`let a: C | null = new C(); a instanceof C`). It is a frontend or lowering + bug outside this phase. `own_declared_constant` tests the value with `typeof`. diff --git a/tslang/lib/TypeScript/OwnershipFacts.h b/tslang/lib/TypeScript/OwnershipFacts.h index d86d1ebfc..d05e0438f 100644 --- a/tslang/lib/TypeScript/OwnershipFacts.h +++ b/tslang/lib/TypeScript/OwnershipFacts.h @@ -139,6 +139,30 @@ inline int resultBorrows(mlir::Operation *op) return -1; } +// Is `narrow` a member of the union or optional `wide` as it is, so that a cast between them keeps +// its value? With `onlyNullable`, every other member is null or undefined. +inline bool holdsAsIs(mlir::Type wide, mlir::Type narrow, bool onlyNullable = false) +{ + llvm::SmallVector members; + if (auto optionalType = mlir::dyn_cast(wide)) + { + members.push_back(optionalType.getElementType()); + } + else if (auto unionType = mlir::dyn_cast(wide)) + { + members.append(unionType.getTypes().begin(), unionType.getTypes().end()); + } + else + { + return false; + } + + return llvm::is_contained(members, narrow) && + (!onlyNullable || llvm::all_of(members, [&](mlir::Type member) { + return member == narrow || mlir::isa(member); + })); +} + // An op whose result is the very block its operand holds: a class widened to a union or an // optional or narrowed back, a union made from its payload or read back from it, an object seen // through an interface (`{vtable, this}`, released through `this`). rc's retain or release of @@ -171,8 +195,17 @@ inline bool isView(mlir::Operation *op) return mlir::isa(castOp.getIn().getType()) && mlir::isa(castOp.getType()); }; - return (keeps(castOp.getIn().getType()) && keeps(castOp.getType())) || - (isClosure(castOp.getIn().getType()) && isClosure(castOp.getType())) || isMadeObject(); + // a string or an array widened to a union that holds it as it is (`string | null`), or narrowed + // back from one whose other members are only null or undefined. A union with other members can + // convert on the way out (a number printed into a string), and an array of another element type + // is a new array. + auto isStringOrArray = [](mlir::Type type) { return mlir::isa(type); }; + auto in = castOp.getIn().getType(); + auto out = castOp.getType(); + auto isNullableView = (isStringOrArray(in) && holdsAsIs(out, in, false)) || + (isStringOrArray(out) && holdsAsIs(in, out, true)); + + return (keeps(in) && keeps(out)) || (isClosure(in) && isClosure(out)) || isMadeObject() || isNullableView; } // The block a value is a view of. @@ -368,10 +401,36 @@ inline bool ownsHeap(mlir::Value value) } // Holds no block: a number, `null` or `undefined` widened, an empty optional, a string literal. +// Also a view of a value whose type owns none: a union's number payload made into another union +// (`GetValueFromUnion`, then `CreateUnionInstance`) is a number all the way. A `value_ref` is not +// such a type: it owns nothing by type, but one `ts.New` made is the object its view shows. inline bool holdsNoBlock(mlir::Value value) { - auto *def = rootOf(value).getDefiningOp(); - return !ownsHeap(value) || (def && isLiteral(def)); + if (!ownsHeap(value)) + { + return true; + } + + for (auto *def = value.getDefiningOp(); def; def = def->getOperand(0).getDefiningOp()) + { + if (isLiteral(def)) + { + return true; + } + + if (!isView(def)) + { + return false; + } + + auto in = def->getOperand(0); + if (!ownsHeap(in) && !mlir::isa(in.getType())) + { + return true; + } + } + + return false; } // Some view of `value` is a `ts.NewInterface`, which boxes what it shows into a block. @@ -433,9 +492,10 @@ inline bool isConstantData(mlir::Value value) return true; } + // a field of such data: of a `ts.Constant`, or of a tuple of literals re-made as a named one if (auto extractOp = mlir::dyn_cast(def)) { - return extractOp.getObject().getDefiningOp() != nullptr; + return isConstantData(extractOp.getObject()); } if (mlir::isa(def)) diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index 56713f0dc..4476dc0b0 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -215,6 +215,14 @@ class OwnershipInferencePass continue; } + // 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())) + { + toErase.insert(op); + continue; + } + auto value = retainedValue(op); if (value && proven.contains(value)) { @@ -265,8 +273,15 @@ class OwnershipInferencePass // data nothing owns (isConstantData): a number or null in a union, a function, a // literal, a tuple of literals. A slot's retain is decided on its initializer, so only - // while nothing else is ever stored into the slot. - if (value && isConstantData(value) && (mlir::isa(op) || slotKeepsInitializer(op))) + // where the slot still holds it. + if (value && isConstantData(value) && (mlir::isa(op) || slotHoldsInitializerAt(op))) + { + toErase.insert(op); + continue; + } + + // an array a spread builds, read into the owning local it is made for + if (value && mlir::isa(op) && isBuiltArrayRead(op, value)) { toErase.insert(op); continue; @@ -2743,38 +2758,39 @@ class OwnershipInferencePass return name; } - // The value a retain acquires: a Retain's operand, or a RetainSlot's variable's initializer, - // seen through its views. None for a variable with no initializer - its storage was hoisted - // in front of a try and its value arrives by a store this phase does not follow. - // A `ts.RetainSlot`'s slot holds its initializer for as long as it lives: nothing is stored - // into it or into a field of it, and no closure captures it. - static bool slotKeepsInitializer(mlir::Operation *op) + // A `ts.RetainSlot`'s slot holds its initializer where the retain is. MLIRGen puts the retain at + // the declaration (takeOwnershipOfLocal), and nothing between the two may write the slot: in + // their block, no op is given the slot, no call runs, and no op has regions (a store could hide + // in them). What is stored into the slot later is decided at its own store; this retain takes + // only the initializer. No closure captures it. + static bool slotHoldsInitializerAt(mlir::Operation *op) { auto slot = mlir::cast(op).getSlot(); auto varOp = slot.getDefiningOp(); - if (!varOp || !varOp.getInitializer() || isCellVariable(slot)) + if (!varOp || !varOp.getInitializer() || isCellVariable(slot) || varOp->getBlock() != op->getBlock()) { return false; } - return llvm::all_of(slot.getUsers(), [&](mlir::Operation *user) { - return mlir::isa(user) || onlyRead(user); - }); - } - - // A read of what a slot holds, or of a field of it (`a[0]` of a tuple local), and nothing that - // writes through it. - static bool onlyRead(mlir::Operation *user) - { - if (mlir::isa(user)) + for (auto *between = varOp->getNextNode(); between; between = between->getNextNode()) { - return true; + if (between == op) + { + return true; + } + + if (llvm::is_contained(between->getOperands(), slot) || isCall(between) || between->getNumRegions() > 0) + { + return false; + } } - auto propertyRefOp = mlir::dyn_cast(user); - return propertyRefOp && llvm::all_of(propertyRefOp->getUsers(), [](mlir::Operation *fieldUser) { return onlyRead(fieldUser); }); + return false; } + // The value a retain acquires: a Retain's operand, or a RetainSlot's variable's initializer, + // seen through its views. None for a variable with no initializer - its storage was hoisted + // in front of a try and its value arrives by a store this phase does not follow. static mlir::Value retainedValue(mlir::Operation *op) { if (auto retainOp = mlir::dyn_cast(op)) @@ -2949,6 +2965,35 @@ class OwnershipInferencePass return retain; } + // `const b = [...a, ...a]`: MLIRGen builds the array in a local of its own that owns nothing, + // starting from a fresh one and pushing into it, and reads it once, when it is done, into the + // owning local it is made for. That local's `ts.RetainSlot` (`retain`) is the reference: the + // fresh array moves into the builder and then into the owner, and nothing else holds it. + bool isBuiltArrayRead(mlir::Operation *retain, mlir::Value value) + { + auto read = value.getDefiningOp(); + auto builder = read ? read.getReference().getDefiningOp() : mlir_ts::VariableOp(); + auto owner = mlir::cast(retain).getSlot().getDefiningOp(); + if (!builder || isOwningVariable(builder) || builder.getCaptured().value_or(false) || !builder.getInitializer() || + !mlir::isa_and_nonnull(builder.getInitializer().getDefiningOp()) || + !owner || owner.getInitializer() != read.getResult() || !read->hasOneUse() || onlyRetainSlotOf(owner) != retain) + { + return false; + } + + // only filled, and only before it is read + return llvm::all_of(builder->getUses(), [&](mlir::OpOperand &use) { + auto *user = use.getOwner(); + if (user == read) + { + return true; + } + + return mlir::isa(user) && use.getOperandNumber() == 0 && + !reachableAfter(read, user, builder); + }); + } + // The one `ts.RetainSlot` of a local; null when there are none or several. static mlir::Operation *onlyRetainSlotOf(mlir_ts::VariableOp varOp) { @@ -3046,6 +3091,11 @@ class OwnershipInferencePass isBoundForCall(castOp); } + if (mlir::isa(user)) + { + return isBoundForCall(user); + } + // arguments are borrowed, but one the callee keeps (`__own_params`) is taken; a callee // with no facts that keeps one retains it, and that retain is its own error if (isCall(user)) @@ -3096,15 +3146,18 @@ class OwnershipInferencePass // `ts.GetMethod` for a call: a method of another module's class - its constructor, when it is // built with `new` - called on an object here. It reads the object as `ts.ThisSymbolRef` does // for a method of this module; nothing else may use the cast or the bound function, and the - // object comes out again only as a call's argument, which is a borrow of its own. - static bool isBoundForCall(mlir_ts::CastOp castOp) + // object comes out again only as a call's argument, which is a borrow of its own. An + // interface's `this` (`ts.ExtractInterfaceThis`) is bound so for a call of a function-typed + // field (`i.toString()` where `toString: () => string`). + static bool isBoundForCall(mlir::Operation *thisOp) { - if (!mlir::isa(castOp.getType()) || castOp->use_empty()) + if (thisOp->getNumResults() != 1 || !mlir::isa(thisOp->getResult(0).getType()) || + thisOp->use_empty()) { return false; } - return llvm::all_of(castOp->getUses(), [](mlir::OpOperand &use) { + return llvm::all_of(thisOp->getUses(), [](mlir::OpOperand &use) { auto boundOp = mlir::dyn_cast(use.getOwner()); if (!boundOp || boundOp.getThisVal() != use.get()) { @@ -3222,7 +3275,7 @@ class OwnershipInferencePass { // the view the second release gives back, where rc took its reference auto *view = second->getOperand(0).getDefiningOp(); - reportSecondReference(view ? view : second); + reportSecondReference(view ? view : second, value); return false; } } @@ -3284,7 +3337,7 @@ class OwnershipInferencePass if (!takerAcquires(taker, value)) { - reportSecondReference(taker); + reportSecondReference(taker, value); return false; } @@ -3719,7 +3772,10 @@ class OwnershipInferencePass return name; } - void reportSecondReference(mlir::Operation *op) + // A second reference nobody proved safe. `value` is the value it is to, when the caller knows + // it; a retain names its own. The shapes this pass recognises get the rule they break (spec + // 21.2); anything else, the catch-all. + void reportSecondReference(mlir::Operation *op, mlir::Value value = {}) { if (quiet) { @@ -3739,11 +3795,195 @@ class OwnershipInferencePass name = varName(varOp); } - auto diag = op->emitError("'") << name << "' takes a second reference; -mm=own cannot prove a move or a borrow here yet"; - noteLostFacts(diag); + if (!value && mlir::isa(op)) + { + value = retainedValue(op); + } + + if (!value || !reportShapeOfSecondReference(op, rootOf(value), name)) + { + auto diag = op->emitError("'") << name << "' takes a second reference; -mm=own cannot prove a move or a borrow here yet"; + noteLostFacts(diag); + } + signalPassFailure(); } + // The named shapes of a second reference to `root` (spec 21.2). False when it is none of them. + bool reportShapeOfSecondReference(mlir::Operation *op, mlir::Value root, llvm::StringRef name) + { + auto f = getFunction(); + + // returned, where another return gives a borrow of an argument and this one does not, or + // the other way round: the callers cannot be told which they get + if (isReturned(root)) + { + auto param = borrowedParam(root); + if (auto *other = otherReturn(param)) + { + auto diag = op->emitError("'") << f.getName() + << "' returns a borrow of its argument on some paths and another value on " + "others; -mm=own cannot tell its callers whether they own the result"; + diag.attachNote(other->getLoc()) + << (param >= 0 ? "returns another value here" : "returns a borrow of its argument here"); + return true; + } + } + + // a parameter's value, or a part of it, returned or kept: a fact its callers must know + // (spec 15), and noteLostFacts says why they cannot + if (auto param = borrowedParam(root); param >= 0) + { + auto diag = op->emitError("'") << (name == "this value" ? paramName(param) : name); + if (isReturned(root)) + { + diag << "' is a parameter, returned here, but -mm=own cannot tell the callers of '" << f.getName() + << "' that the result borrows it"; + } + else + { + diag << "' is a parameter, kept here, but -mm=own cannot make the callers of '" << f.getName() + << "' give it up"; + } + + noteLostFacts(diag); + return true; + } + + // a store into a local whose value is not this function's to replace: a parameter's slot, or + // a local that owns nothing (MLIRGen's takeOwnershipOfLocal) + auto storeOp = mlir::dyn_cast(op); + if (auto slot = storeOp ? storeOp.getReference().getDefiningOp() : mlir_ts::VariableOp(); + slot && !isOwningVariable(slot) && !isResultSlot(slot)) + { + // a parameter's slot, assigned (isParameterSlot is the unassigned one) + auto argument = mlir::dyn_cast_or_null(slot.getInitializer()); + if (argument && argument.getOwner()->isEntryBlock() && + mlir::isa(argument.getOwner()->getParentOp())) + { + op->emitError("'") << varName(slot) << "' is a parameter and cannot be assigned under -mm=own yet"; + return true; + } + + // MLIRGen makes no owner of a local declared without a value, nor of one declared in a + // catch or finally clause, whose release would be a call inside an exception funclet + if (!slot.getInitializer()) + { + op->emitError("'") << varName(slot) + << "' owns nothing under -mm=own (it is declared without a value, or in a catch or " + "finally clause), so it cannot be given a new value here"; + return true; + } + } + + // merged from branches or loop iterations + if (auto argument = mlir::dyn_cast(root); argument && !argument.getOwner()->isEntryBlock()) + { + op->emitError("'") << name + << "' is merged from several branches (`?:`, `&&`, `||`, `??`, a loop); -mm=own cannot " + "prove which one owns it yet"; + return true; + } + + auto loadOp = root.getDefiningOp(); + + // a read of a global + if (auto addressOfOp = loadOp ? loadOp.getReference().getDefiningOp() : mlir_ts::AddressOfOp()) + { + // a local that reads it: phase 3's borrow, which a call or an assignment of the global + // may end + if (mlir::isa(op)) + { + op->emitError("'") << name << "' borrows the global '" << addressOfOp.getGlobalName() + << "', but -mm=own cannot prove the global keeps its value while '" << name + << "' is used: a call or an assignment may overwrite it"; + return true; + } + + op->emitError("'") << name << "' borrows the global '" << addressOfOp.getGlobalName() + << "' and cannot be stored, returned or captured"; + return true; + } + + // a read of a captured variable, through its cell + if (loadOp && isLoadedCell(loadOp.getReference())) + { + op->emitError("'") << name << "' borrows a captured variable and cannot be stored, returned or captured"; + return true; + } + + // an object literal built in a local, holding a value it owns: moved only into an owning + // local or the result (recordRetainOf) + auto record = loadOp ? loadOp.getReference().getDefiningOp() : mlir_ts::VariableOp(); + if (!record && storeOp) + { + auto propertyRefOp = storeOp.getReference().getDefiningOp(); + record = propertyRefOp ? propertyRefOp->getOperand(0).getDefiningOp() : mlir_ts::VariableOp(); + } + + if (record && !isOwningVariable(record) && record.getInitializer() && + record.getInitializer().getDefiningOp()) + { + op->emitError("an object literal that holds a value it owns is moved only into a local or " + "returned as it is; -mm=own cannot move it here yet"); + return true; + } + + return false; + } + + // The name of parameter `index`, as its slot carries it (under `--di`). + llvm::StringRef paramName(int index) + { + llvm::StringRef name = "this value"; + getFunction()->walk([&](mlir_ts::VariableOp varOp) { + auto argument = mlir::dyn_cast_or_null(varOp.getInitializer()); + if (argument && argument.getOwner()->isEntryBlock() && static_cast(argument.getArgNumber()) == index) + { + name = varName(varOp); + } + }); + + return name; + } + + // Is `root`, or a view of it, stored into the result slot? + static bool isReturned(mlir::Value root) + { + auto returned = false; + forEachUse(root, [&](mlir::Operation *user, mlir::Value used) { + auto storeOp = mlir::dyn_cast(user); + auto slot = storeOp && storeOp.getValue() == used ? storeOp.getReference().getDefiningOp() + : mlir_ts::VariableOp(); + returned = returned || (slot && isResultSlot(slot)); + }); + + return returned; + } + + // A return that disagrees with one of a borrow of parameter `param` (with -1, with one that + // borrows nothing): a store into the result slot of a value that holds a block and borrows + // another parameter or none - or, for -1, borrows a parameter. Null when there is none. + mlir::Operation *otherReturn(int param) + { + mlir::Operation *found = nullptr; + getFunction()->walk([&](mlir_ts::StoreOp storeOp) { + auto slot = storeOp.getReference().getDefiningOp(); + if (found || !slot || !isResultSlot(slot) || holdsNoBlock(storeOp.getValue())) + { + return; + } + + auto borrows = borrowedParam(storeOp.getValue()); + if (param >= 0 ? borrows != param : borrows >= 0) + { + found = storeOp; + } + }); + + return found; + } + // Where the body would have been fine had its callers known a fact about it, say why they // cannot. void noteLostFacts(mlir::InFlightDiagnostic &diag) diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index f7327154a..100a7927a 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -2568,7 +2568,8 @@ foreach(own_test own_fresh_string own_fresh_array own_return_new own_try_local o own_string_length_borrow own_closure_borrow own_closure_escape own_generator own_interface_view own_async own_record_return own_generator_borrow own_global_script - own_global_init own_constant_data own_record_local) + own_global_init own_constant_data own_record_local + own_nullable_view own_declared_constant own_spread_build own_interface_field_call) 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. @@ -2695,7 +2696,18 @@ set(own_error_cases "own_err_param_kept_some_paths|is moved here on some paths only" "own_err_param_kept_loop|is moved inside a loop" "own_err_param_kept_not_owned|is given to 'keep', which keeps it, but -mm=own cannot move it here" - "own_err_delete|'delete' is not supported by -mm=own yet") + "own_err_delete|'delete' is not supported by -mm=own yet" + "own_err_mixed_return|'greet' returns a borrow of its argument on some paths and another value on others" + "own_err_unbox_string|'___unbox' returns a borrow of its argument on some paths and another value on others" + "own_err_param_returned_exported|is a parameter, returned here, but -mm=own cannot tell the callers of 'identity' that the result borrows it" + "own_err_param_kept_exported|is a parameter, kept here, but -mm=own cannot make the callers of 'Animal.constructor' give it up" + "own_err_param_assigned|is a parameter and cannot be assigned under -mm=own yet" + "own_err_local_owns_nothing|owns nothing under -mm=own .* so it cannot be given a new value here" + "own_err_merged_result|is merged from several branches" + "own_err_global_returned|borrows the global 'V.def' and cannot be stored, returned or captured" + "own_err_global_let_call|borrows the global 'names', but -mm=own cannot prove the global keeps its value" + "own_err_captured_returned|borrows a captured variable and cannot be stored, returned or captured" + "own_err_record_literal_given|an object literal that holds a value it owns is moved only into a local or returned as it is") foreach(own_error_case ${own_error_cases}) string(FIND "${own_error_case}" "|" own_error_sep) string(SUBSTRING "${own_error_case}" 0 ${own_error_sep} own_error_name) diff --git a/tslang/test/tester/own/own_declared_constant.ts b/tslang/test/tester/own/own_declared_constant.ts new file mode 100644 index 000000000..48928d23f --- /dev/null +++ b/tslang/test/tester/own/own_declared_constant.ts @@ -0,0 +1,31 @@ +// A local's retain at its declaration takes its initializer, whatever is stored into it later: +// MLIRGen puts the `ts.RetainSlot` right after the declaration, before anything else can write the +// slot. Where the initializer is data nothing owns - null in a union, a tuple of literals - the +// retain takes nothing, and each later store is decided on its own. +class C { + constructor(public x: number) {} +} + +function main() { + let y: number | null = null; + assert((y ?? 8) == 8); + y = 0; + assert((y ?? 9) == 0); + + let u: string | number | null = null; + u = "s"; + assert(typeof u == "string"); + u = 3; + assert(typeof u == "number"); + + let c: C | number | null = null; + c = new C(4); + assert(c !== null && typeof c != "number"); + + let pair: [name: string, age: number] = ["user", 10.0]; + assert(pair.name == "user" && pair.age == 10.0); + pair.name = "other"; + assert(pair.name == "other"); + + print("done."); +} diff --git a/tslang/test/tester/own/own_err_captured_returned.ts b/tslang/test/tester/own/own_err_captured_returned.ts new file mode 100644 index 000000000..8c25200c1 --- /dev/null +++ b/tslang/test/tester/own/own_err_captured_returned.ts @@ -0,0 +1,10 @@ +// -mm=own rejects: `getarr` returns the value of a variable it captures, which the enclosing +// function still owns. +function main() { + let arr = [1]; + function getarr() { + return arr; + } + getarr()[0]++; + print(arr[0]); +} diff --git a/tslang/test/tester/own/own_err_global_let_call.ts b/tslang/test/tester/own/own_err_global_let_call.ts new file mode 100644 index 000000000..92bf22460 --- /dev/null +++ b/tslang/test/tester/own/own_err_global_let_call.ts @@ -0,0 +1,11 @@ +// -mm=own rejects: `local` borrows the global's value, but a call may store another value into +// the global - and destroy the one `local` reads - while `local` is still used. +let names: string[] = ["a", "b"]; +function churn() { + names = ["c"]; +} +function main() { + let local = names; + churn(); + print(local.length); +} diff --git a/tslang/test/tester/own/own_err_global_returned.ts b/tslang/test/tester/own/own_err_global_returned.ts new file mode 100644 index 000000000..51916450b --- /dev/null +++ b/tslang/test/tester/own/own_err_global_returned.ts @@ -0,0 +1,12 @@ +// -mm=own rejects: a global owns its value, and a function that returns it gives its caller a +// second owner. +class V { + constructor(public x: number) {} + static def = new V(1); +} +function getDef() { + return V.def; +} +function main() { + print(getDef().x); +} diff --git a/tslang/test/tester/own/own_err_local_owns_nothing.ts b/tslang/test/tester/own/own_err_local_owns_nothing.ts new file mode 100644 index 000000000..84fc9374f --- /dev/null +++ b/tslang/test/tester/own/own_err_local_owns_nothing.ts @@ -0,0 +1,10 @@ +// -mm=own rejects: a local declared without a value owns nothing (MLIRGen makes no owner of it), +// so a new value stored into it later would have no owner to release it. +class C { + constructor(public x: number) {} +} +function main() { + let d: C; + d = new C(1); + print(d.x); +} diff --git a/tslang/test/tester/own/own_err_merged_result.ts b/tslang/test/tester/own/own_err_merged_result.ts new file mode 100644 index 000000000..44ee10df6 --- /dev/null +++ b/tslang/test/tester/own/own_err_merged_result.ts @@ -0,0 +1,8 @@ +// -mm=own rejects: the result of a `?:` is either branch's value, and nothing says which one +// owns it. +function max(a: string, b: string) { + return a > b ? a : b; +} +function main() { + print(max("a" + 1, "b" + 2)); +} diff --git a/tslang/test/tester/own/own_err_mixed_return.ts b/tslang/test/tester/own/own_err_mixed_return.ts new file mode 100644 index 000000000..9d78247f6 --- /dev/null +++ b/tslang/test/tester/own/own_err_mixed_return.ts @@ -0,0 +1,9 @@ +// -mm=own rejects: `greet` returns its argument on one path and a new string on the other, so +// its callers cannot be told whether they own the result. +function greet(name: string) { + if (name === "Honda") return name; + return "Sorry, " + name; +} +function main() { + print(greet("Honda")); +} diff --git a/tslang/test/tester/own/own_err_param_assigned.ts b/tslang/test/tester/own/own_err_param_assigned.ts new file mode 100644 index 000000000..811dde0b1 --- /dev/null +++ b/tslang/test/tester/own/own_err_param_assigned.ts @@ -0,0 +1,13 @@ +// -mm=own rejects: a parameter's value is its caller's; this function cannot put a value of its +// own in its place. +class C { + constructor(public x: number) {} +} +function reset(c: C): number { + c = new C(2); + return c.x; +} +function main() { + const c = new C(1); + print(reset(c), c.x); +} diff --git a/tslang/test/tester/own/own_err_param_kept_exported.ts b/tslang/test/tester/own/own_err_param_kept_exported.ts new file mode 100644 index 000000000..58f45ee49 --- /dev/null +++ b/tslang/test/tester/own/own_err_param_kept_exported.ts @@ -0,0 +1,12 @@ +// -mm=own rejects: the constructor keeps its parameter, which its callers would have to give up, +// but an exported class can be built from another module, which does not. +export class Animal { + name: string; + constructor(name: string) { + this.name = name; + } +} +function main() { + const a = new Animal("cat" + 1); + print(a.name); +} diff --git a/tslang/test/tester/own/own_err_param_returned_exported.ts b/tslang/test/tester/own/own_err_param_returned_exported.ts new file mode 100644 index 000000000..c50e155eb --- /dev/null +++ b/tslang/test/tester/own/own_err_param_returned_exported.ts @@ -0,0 +1,8 @@ +// -mm=own rejects: `identity` returns its parameter, a result its callers would have to know +// borrows the argument, but an exported function can be called from another module, which does not. +export function identity(x: string) { + return x; +} +function main() { + print(identity("x")); +} diff --git a/tslang/test/tester/own/own_err_record_literal_given.ts b/tslang/test/tester/own/own_err_record_literal_given.ts new file mode 100644 index 000000000..aa5ff5fd8 --- /dev/null +++ b/tslang/test/tester/own/own_err_record_literal_given.ts @@ -0,0 +1,14 @@ +// -mm=own rejects: an object literal that holds a new value is moved only into a local or returned +// as it is; given as an interface, it is boxed into a block of its own. +class Vec { + constructor(public n: number) {} +} +interface Holder { + v: Vec; +} +function make(n: number): Holder { + return { v: new Vec(n) }; +} +function main() { + print(make(1).v.n); +} diff --git a/tslang/test/tester/own/own_err_unbox_string.ts b/tslang/test/tester/own/own_err_unbox_string.ts new file mode 100644 index 000000000..92cd97dd1 --- /dev/null +++ b/tslang/test/tester/own/own_err_unbox_string.ts @@ -0,0 +1,9 @@ +// -mm=own rejects: `` of an `any` is the box's own string when it holds one, and a new +// string made from its number, boolean or bigint otherwise - a borrow on some paths and a value of +// its own on others. +function main() { + const s = "value"; + const a = s; + const back = a; + print(back); +} diff --git a/tslang/test/tester/own/own_interface_field_call.ts b/tslang/test/tester/own/own_interface_field_call.ts new file mode 100644 index 000000000..af0d6c69a --- /dev/null +++ b/tslang/test/tester/own/own_interface_field_call.ts @@ -0,0 +1,27 @@ +// A function-typed field of an interface (`toString: () => string`), called: MLIRGen binds the +// interface's `this` to the function it read out of the field and splits the bound function straight +// back into `this` and the function for the call. The call borrows the object, as a method call does. +interface Formattable { + prefix: string; + toString: () => string; +} + +function show(f: Formattable) { + return f.toString(); +} + +function main() { + const a = { + prefix: "A: ", + toString() { + return this.prefix + "hello"; + }, + }; + + const iface: Formattable = a; + print(iface.toString()); + assert(iface.toString() == "A: hello"); + assert(show(iface) == "A: hello"); + + print("done."); +} diff --git a/tslang/test/tester/own/own_nullable_view.ts b/tslang/test/tester/own/own_nullable_view.ts new file mode 100644 index 000000000..1bf80cc8f --- /dev/null +++ b/tslang/test/tester/own/own_nullable_view.ts @@ -0,0 +1,45 @@ +// A string or an array widened to a union that also holds null or undefined is the same block, and +// so is one narrowed back out of it: rc's retain of either is one of the block. A number widened +// into a union holds no block, and `ts.SafeCast` - the payload a `typeof` test narrowed - is the +// payload. own takes nothing for any of them. +function nameOr(n: number): string | null { + if (n > 0) return "named"; + return null; +} + +function keepName(s: string): string | null { + return s; +} + +function sure(s: string | null): string { + return s; +} + +function listOf(x: number[]): number[] | null { + return x; +} + +function widen(u: number | string): boolean | number | string { + if (typeof u == "string") return u; + return u; +} + +function main() { + const a = nameOr(1); + const b = nameOr(0); + assert(a == "named" && b == null); + + let made = "made" + 1; + const kept = keepName(made); + assert(kept == "made1"); + assert(sure(kept) == "made1"); + + const xs = listOf([1, 2, 3]); + assert(xs != null && xs.length == 3); + + const w = widen("w"); + const n = widen(2); + assert(typeof w == "string" && typeof n == "number"); + + print("done."); +} diff --git a/tslang/test/tester/own/own_spread_build.ts b/tslang/test/tester/own/own_spread_build.ts new file mode 100644 index 000000000..c6cde0545 --- /dev/null +++ b/tslang/test/tester/own/own_spread_build.ts @@ -0,0 +1,23 @@ +// An array a spread builds: MLIRGen makes it fresh in a local of its own that owns nothing, pushes +// into it, and reads it once, when it is done, into the owning local it is made for. The fresh +// array moves into the builder and on into that local. +function main() { + const a = [1, 2]; + const b = [...a, ...a]; + assert(b.length == 4 && b[3] == 2); + + const src: number[] = [1.5, 2.5, 3.5]; + const t: number[] = [...src]; + t.push(4.5); + assert(t.length == 4 && src.length == 3); + + let total = 0; + for (let i = 0; i < 100; i++) { + const twice = [...src, ...src]; + total += twice.length; + } + + assert(total == 600); + + print("done."); +}