diff --git a/tslang/include/TypeScript/Defines.h b/tslang/include/TypeScript/Defines.h index e4d933fcc..2db4725d0 100644 --- a/tslang/include/TypeScript/Defines.h +++ b/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 diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index ca38f6eb0..0b8ef48da 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -6352,6 +6352,8 @@ struct ThisVirtualSymbolRefOpLowering : public TsLlvmPattern(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 auto method = rewriter.create( loc, mlir_ts::FunctionType::get(rewriter.getContext(), boundFunc.getInputs(), boundFunc.getResults()), thisSymbolRefOp.getIdentifierAttr()); - rewriter.replaceOpWithNewOp(thisSymbolRefOp, boundFunc, thisOpaque, method); + auto boundFuncVal = rewriter.replaceOpWithNewOp(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 TsLlvmPatternhasAttr(OWNS_CAPTURE_ATTR_NAME)) { - return nullTag(); + return getBoundObjectTypeTag(createBoundFunctionOp, rewriter); } auto captureRefType = dyn_cast(createBoundFunctionOp.getThisVal().getType()); @@ -6984,6 +6987,43 @@ struct CreateBoundFunctionOpLowering : public TsLlvmPatterngetAttrOfType(BOUND_OBJECT_TYPE_ATTR_NAME); + auto thisType = boundType ? boundType.getValue() : createBoundFunctionOp.getThisVal().getType(); + if (!tsLlvmContext->compileOptions.isRefCounted() || !isa(thisType)) + { + return rewriter.create(loc, th.getPtrType()); + } + + TypeOfOpHelper toh(rewriter); + auto name = toh.typeOfAsString(thisType); + if (name.empty()) + { + return rewriter.create(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 { diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 2f61af6f5..a611bb063 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/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 diff --git a/tslang/test/tester/tests/00bound_method_count.ts b/tslang/test/tester/tests/00bound_method_count.ts new file mode 100644 index 000000000..44cc99c31 --- /dev/null +++ b/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."); +}