A const of an owning let has a slot of its own (#454)

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>() : mlir_ts::CastOp())
+            {
+                value = castOp.getIn();
+            }
+
+            auto loadOp = value ? value.getDefiningOp<mlir_ts::LoadOp>() : mlir_ts::LoadOp();
+            auto varOp = loadOp ? loadOp.getReference().getDefiningOp<mlir_ts::VariableOp>() : 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<T> | 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<mlir_ts::ConstArrayType>(type)
-                || ((isa<mlir_ts::ArrayType>(type) || MLIRTypeHelper::isSharedHandleType(type))
+                || ((isa<mlir_ts::ArrayType>(type) || MLIRTypeHelper::isSharedHandleType(type)
+                     || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type)))
                     && !(varClass == VariableType::ConstRef)
                     && !(initial && initial.getDefiningOp<mlir_ts::SafeCastOp>()));
             if (needsIdentityStorage)

tslang/test/tester/CMakeLists.txt

@@ -468,6 +468,7 @@ tslang_add_test(NAME test-compile-00-in COMMAND test-runner "${PROJECT_SOURCE_DI
 tslang_add_test(NAME test-compile-00-in-method-names COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00in_method_names.ts")
 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-const-of-let COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_of_let.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-nullable-to-any COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00nullable_to_any.ts")
 tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
@@ -952,6 +953,7 @@ tslang_add_test(NAME test-jit-00-in COMMAND test-runner -jit "${PROJECT_SOURCE_D
 tslang_add_test(NAME test-jit-00-in-method-names COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00in_method_names.ts")
 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-const-of-let COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_of_let.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-nullable-to-any COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00nullable_to_any.ts")
 tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
@@ -1767,6 +1769,7 @@ set(TSLANG_CORPUS
     00in.ts
     00infer.ts
     00instanceof.ts
+    00const_of_let.ts
     00instanceof_namespace_narrowing.ts
     00instanceof_union.ts
     00nullable_to_any.ts
@@ -2677,7 +2680,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.
@@ -2691,7 +2694,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"

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; }

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.");
+}