rc: a kept bound method counts the object it is bound to

tslang/include/TypeScript/Defines.h

@@ -90,10 +90,16 @@
 
 // Marks a `ts.CreateBoundFunction` whose `this` is a capture box built for it a moment earlier,
 // rather than a receiver that belongs to somebody else. Only such a closure owns its `this`, and
-// only it gets the type tag at CLOSURE_TYPE_INDEX - a bound method must not take ownership of
-// the object it is bound to. Set where the closure is built and the difference is known.
+// gets the box's type tag at CLOSURE_TYPE_INDEX - a bound method does not own the object it is
+// bound to (under rc its receivers count it, see BOUND_OBJECT_TYPE_ATTR_NAME). Set where the
+// closure is built and the difference is known.
 #define OWNS_CAPTURE_ATTR_NAME "__owns_capture"
 
+// On a `ts.CreateBoundFunction` that binds a method to an object: the object's type, which the
+// lowering casts `this` away from. Under rc the bound value counts the object through that type's
+// tag at CLOSURE_TYPE_INDEX, as an interface over it does.
+#define BOUND_OBJECT_TYPE_ATTR_NAME "__bound_object_type"
+
 // Marks the `ts.Variable` that CaptureOpLowering creates for a capture box. It carries
 // `captured = true` only because that is how a variable asks to be allocated in the heap, and
 // without this marker it would be indistinguishable from the cell of a captured variable - and

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -6352,6 +6352,8 @@ struct ThisVirtualSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::ThisVirtua
                 methodPtr);
             auto boundFuncVal =
                 rewriter.create<mlir_ts::CreateBoundFunctionOp>(loc, boundFunc, thisOpaque, methodTyped);
+            boundFuncVal->setAttr(BOUND_OBJECT_TYPE_ATTR_NAME,
+                                  mlir::TypeAttr::get(thisVirtualSymbolRefOp.getThisVal().getType()));
             rewriter.replaceOp(thisVirtualSymbolRefOp, ValueRange{boundFuncVal});
         }
         else
@@ -6390,7 +6392,8 @@ struct ThisSymbolRefOpLowering : public TsLlvmPattern<mlir_ts::ThisSymbolRefOp>
         auto method = rewriter.create<mlir_ts::SymbolRefOp>(
             loc, mlir_ts::FunctionType::get(rewriter.getContext(), boundFunc.getInputs(), boundFunc.getResults()),
             thisSymbolRefOp.getIdentifierAttr());
-        rewriter.replaceOpWithNewOp<mlir_ts::CreateBoundFunctionOp>(thisSymbolRefOp, boundFunc, thisOpaque, method);
+        auto boundFuncVal = rewriter.replaceOpWithNewOp<mlir_ts::CreateBoundFunctionOp>(thisSymbolRefOp, boundFunc, thisOpaque, method);
+        boundFuncVal->setAttr(BOUND_OBJECT_TYPE_ATTR_NAME, mlir::TypeAttr::get(thisSymbolRefOp.getThisVal().getType()));
         return success();
     }
 };
@@ -6924,9 +6927,9 @@ struct CreateBoundFunctionOpLowering : public TsLlvmPattern<mlir_ts::CreateBound
 
     // The runtime type tag of the capture box, for CLOSURE_TYPE_INDEX.
     //
-    // Null unless MLIRGen marked this closure as owning its `this` - a bound method's receiver
-    // belongs to whoever holds the object, and giving the closure a tag for it would have `obj.m`
-    // take a reference to `obj` and hand it back when the bound value dies.
+    // Only for a closure MLIRGen marked as owning its `this`. A bound method's receiver belongs to
+    // whoever holds the object: it gets the object's own tag under rc (getBoundObjectTypeTag), so
+    // a receiver of `obj.m` holds a reference to `obj`, and a null tag elsewhere.
     //
     // The box arrives as a `ref` to its tuple, which owns nothing by itself (a reference into
     // storage is not ownership). What has to be released is the block, so the tag names the
@@ -6941,7 +6944,7 @@ struct CreateBoundFunctionOpLowering : public TsLlvmPattern<mlir_ts::CreateBound
 
         if (!createBoundFunctionOp->hasAttr(OWNS_CAPTURE_ATTR_NAME))
         {
-            return nullTag();
+            return getBoundObjectTypeTag(createBoundFunctionOp, rewriter);
         }
 
         auto captureRefType = dyn_cast<mlir_ts::RefType>(createBoundFunctionOp.getThisVal().getType());
@@ -6984,6 +6987,43 @@ struct CreateBoundFunctionOpLowering : public TsLlvmPattern<mlir_ts::CreateBound
                                                 releaseRoutineName, retainRoutineName, borrows ? "b" : "");
     }
 
