A const object literal holding a borrowed field has a slot of its own (#460)

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -1475,6 +1475,70 @@ class MLIRGenImpl
             return varOp && varOp->hasAttr(OWNED_LOCAL_ATTR_NAME);
         }
 
+        // A record read out whole from the temporary slot an object literal is built in (one that
+        // owns nothing of its own), with a field given a value read out of another object's field
+        // or element, or out of an owning local: what `{ s: p.child }` and `{ x: a }` are. Such a
+        // value carries no reference, and the literal takes none for it.
+        static bool readsLiteralSlot(MLIRTypeHelper &mth, mlir::Location location, mlir::Value value)
+        {
+            auto loadOp = value ? value.getDefiningOp<mlir_ts::LoadOp>() : mlir_ts::LoadOp();
+            auto varOp = loadOp ? loadOp.getReference().getDefiningOp<mlir_ts::VariableOp>() : mlir_ts::VariableOp();
+            if (!varOp || varOp->hasAttr(OWNED_LOCAL_ATTR_NAME))
+            {
+                return false;
+            }
+
+            for (auto *user : varOp->getUsers())
+            {
+                auto fieldRef = dyn_cast<mlir_ts::PropertyRefOp>(user);
+                if (!fieldRef)
+                {
+                    continue;
+                }
+
+                for (auto *fieldUser : fieldRef->getUsers())
+                {
+                    auto storeOp = dyn_cast<mlir_ts::StoreOp>(fieldUser);
+                    if (!storeOp || storeOp.getReference() != fieldRef.getResult())
+                    {
+                        continue;
+                    }
+
+                    // only a field that owns a block counts: not a reference (a generator's
+                    // `.captured`), and not a method bound to the object it was read from (`{ ...it }`
+                    // of a generator), whose routines do not pair up for it
+                    auto storedType = storeOp.getValue().getType();
+                    if (!mth.ownsHeapMemory(location, storedType) ||
+                        isa<mlir_ts::BoundFunctionType, mlir_ts::HybridFunctionType>(storedType))
+                    {
+                        continue;
+                    }
+
+                    auto stored = storeOp.getValue();
+                    while (auto castOp = stored.getDefiningOp<mlir_ts::CastOp>())
+                    {
+                        stored = castOp.getIn();
+                    }
+
+                    auto storedLoad = stored.getDefiningOp<mlir_ts::LoadOp>();
+                    auto *place = storedLoad ? storedLoad.getReference().getDefiningOp() : nullptr;
+                    if (place && isa<mlir_ts::PropertyRefOp, mlir_ts::ElementRefOp>(place))
+                    {
+                        return true;
+                    }
+
+                    // a local that owns what it holds (`{ x: a }`, #454's shape), or a literal
+                    // nested in this one
+                    if (readsLocalSlot(stored) || readsLiteralSlot(mth, location, stored))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+
         mlir::LogicalResult processConstRef(mlir::Location location, mlir::OpBuilder &builder, const GenContext &genContext)
         {
             if (mlir::failed(getVariableTypeAndInit(location, genContext)))
@@ -1518,10 +1582,17 @@ class MLIRGenImpl
             // (`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.
+            //
+            // And a const record whose fields own blocks (`const o = { s: p.child }`): the literal
+            // hands its fields over unretained - its receiver is the one that retains them, as a
+            // `let` does - so folded, `o` held none of them, and `p.child = d` freed what `o.s`
+            // still read (#460).
             MLIRTypeHelper mth(builder.getContext(), compileOptions);
             needsIdentityStorage = mth.hasBoundMethodField(type) || isa<mlir_ts::ConstArrayType>(type)
                 || ((isa<mlir_ts::ArrayType>(type) || MLIRTypeHelper::isSharedHandleType(type)
-                     || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type)))
+                     || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type))
+                     || (readsLiteralSlot(mth, location, initial) && isa<mlir_ts::TupleType>(type)
+                         && mth.ownsHeapMemory(location, type)))
                     && !(varClass == VariableType::ConstRef)
                     && !(initial && initial.getDefiningOp<mlir_ts::SafeCastOp>()));
             if (needsIdentityStorage)

tslang/test/tester/CMakeLists.txt

@@ -471,6 +471,7 @@ tslang_add_test(NAME test-compile-00-global-object-literal COMMAND test-runner "
 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-const-object-literal-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_object_literal_field.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-ternary-new-or-null COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts")
 tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
@@ -959,6 +960,7 @@ tslang_add_test(NAME test-jit-00-global-object-literal COMMAND test-runner -jit
 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-const-object-literal-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_object_literal_field.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-ternary-new-or-null COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts")
 tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
@@ -1775,6 +1777,7 @@ set(TSLANG_CORPUS
     00in.ts
     00global_object_literal.ts
     00infer.ts
+    00const_object_literal_field.ts
     00global_union_from_call.ts
     00instanceof.ts
     00const_of_let.ts

tslang/test/tester/tests/00const_object_literal_field.ts

@@ -0,0 +1,49 @@
+// A `const` object literal keeps what its fields hold (#460). Under rc the const was folded into a
+// read of the literal's temporary slot, which took no reference for a field read out of another
+// object (`{ s: p.child }`), so overwriting `p.child` freed what the literal still held. Each field
+// is read after allocations of the same size have reused anything freed.
+class Node {
+    v = 0;
+    name = "";
+}
+
+class P {
+    child: Node = new Node();
+}
+
+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 main() {
+    const p = new P();
+    p.child.name = "first" + 1;
+    const o = { s: p.child, n: 2 };
+    p.child = new Node();
+    assert(churn() == 1000);
+    assert(o.s.name == "first1" && o.n == 2, "a const literal keeps the field it read");
+
+    let a = new Node();
+    a.name = "local" + 3;
+    const t = { x: a, inner: { y: a } };
+    a = new Node();
+    assert(churn() == 1000);
+    assert(t.x.name == "local3" && t.inner.y.name == "local3", "a const literal keeps a local it read");
+
+    for (let i = 0; i < 1000; i++) {
+        const q = new P();
+        const held = { c: q.child };
+        q.child = new Node();
+        held.c.v = i;
+    }
+
+    print("done.");
+}