From d86e7539d26a6b5549016b13b437005ef8ffc214 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sat, 14 Nov 2026 15:07:17 +0000 Subject: [PATCH 1/2] own: a const declared from a merge of owning locals borrows them; a merge made in a loop's body (TSLANG 58) `const selected = c ? tensor0 : tensor1` (native_ts_graphics core/directx_math.ts) was refused as merged from several branches. Each local keeps its value, so the const borrows each of them (slotLoadsOfMerge, decideSlot). takerLoops took a merge's own declaration as reached again around a loop before the merge is remade, which refused it in a loop's body, and TSLANG 54's fresh merge there too. Co-Authored-By: Claude Opus 5.5 --- TSLANG_ISSUES.md | 27 +++++-- tslang/lib/TypeScript/OwnershipFacts.h | 66 ++++++++++++++++ .../TypeScript/OwnershipInferenceClosures.cpp | 5 +- .../TypeScript/OwnershipInferenceMoves.cpp | 7 +- .../lib/TypeScript/OwnershipInferencePass.cpp | 22 ++++++ .../lib/TypeScript/OwnershipInferencePass.h | 5 ++ tslang/test/tester/CMakeLists.txt | 8 +- .../tester/own/own_err_merged_locals_moved.ts | 16 ++++ .../own/own_err_merged_locals_overwritten.ts | 14 ++++ tslang/test/tester/tests/00tslang_issue_58.ts | 78 +++++++++++++++++++ 10 files changed, 238 insertions(+), 10 deletions(-) create mode 100644 tslang/test/tester/own/own_err_merged_locals_moved.ts create mode 100644 tslang/test/tester/own/own_err_merged_locals_overwritten.ts create mode 100644 tslang/test/tester/tests/00tslang_issue_58.ts diff --git a/TSLANG_ISSUES.md b/TSLANG_ISSUES.md index 0e42c79ca..325b7f0a0 100644 --- a/TSLANG_ISSUES.md +++ b/TSLANG_ISSUES.md @@ -353,6 +353,21 @@ own now, and with it `examples/render_octomap.ts`. Six of ntg's `core/` modules Regression tests `tslang/test/tester/tests/00tslang_issue_57.ts` (every model, and as an exe) and `own/own_err_parameter_let_stored.ts`. +58 (a `const` declared from a merge of owning locals: `const selected = x2py2gez2pw2 ? (x2gey2 ? tensor0 : +tensor1) : (z2gew2 ? tensor2 : tensor3)` in `core/directx_math.ts`'s quaternion-from-matrix, refused "is +merged from several branches; -mm=own cannot prove which one owns it yet") is fixed. Even `const s = c ? +t0 : t1` was refused: each local keeps its value, and is released at its scope's end, so neither can move +into the merge. The `const` (or `let`) now borrows each of them, as a `let` given one local while that +local is still used already did: rc's retain and release for it go, and each local's own releases and +assignments end the borrow. One of them assigned or moved while the `const` is still used, or the +`const` stored, returned or assigned, is still refused. null or undefined may be one of the ways in. A +merge made in a loop's body was also refused, "moved inside a loop but was made outside it" (the merge's +own declaration was taken as reached again around the loop before the merge is remade), which also hit +54's `const p = c ? mk() : null` in a loop. With `directx_math.ts` compiling, its facts object exists, so +`core/simple_camera.ts` compiles too (it had been refused for calling into a module with no facts), and +the sweep went from 141 failing sources to 126. Regression tests `tslang/test/tester/tests/00tslang_issue_58.ts` +(every model) and `own/own_err_merged_locals_overwritten.ts`, `own/own_err_merged_locals_moved.ts`. + For native_ts_graphics, from the same sweep (its code, not the compiler): - `core/ray_tracing_extended_scene.ts`: the exported classes' constructors keep their array parameters (`this.origin = origin`), which an exported constructor has to say: `@keeps("origin", "direction")` @@ -361,8 +376,10 @@ For native_ts_graphics, from the same sweep (its code, not the compiler): - `examples/dynamic_primitive_clipping.ts:223`, `desc.setDepthClip(clip)` with `clip` the loop's `int`, fails under every model since #846 (a number is not a `boolean` argument, TS2345). alpha127 still takes it; the next release won't. `desc.setDepthClip(clip != 0)` fixes it. -- Under `-mm=own`, 7 `core/` modules do not compile yet (`directx_math.ts`, `donut_first_person_camera.ts`, - `nri_donut_textures.ts`, `octomap_common.ts`, `ray_tracing_extended_scene.ts`, `simple_camera.ts`, - `rtxpt_game.ts`), and nor do most examples, which #310 did not port: with every facts object built - and 50-53 in, 143 of the 276 sources fail. The largest group left is "borrows a field but is used - here after it may be released or overwritten" (about 280 sites). +- Under `-mm=own`, 4 `core/` modules do not compile yet (`donut_first_person_camera.ts`, + `nri_donut_textures.ts`, `ray_tracing_extended_scene.ts`, `rtxpt_game.ts`), and nor do many examples, + which #310 did not port: with every facts object built and 50-58 in, 126 of the 276 sources fail. A + compiler gap 58 uncovered: 50's "a value moved into a field and used after borrows the field" does not + hold inside a loop's body (`frame.buf = buf; if (buf) ...` in `examples/nri/procedural_geometry_nri.ts` + is "used here after its value was moved", 49 sites in all). The largest group left is "borrows a field + but is used here after it may be released or overwritten" (about 200 sites). diff --git a/tslang/lib/TypeScript/OwnershipFacts.h b/tslang/lib/TypeScript/OwnershipFacts.h index 4dfedecf5..fb9923743 100644 --- a/tslang/lib/TypeScript/OwnershipFacts.h +++ b/tslang/lib/TypeScript/OwnershipFacts.h @@ -1115,6 +1115,72 @@ inline mlir_ts::LoadOp slotLoadOf(mlir::Value value) return loadOp; } +// A merge of owning locals' values (TSLANG 58): `const selected = c ? tensor0 : tensor1`. Every way +// into it brings a read of an owning local (slotLoadOf), null or undefined, or another such merge; +// `loads` gets the reads. Not across a loop, as isFreshMerge. +inline bool slotLoadsOfMerge(mlir::BlockArgument argument, llvm::SmallVectorImpl &loads, unsigned depth = 0) +{ + auto *block = argument.getOwner(); + if (block->isEntryBlock() || block->hasNoPredecessors() || depth > 8) + { + return false; + } + + mlir::DominanceInfo dominance(block->getParentOp()); + for (auto predecessor = block->pred_begin(); predecessor != block->pred_end(); ++predecessor) + { + auto branchOp = mlir::dyn_cast((*predecessor)->getTerminator()); + if (dominance.dominates(block, *predecessor) || !branchOp) + { + return false; + } + + auto operands = branchOp.getSuccessorOperands(predecessor.getSuccessorIndex()); + if (argument.getArgNumber() < operands.getProducedOperandCount()) + { + return false; + } + + auto incoming = operands[argument.getArgNumber()]; + if (!incoming) + { + return false; + } + + auto constant = incoming; + while (auto castOp = constant.getDefiningOp()) + { + constant = castOp.getIn(); + } + + if (constant.getDefiningOp() || constant.getDefiningOp() || + constant.getDefiningOp()) + { + continue; + } + + if (auto nested = mlir::dyn_cast(incoming)) + { + if (!nested.hasOneUse() || !slotLoadsOfMerge(nested, loads, depth + 1)) + { + return false; + } + + continue; + } + + auto load = slotLoadOf(incoming); + if (!load) + { + return false; + } + + loads.push_back(load); + } + + return true; +} + // The ownership passes ask MLIRTypeHelper about types only, under the default options. inline MLIRTypeHelper typeHelper(mlir::MLIRContext *context) { diff --git a/tslang/lib/TypeScript/OwnershipInferenceClosures.cpp b/tslang/lib/TypeScript/OwnershipInferenceClosures.cpp index a1e615f24..ead2c22e9 100644 --- a/tslang/lib/TypeScript/OwnershipInferenceClosures.cpp +++ b/tslang/lib/TypeScript/OwnershipInferenceClosures.cpp @@ -1906,7 +1906,10 @@ namespace own_inference void OwnershipInferencePass::decideSlot(mlir::Value slot, llvm::ArrayRef receivers, llvm::SetVector &toErase) { auto owner = varName(slot.getDefiningOp()); - if (receivers.size() == 1 || takeValuesOfTheirOwn(slot, receivers)) + + // a `let` declared from a merge of this and other locals borrows it, so nothing moves it (TSLANG 58) + auto merged = llvm::any_of(receivers, [&](const SlotReceiver &receiver) { return mergedSlotRetains.contains(receiver.retain); }); + if (!merged && (receivers.size() == 1 || takeValuesOfTheirOwn(slot, receivers))) { // a move on some paths only is placed when it is the local's one receiver (spec 30) for (auto &receiver : receivers) diff --git a/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp b/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp index ad9c08299..2afa5b01b 100644 --- a/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp +++ b/tslang/lib/TypeScript/OwnershipInferenceMoves.cpp @@ -1091,10 +1091,13 @@ namespace own_inference continue; } + // the value is made again before the taker runs again - a merge (a block's argument, + // whose definition is its block's first op) made on entering the taker's block, even + // when that first op is the taker itself (`const s = c ? t0 : t1` in a loop, TSLANG 58) if (kill && kill->getBlock() == block && - (block != top->getBlock() || kill->isBeforeInBlock(top))) + (block != top->getBlock() || kill == top || kill->isBeforeInBlock(top))) { - continue; // the value is made again before the taker runs again + continue; } if (block == top->getBlock()) diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index 8fa217b69..b8ade0d49 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -393,6 +393,7 @@ namespace own_inference fieldBorrowRetains.clear(); fieldBorrowReleases.clear(); mergeBorrowRetains.clear(); + mergedSlotRetains.clear(); captureStores.clear(); captureRetains.clear(); callCopies.clear(); @@ -702,6 +703,27 @@ namespace own_inference continue; } + // a `let` declared from a merge of owning locals (TSLANG 58): it borrows each of them + if (auto argument = value ? mlir::dyn_cast(rootOf(value)) : mlir::BlockArgument(); + argument && mlir::isa(op) && borrowerOf(op)) + { + llvm::SmallVector loads; + if (slotLoadsOfMerge(argument, loads) && !loads.empty()) + { + mergedSlotRetains.insert(op); + llvm::SmallPtrSet slots; + for (auto load : loads) + { + if (slots.insert(load.getReference()).second) + { + slotReceivers[load.getReference()].push_back({op, value, load}); + } + } + + continue; + } + } + if (value && isReadOutOfException(value)) { toErase.insert(op); diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.h b/tslang/lib/TypeScript/OwnershipInferencePass.h index 31d52ec06..5abb515a8 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.h +++ b/tslang/lib/TypeScript/OwnershipInferencePass.h @@ -1104,6 +1104,11 @@ class OwnershipInferencePass // decided there llvm::DenseSet mergeBorrowRetains; + // the `ts.RetainSlot`s of the `let`s declared from a merge of owning locals (TSLANG 58, `const + // selected = c ? tensor0 : tensor1`): a receiver of each of those locals, which the `let` borrows + // (decideSlot) + llvm::DenseSet mergedSlotRetains; + // 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 e7ad024e2..c54ce8198 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -1911,9 +1911,11 @@ endforeach() # TSLANG_ISSUES 55: an array literal of fields, only read, borrows them # TSLANG_ISSUES 56: a `let` given only fields borrows each until the next assignment # TSLANG_ISSUES 57: a `let` that only holds parameters owns nothing +# TSLANG_ISSUES 58: a `const` declared from a merge of owning locals borrows them; a merge made in a loop's body foreach(issues_mm gc rc none own) foreach(issues_test 00tslang_issue_44 00tslang_issue_45 00tslang_issue_46 00tslang_issue_47 00tslang_issue_48 00tslang_issue_49 00tslang_issue_50 - 00tslang_issue_52 00tslang_issue_53 00tslang_issue_54 00tslang_issue_55 00tslang_issue_56 00tslang_issue_57) + 00tslang_issue_52 00tslang_issue_53 00tslang_issue_54 00tslang_issue_55 00tslang_issue_56 00tslang_issue_57 + 00tslang_issue_58) tslang_add_test(NAME test-jit-${issues_test}-${issues_mm} COMMAND test-runner -jit -mm=${issues_mm} "${PROJECT_SOURCE_DIR}/test/tester/tests/${issues_test}.ts") tslang_add_test(NAME test-compile-${issues_test}-${issues_mm} COMMAND test-runner -mm=${issues_mm} "${PROJECT_SOURCE_DIR}/test/tester/tests/${issues_test}.ts") endforeach() @@ -5463,7 +5465,9 @@ set(own_error_cases "own_err_merge_moved_twice|:19:9: error: '[^']*' borrows a field and cannot be stored, returned or captured" "own_err_borrowing_array_field_reset|:13:16: error: '[^']*' borrows a field but is used here after it may be released or overwritten" "own_err_let_borrows_reassigned_overwritten|:17:16: error: '[^']*' borrows a field but is used here after it may be released or overwritten" - "own_err_parameter_let_stored|:10:5: error: '[^']*' takes a second reference") + "own_err_parameter_let_stored|:10:5: error: '[^']*' takes a second reference" + "own_err_merged_locals_overwritten|:11:16: error: '[^']*' borrows '[^']*' but is used here after '[^']*' is released or overwritten" + "own_err_merged_locals_moved|:12:5: error: '[^']*' is moved here while '[^']*' borrows it") 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_err_merged_locals_moved.ts b/tslang/test/tester/own/own_err_merged_locals_moved.ts new file mode 100644 index 000000000..c4e8d8a03 --- /dev/null +++ b/tslang/test/tester/own/own_err_merged_locals_moved.ts @@ -0,0 +1,16 @@ +// TSLANG_ISSUES 58: a `const` declared from a merge of owning locals borrows each of them, so none of +// them can be moved out while it does. +function mk(a: number): number[] { return [a, a]; } +function sum(v: number[]): number { return v[0] + v[1]; } + +let kept: number[] = []; + +export function f(a: boolean): number { + const t0 = mk(1.0); + const t1 = mk(2.0); + const s = a ? t0 : t1; + kept = t0; + return sum(s); +} + +function main() { print(f(true)); } diff --git a/tslang/test/tester/own/own_err_merged_locals_overwritten.ts b/tslang/test/tester/own/own_err_merged_locals_overwritten.ts new file mode 100644 index 000000000..7e11774fa --- /dev/null +++ b/tslang/test/tester/own/own_err_merged_locals_overwritten.ts @@ -0,0 +1,14 @@ +// TSLANG_ISSUES 58: a `const` declared from a merge of owning locals borrows each of them, so one of +// them given a new value while it is still used frees what the `const` may hold. +function mk(a: number): number[] { return [a, a]; } +function sum(v: number[]): number { return v[0] + v[1]; } + +export function f(a: boolean): number { + let t0 = mk(1.0); + const t1 = mk(2.0); + const s = a ? t0 : t1; + t0 = mk(3.0); + return sum(s); +} + +function main() { print(f(true)); } diff --git a/tslang/test/tester/tests/00tslang_issue_58.ts b/tslang/test/tester/tests/00tslang_issue_58.ts new file mode 100644 index 000000000..5edb6d390 --- /dev/null +++ b/tslang/test/tester/tests/00tslang_issue_58.ts @@ -0,0 +1,78 @@ +// TSLANG_ISSUES 58 (-mm=own), from native_ts_graphics' core/directx_math.ts (XMQuaternionRotationMatrix): +// `const selected = c1 ? (c2 ? tensor0 : tensor1) : (c3 ? tensor2 : tensor3)` over four owned locals, +// then only read. Each local keeps its value, so `selected` borrows whichever one it was given - in a +// loop's body too, and with null as one way in. + +function mk(a: number): number[] { + return [a, a + 1.0, a + 2.0, a + 3.0]; +} + +function sum(v: number[]): number { + let s = 0.0; + for (let i = 0; i < v.length; i++) { + s += v[i]; + } + + return s; +} + +function scaled(v: number[], s: number): number[] { + return [v[0] / s, v[1] / s, v[2] / s, v[3] / s]; +} + +export function pick(a: boolean, b: boolean, c: boolean): number[] { + const tensor0 = mk(1.0); + const tensor1 = mk(2.0); + const tensor2 = mk(3.0); + const tensor3 = mk(4.0); + const selected = a ? (b ? tensor0 : tensor1) : (c ? tensor2 : tensor3); + return scaled(selected, sum(selected)); +} + +export function total(a: boolean): number { + let r = 0.0; + for (let i = 0; i < 3; i++) { + const t0 = mk(i); + const t1 = mk(10.0); + const s = a ? t0 : t1; + r += sum(s); + } + + return r; +} + +export function outer(a: boolean): number { + const t0 = mk(1.0); + const t1 = mk(2.0); + let r = 0.0; + for (let i = 0; i < 2; i++) { + const s = i == 0 ? t0 : t1; + r += sum(s); + } + + const n = a ? t0 : null; + return n ? r + sum(n) : r; +} + +export function freshInLoop(a: boolean): number { + let r = 0.0; + for (let i = 0; i < 3; i++) { + const p = a ? mk(i) : null; + if (p) { + r += sum(p); + } + } + + return r; +} + +function main() { + const p = pick(false, true, false); + print(p[0], p[3]); + assert(p[0] == 4.0 / 22.0 && p[3] == 7.0 / 22.0); + assert(pick(true, true, true)[0] == 0.1 && pick(true, false, true)[0] == 2.0 / 14.0); + assert(total(true) == 6.0 + 10.0 + 14.0 && total(false) == 3.0 * 46.0); + assert(outer(true) == 10.0 + 14.0 + 10.0 && outer(false) == 24.0); + assert(freshInLoop(true) == 30.0 && freshInLoop(false) == 0.0); + print("done."); +} From 8657e1951b60c2ee06a58e3a9d31f0dcd8861287 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sat, 14 Nov 2026 15:24:51 +0000 Subject: [PATCH 2/2] own: a value moved into a field of an element or field read, then used, borrows that field (TSLANG 59) `const frame = this.frames[i]; frame.buf = buf; if (buf) ...` (native_ts_graphics examples/nri/procedural_geometry_nri.ts) was a use after a move: TSLANG 50/52 read the field back right after the move only through an object read out of a local or parameter. Right after the store nothing runs, so an object read out of an element or a field is the same object (objectReadableRightAfter). Co-Authored-By: Claude Opus 5.5 --- TSLANG_ISSUES.md | 25 +++++-- .../OwnershipSignatureFieldLocals.cpp | 19 ++++- tslang/test/tester/CMakeLists.txt | 6 +- .../own_err_element_field_moved_replaced.ts | 14 ++++ tslang/test/tester/tests/00tslang_issue_59.ts | 73 +++++++++++++++++++ 5 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 tslang/test/tester/own/own_err_element_field_moved_replaced.ts create mode 100644 tslang/test/tester/tests/00tslang_issue_59.ts diff --git a/TSLANG_ISSUES.md b/TSLANG_ISSUES.md index 325b7f0a0..3dbd83fe5 100644 --- a/TSLANG_ISSUES.md +++ b/TSLANG_ISSUES.md @@ -368,6 +368,22 @@ own declaration was taken as reached again around the loop before the merge is r the sweep went from 141 failing sources to 126. Regression tests `tslang/test/tester/tests/00tslang_issue_58.ts` (every model) and `own/own_err_merged_locals_overwritten.ts`, `own/own_err_merged_locals_moved.ts`. +59 (a value moved into a field of an object read out of an array, then used: `const queuedFrame = +this.queuedFrames[i]; ... queuedFrame.aabbAttributeBuffer = buffer; const view = buffer ? +buffer.createStructuredView(0) : null;` in `examples/nri/procedural_geometry_nri.ts`, refused "used here +after its value was moved") is fixed. 50 and 52 follow a value moved into a field, or pushed into a field +array, by reading the field right after the move; they took only an object read out of a local or +parameter nothing assigns. A `const` given an element or a field is no local by then (it is the read +itself), so `frame.view = view` with `const frame = frames[i]` was left a move. The read right after the +move now takes an object read out of an element or a field too: nothing runs between, so it is the same +object, and the checks then follow the borrow through that chain. The element or the field it hangs from +replaced, popped or reset before a use is still refused. In the +sweep, "used here after its value was moved" went from 49 sites to 37, and `core/ray_tracing_extended_scene.ts`'s +own errors are left (its `@keeps` and `Shared`, below). One site of this shape is left, +`procedural_geometry_nri.ts:425` (`!aabbAttributeView` after the `||` chain), which a reduced copy of it +does not reproduce. Regression tests `tslang/test/tester/tests/00tslang_issue_59.ts` (every model) and +`own/own_err_element_field_moved_replaced.ts`. + For native_ts_graphics, from the same sweep (its code, not the compiler): - `core/ray_tracing_extended_scene.ts`: the exported classes' constructors keep their array parameters (`this.origin = origin`), which an exported constructor has to say: `@keeps("origin", "direction")` @@ -378,8 +394,7 @@ For native_ts_graphics, from the same sweep (its code, not the compiler): takes it; the next release won't. `desc.setDepthClip(clip != 0)` fixes it. - Under `-mm=own`, 4 `core/` modules do not compile yet (`donut_first_person_camera.ts`, `nri_donut_textures.ts`, `ray_tracing_extended_scene.ts`, `rtxpt_game.ts`), and nor do many examples, - which #310 did not port: with every facts object built and 50-58 in, 126 of the 276 sources fail. A - compiler gap 58 uncovered: 50's "a value moved into a field and used after borrows the field" does not - hold inside a loop's body (`frame.buf = buf; if (buf) ...` in `examples/nri/procedural_geometry_nri.ts` - is "used here after its value was moved", 49 sites in all). The largest group left is "borrows a field - but is used here after it may be released or overwritten" (about 200 sites). + which #310 did not port: with every facts object built and 50-59 in, 126 of the 276 sources fail. The + largest group left is "borrows a field but is used here after it may be released or overwritten" (about + 200 sites), then "used here after its value was moved" (37: mostly a value a pushed object or an + earlier call keeps, then returned or used). diff --git a/tslang/lib/TypeScript/OwnershipSignatureFieldLocals.cpp b/tslang/lib/TypeScript/OwnershipSignatureFieldLocals.cpp index 472f6d7a8..1eb6bbe60 100644 --- a/tslang/lib/TypeScript/OwnershipSignatureFieldLocals.cpp +++ b/tslang/lib/TypeScript/OwnershipSignatureFieldLocals.cpp @@ -119,6 +119,21 @@ namespace own_signature return isUnassignedSlot(loadOp.getReference()); } + // The object can be read again right after a store into its field or a push into it - nothing + // runs between, so a read of an element or of a field is the same object (TSLANG 59: `const + // frame = this.frames[i]; frame.view = view;`, with `frame` the element read itself). + bool objectReadableRightAfter(mlir_ts::PropertyRefOp field) + { + if (objectReadable(field)) + { + return true; + } + + auto loadOp = field.getObjectRef().getDefiningOp(); + return loadOp && (loadOp.getReference().getDefiningOp() || + loadOp.getReference().getDefiningOp()); + } + // `value` seen through the views that keep its block: an optional made of it or opened, a // widening or narrowing cast that is a view. mlir::Value stripViews(mlir::Value value) @@ -606,7 +621,7 @@ namespace own_signature llvm::SmallVector movedStores; body->walk([&](mlir_ts::StoreOp storeOp) { auto field = storeOp.getReference().getDefiningOp(); - if (field && objectReadable(field)) + if (field && objectReadableRightAfter(field)) { movedStores.push_back(storeOp); } @@ -670,7 +685,7 @@ namespace own_signature for (auto pushOp : pushes) { auto field = pushOp.getOp().getDefiningOp(); - if (pushOp.getItems().size() != 1 || !field || !objectReadable(field)) + if (pushOp.getItems().size() != 1 || !field || !objectReadableRightAfter(field)) { continue; } diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index c54ce8198..c9dd59d1a 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -1912,10 +1912,11 @@ endforeach() # TSLANG_ISSUES 56: a `let` given only fields borrows each until the next assignment # TSLANG_ISSUES 57: a `let` that only holds parameters owns nothing # TSLANG_ISSUES 58: a `const` declared from a merge of owning locals borrows them; a merge made in a loop's body +# TSLANG_ISSUES 59: a value moved into a field of an element or field read, then used, borrows that field foreach(issues_mm gc rc none own) foreach(issues_test 00tslang_issue_44 00tslang_issue_45 00tslang_issue_46 00tslang_issue_47 00tslang_issue_48 00tslang_issue_49 00tslang_issue_50 00tslang_issue_52 00tslang_issue_53 00tslang_issue_54 00tslang_issue_55 00tslang_issue_56 00tslang_issue_57 - 00tslang_issue_58) + 00tslang_issue_58 00tslang_issue_59) tslang_add_test(NAME test-jit-${issues_test}-${issues_mm} COMMAND test-runner -jit -mm=${issues_mm} "${PROJECT_SOURCE_DIR}/test/tester/tests/${issues_test}.ts") tslang_add_test(NAME test-compile-${issues_test}-${issues_mm} COMMAND test-runner -mm=${issues_mm} "${PROJECT_SOURCE_DIR}/test/tester/tests/${issues_test}.ts") endforeach() @@ -5467,7 +5468,8 @@ set(own_error_cases "own_err_let_borrows_reassigned_overwritten|:17:16: error: '[^']*' borrows a field but is used here after it may be released or overwritten" "own_err_parameter_let_stored|:10:5: error: '[^']*' takes a second reference" "own_err_merged_locals_overwritten|:11:16: error: '[^']*' borrows '[^']*' but is used here after '[^']*' is released or overwritten" - "own_err_merged_locals_moved|:12:5: error: '[^']*' is moved here while '[^']*' borrows it") + "own_err_merged_locals_moved|:12:5: error: '[^']*' is moved here while '[^']*' borrows it" + "own_err_element_field_moved_replaced|:11:12: error: '[^']*' borrows an element but is used here after it may be released or overwritten") 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_err_element_field_moved_replaced.ts b/tslang/test/tester/own/own_err_element_field_moved_replaced.ts new file mode 100644 index 000000000..058c417a9 --- /dev/null +++ b/tslang/test/tester/own/own_err_element_field_moved_replaced.ts @@ -0,0 +1,14 @@ +// TSLANG_ISSUES 59: a value moved into a field of an array's element borrows that field, so the +// element replaced before a use of the value frees it. +class View { n = 1; } +class Frame { view: View | null = null; } + +export function f(frames: Frame[]): int { + const frame = frames[0]; + const view = new View(); + frame.view = view; + frames[0] = new Frame(); + return view.n; +} + +function main() { print(f([new Frame()])); } diff --git a/tslang/test/tester/tests/00tslang_issue_59.ts b/tslang/test/tester/tests/00tslang_issue_59.ts new file mode 100644 index 000000000..7d448ebe7 --- /dev/null +++ b/tslang/test/tester/tests/00tslang_issue_59.ts @@ -0,0 +1,73 @@ +// TSLANG_ISSUES 59 (-mm=own), from native_ts_graphics' examples/nri/procedural_geometry_nri.ts: a value moved +// into a field of an object read out of an array, then used - `const queuedFrame = this.queuedFrames[i]; +// ... queuedFrame.aabbAttributeBuffer = buffer; const view = buffer ? buffer.createStructuredView(0) : +// null;`. After the store the value is what the field holds (TSLANG 50), whether the object is a local, a +// parameter or an element or field read. + +class View { + n: int; + constructor(n: int) { + this.n = n; + } +} + +class Buffer { + size: int; + constructor(size: int) { + this.size = size; + } + + makeView(): View { + return new View(this.size * 10); + } +} + +class Frame { + buffer: Buffer | null = null; + view: View | null = null; + views: View[] = []; +} + +class Holder { + frame = new Frame(); + frames: Frame[] = [new Frame(), new Frame()]; +} + +function makeBuffer(i: int): Buffer | null { + return i > 0 ? new Buffer(i) : null; +} + +export function setup(frames: Frame[]): int { + let total = 0; + for (let i = 0; i < frames.length; i++) { + const frame = frames[i]; + const buffer = makeBuffer(i); + frame.buffer = buffer; + const view = buffer ? buffer.makeView() : null; + frame.view = view; + if (!view) { + continue; + } + + total += view.n; + } + + return total; +} + +export function throughField(h: Holder): int { + const view = new View(7); + h.frame.view = view; + const pushed = new View(8); + h.frames[1].views.push(pushed); + return view.n + pushed.n; +} + +function main() { + const frames = [new Frame(), new Frame(), new Frame()]; + assert(setup(frames) == 30); + assert(frames[0].view == null && frames[2].buffer != null && frames[2].view != null); + const h = new Holder(); + assert(throughField(h) == 15 && h.frames[1].views.length == 1); + print("done."); +}