From a546596a38e2ffd328c024e2040447f21c537715 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sat, 3 Oct 2026 19:00:37 +0100 Subject: [PATCH] A const of an owning let has a slot of its own (#454) Under rc, `let a = new C(); const b = a; a = new C();` freed the first object while `b` still pointed at it. A `const` whose initializer is a plain value is folded: the name is that value, with no storage. Here the value is a read of `a`'s slot, so `b` held no reference of its own, and reassigning `a` released the only one. processConstRef now gives such a const identity storage, as it already does for arrays and Shared handles: a const of anything that owns a block, read (through any casts) out of a local that owns what it holds (OWNED_LOCAL_ATTR_NAME - a declared `let`, a parameter; not a literal's temporary slot). It then takes its own reference, as a `let` does, in every model. A const of a const, a call or a field is unchanged. Under own, `b` is now a borrow of `a` rather than a later read of its slot, so own_err_const_alias_field is rejected for what it is - `'a' is moved here while 'b' borrows it` - and its two expectations are updated. The own corpus is unchanged (453 / 593, 451 with --opt). Test: 00const_of_let (compile and JIT, and in the rc/none corpus): a class, a string and an array const of a let, a const of a parameter, and a loop, each read after allocations of the same size have reused anything freed. It failed under rc before the change. Fixes #454. Co-Authored-By: Claude Opus 5.5 --- tslang/lib/TypeScript/MLIRGenImpl.h | 22 ++++++- tslang/test/tester/CMakeLists.txt | 7 ++- .../tester/own/own_err_const_alias_field.ts | 5 +- tslang/test/tester/tests/00const_of_let.ts | 61 +++++++++++++++++++ 4 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 tslang/test/tester/tests/00const_of_let.ts diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index 3d8f95dcd..faade2dbf 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -1461,6 +1461,20 @@ class MLIRGenImpl isUsed = varClass.isUsed; } + // A value read out of a local variable that owns what it holds, through any casts: what + // `a` is in `const b = a` when `a` is a `let`. A literal's temporary slot is not one. + static bool readsLocalSlot(mlir::Value value) + { + while (auto castOp = value ? value.getDefiningOp() : mlir_ts::CastOp()) + { + value = castOp.getIn(); + } + + auto loadOp = value ? value.getDefiningOp() : mlir_ts::LoadOp(); + auto varOp = loadOp ? loadOp.getReference().getDefiningOp() : mlir_ts::VariableOp(); + return varOp && varOp->hasAttr(OWNED_LOCAL_ATTR_NAME); + } + mlir::LogicalResult processConstRef(mlir::Location location, mlir::OpBuilder &builder, const GenContext &genContext) { if (mlir::failed(getVariableTypeAndInit(location, genContext))) @@ -1499,9 +1513,15 @@ class MLIRGenImpl // (`Shared | null`): `const b = a` is a second handle, and counts as one. A folded // name would be `a` itself, with no reference of its own, so `a = c` would free the // block `b` still reads, and `Shared.count` would miss it. + // + // So does a const of anything that owns a block, read out of a local variable's slot + // (`let a = new C(); const b = a;`, a parameter too): folded, it would be a read of + // that slot, holding no reference of its own, and `a = d` would free what `b` still + // reads (#454). A const of a const, a call or a field has no slot to lose it from. MLIRTypeHelper mth(builder.getContext(), compileOptions); needsIdentityStorage = mth.hasBoundMethodField(type) || isa(type) - || ((isa(type) || MLIRTypeHelper::isSharedHandleType(type)) + || ((isa(type) || MLIRTypeHelper::isSharedHandleType(type) + || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type))) && !(varClass == VariableType::ConstRef) && !(initial && initial.getDefiningOp())); if (needsIdentityStorage) diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 2ba3fadfa..df11b2bee 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -469,6 +469,7 @@ tslang_add_test(NAME test-compile-00-in-method-names COMMAND test-runner "${PROJ tslang_add_test(NAME test-compile-00-class-structural-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_structural_extends.ts") tslang_add_test(NAME test-compile-00-instanceof COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts") tslang_add_test(NAME test-compile-00-instanceof-union COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof_union.ts") +tslang_add_test(NAME test-compile-00-const-of-let COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_of_let.ts") tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts") tslang_add_test(NAME test-compile-00-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts") tslang_add_test(NAME test-compile-00-capture-in-new-arguments COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts") @@ -952,6 +953,7 @@ tslang_add_test(NAME test-jit-00-in-method-names COMMAND test-runner -jit "${PRO tslang_add_test(NAME test-jit-00-class-structural-extends COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_structural_extends.ts") tslang_add_test(NAME test-jit-00-instanceof COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts") tslang_add_test(NAME test-jit-00-instanceof-union COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof_union.ts") +tslang_add_test(NAME test-jit-00-const-of-let COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_of_let.ts") tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts") tslang_add_test(NAME test-jit-00-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts") tslang_add_test(NAME test-jit-00-capture-in-new-arguments COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts") @@ -1767,6 +1769,7 @@ set(TSLANG_CORPUS 00instanceof.ts 00instanceof_namespace_narrowing.ts 00instanceof_union.ts + 00const_of_let.ts 00interface_captures.ts 00interface_conjunction.ts 00interface_function_typed_field.ts @@ -2674,7 +2677,7 @@ add_test(NAME test-own-err-names-the-variable "${PROJECT_SOURCE_DIR}/test/tester/own/own_err_const_alias_field.ts" -o "${CMAKE_CURRENT_BINARY_DIR}/own-err-names-the-variable.obj") set_tests_properties(test-own-err-names-the-variable - PROPERTIES PASS_REGULAR_EXPRESSION "'a' is used here after its value was moved" + PROPERTIES PASS_REGULAR_EXPRESSION "'a' is moved here while 'b' borrows it" FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed") # Each negative program has one shape phase 0 cannot prove, and must be rejected with this text. @@ -2688,7 +2691,7 @@ set(own_error_cases "own_err_borrow_finally_local|borrows .* but is used here after .* is released or overwritten" "own_err_borrow_derived_ref|borrows .* but is used here after .* is released or overwritten" "own_err_const_alias_move|borrows .* and cannot be assigned" - "own_err_const_alias_field|is used here after its value was moved" + "own_err_const_alias_field|is moved here while .* borrows it" "own_err_field_alias|is used here after its value was moved" "own_err_call_in_try_drops|borrows .* but is used here after it may be released or overwritten" "own_err_closure_escape_used|is captured by a closure that escapes, and is still used here" diff --git a/tslang/test/tester/own/own_err_const_alias_field.ts b/tslang/test/tester/own/own_err_const_alias_field.ts index 5203cd1a8..40e273138 100644 --- a/tslang/test/tester/own/own_err_const_alias_field.ts +++ b/tslang/test/tester/own/own_err_const_alias_field.ts @@ -1,5 +1,6 @@ -// -mm=own, phase 1 rejects: `b` is a later read of `a`'s slot, and `a` moves into the field in -// between. Overwriting the field destroys the value `b` still reads. +// -mm=own rejects: `b` borrows `a` (a const of a `let` has a slot of its own, #454), and `a` +// moves into the field while `b` still borrows it. Overwriting the field destroys the value `b` +// still reads. class C { x: number; constructor(x: number) { this.x = x; } diff --git a/tslang/test/tester/tests/00const_of_let.ts b/tslang/test/tester/tests/00const_of_let.ts new file mode 100644 index 000000000..770353531 --- /dev/null +++ b/tslang/test/tester/tests/00const_of_let.ts @@ -0,0 +1,61 @@ +// `const b = a`, with `a` a `let` (or a parameter) holding a heap value, keeps that value alive +// after `a` is reassigned (#454). Under rc the const was folded into a read of `a`'s slot and took +// no reference, so reassigning `a` freed what `b` still pointed at. Each `b` is read after +// allocations of the same size have reused anything freed. +class Node { + v = 0; + name = ""; +} + +function churn() { + let keep: Node[] = []; + for (let i = 0; i < 1000; i++) { + const n = new Node(); + n.v = -i; + n.name = "junk" + i; + keep.push(n); + } + + return keep.length; +} + +function fromParam(p: Node) { + const kept = p; + p = new Node(); + assert(churn() == 1000); + return kept.name; +} + +function main() { + let a = new Node(); + a.name = "first" + 1; + const b = a; + a = new Node(); + assert(churn() == 1000); + assert(b.name == "first1", "a const of a let keeps the first node"); + + let s = "text" + 2; + const t = s; + s = "other" + 3; + assert(churn() == 1000); + assert(t == "text2", "a const of a let keeps the first string"); + + let arr = [1, 2, 3]; + const kept = arr; + arr = [4]; + assert(churn() == 1000); + assert(kept.length == 3 && kept[2] == 3, "a const of a let keeps the first array"); + + const n = new Node(); + n.name = "param" + 4; + assert(fromParam(n) == "param4", "a const of a parameter keeps its node"); + + for (let i = 0; i < 1000; i++) { + let x = new Node(); + const y = x; + x = new Node(); + y.v = i; + } + + print("done."); +}