+    // Under rc, a method bound to a class object counts that object like an interface over it does
+    // (NewInterfaceOpLowering): the tag is the class's, so a `let m = v.get` - or a field, or an
+    // element - holds a reference to `v`'s object, and `v = d` no longer frees what `m()` runs on.
+    // A bound value is born with no count of its own, as an interface is; only its receivers take
+    // one. Under own a bound method owns nothing (a second reference is an error there), and gc and
+    // none count nothing, so their tag stays null.
+    mlir::Value getBoundObjectTypeTag(mlir_ts::CreateBoundFunctionOp createBoundFunctionOp,
+                                      ConversionPatternRewriter &rewriter) const
+    {
+        TypeHelper th(rewriter);
+        auto loc = createBoundFunctionOp.getLoc();
+        // a method bound to an object has its `this` cast to an opaque pointer, and the object's
+        // type beside it (ThisSymbolRefOpLowering, ThisVirtualSymbolRefOpLowering)
+        auto boundType = createBoundFunctionOp->getAttrOfType<mlir::TypeAttr>(BOUND_OBJECT_TYPE_ATTR_NAME);
+        auto thisType = boundType ? boundType.getValue() : createBoundFunctionOp.getThisVal().getType();
+        if (!tsLlvmContext->compileOptions.isRefCounted() || !isa<mlir_ts::ClassType>(thisType))
+        {
+            return rewriter.create<LLVM::ZeroOp>(loc, th.getPtrType());
+        }
+
+        TypeOfOpHelper toh(rewriter);
+        auto name = toh.typeOfAsString(thisType);
+        if (name.empty())
+        {
+            return rewriter.create<LLVM::ZeroOp>(loc, th.getPtrType());
+        }
+
+        // generated first: the descriptor's initializer takes their addresses
+        OwnershipRoutineLogic orl(createBoundFunctionOp, rewriter, getTypeConverter(), tsLlvmContext->compileOptions);
+        auto releaseRoutineName = orl.getOrCreateReleaseRoutine(thisType);
+        auto retainRoutineName = orl.getOrCreateRetainRoutine(thisType);
+
+        LLVMCodeHelper ch(createBoundFunctionOp, rewriter, getTypeConverter(), tsLlvmContext->compileOptions);
+        return ch.getOrCreateTypeDescriptorName(thisType, name, TypeOfOpHelper::typeKindFromName(name),
+                                                releaseRoutineName, retainRoutineName);
+    }
+
     LogicalResult matchAndRewrite(mlir_ts::CreateBoundFunctionOp createBoundFunctionOp, Adaptor transformed,
                                   ConversionPatternRewriter &rewriter) const final
     {

tslang/test/tester/CMakeLists.txt

@@ -388,6 +388,7 @@ tslang_add_test(NAME test-compile-00-string-empty-falsy COMMAND test-runner "${P
 tslang_add_test(NAME test-compile-00-string-concat-bounds COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_concat_bounds.ts")
 tslang_add_test(NAME test-compile-00-parse-number-range COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00parse_number_range.ts")
 tslang_add_test(NAME test-compile-00-narrowed-assign-other-member COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00narrowed_assign_other_member.ts")
+tslang_add_test(NAME test-compile-00-bound-method-count COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00bound_method_count.ts")
 tslang_add_test(NAME test-compile-00-const-record-owned-fields COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts")
 tslang_add_test(NAME test-compile-00-generator-temporaries COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_temporaries.ts")
 tslang_add_test(NAME test-compile-00-array-length-valid COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
@@ -957,6 +958,7 @@ tslang_add_test(NAME test-jit-00-string-empty-falsy COMMAND test-runner -jit "${
 tslang_add_test(NAME test-jit-00-string-concat-bounds COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_concat_bounds.ts")
 tslang_add_test(NAME test-jit-00-parse-number-range COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00parse_number_range.ts")
 tslang_add_test(NAME test-jit-00-narrowed-assign-other-member COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00narrowed_assign_other_member.ts")
+tslang_add_test(NAME test-jit-00-bound-method-count COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00bound_method_count.ts")
 tslang_add_test(NAME test-jit-00-const-record-owned-fields COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts")
 tslang_add_test(NAME test-jit-00-generator-temporaries COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_temporaries.ts")
 tslang_add_test(NAME test-jit-00-array-length-valid COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
@@ -1916,6 +1918,7 @@ set(TSLANG_CORPUS
     00string_concat_bounds.ts
     00parse_number_range.ts
     00narrowed_assign_other_member.ts
+    00bound_method_count.ts
     00const_record_owned_fields.ts
     00generator_temporaries.ts
     00array_length_valid.ts

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

@@ -0,0 +1,67 @@
+// A method bound to an object and kept - in a `let`, a field, an element - keeps that object alive
+// when the variable it was read from is given a new value. Under rc a bound method carried no type
+// tag, so keeping it counted nothing, and `v = mkv(2)` freed the object `m()` then ran on. Each
+// call follows allocations of the same size, so a freed object is reused.
+class W {
+    constructor(public x: number) {}
+}
+
+class V {
+    items: number[] = [1, 2, 3];
+    constructor(public w: W) {}
+
+    get() {
+        return this.w.x;
+    }
+}
+
+class H {
+    f: () => number = () => 0;
+}
+
+function mkv(n: number) {
+    return new V(new W(n));
+}
+
+function churn(): number {
+    let s = 0;
+    const a: V[] = [];
+    for (let i = 0; i < 64; i++) a.push(mkv(1000 + i));
+    for (const e of a) s += e.w.x;
+    return s;
+}
+
+function inLet(n: number) {
+    let v = mkv(n);
+    let m = v.get;
+    v = mkv(n + 1);
+    churn();
+    return m() == n && v.w.x == n + 1;
+}
+
+function inField(n: number) {
+    let v = mkv(n);
+    const h = new H();
+    h.f = v.get;
+    v = mkv(n + 1);
+    churn();
+    return h.f() == n;
+}
+
+function inElement(n: number) {
+    let v = mkv(n);
+    const methods = [v.get];
+    v = mkv(n + 1);
+    churn();
+    return methods[0]() == n;
+}
+
+function main() {
+    for (let n = 0; n < 20; n++) {
+        assert(inLet(n), "a bound method in a let");
+        assert(inField(n), "a bound method in a field");
+        assert(inElement(n), "a bound method in an element");
+    }
+
+    print("done.");
+}