From 1078f070fb02d675612e6137fec034ac7753fac8 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Fri, 23 Oct 2026 23:02:25 +0100 Subject: [PATCH] -mm=own: `@record` beside `@lends` carries a borrowing record across modules A declaration seen from another module has no body to say whether its result is a borrowing record (own spec 32) - a box the caller owns - or a plain borrow of an argument. The importer took it for a plain borrow and never freed the box. `@record` beside `@lends` says it: - MLIRGen reads it (OWN_DECLARED_RECORD_ATTR_NAME, a unit attribute) on functions and methods; the declaration printer emits it after `@lends`. - The importer's declaration has it: meetFacts and, for a call nothing resolves, calleesDeclare give the call `__own_result_record`, and under own OwnedReturnConsumptionPass counts its result owned (calleeDeclaresRecord: by name, through a dynamic import's global, or as a method), so the box is released where the caller lets it go. Before, no release was emitted at all. - checkDeclared: `@record` without `@lends`, or on a body that returns a borrow as it is, is an error; an exported function of a library whose body returns a record has to say it. An override says the same `@record`. Guessing record-ness from the result type or from several `@lends` names is not safe: a caller told it owns a record would free an argument. Spec 32 updated. Tests: export_own_record / import_own_record (-shared, gc rc own), import_own_record_outlives, own-shared-record.cmake (the importer releases each box), three negatives; own_lends's exported pairOf says @record. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-24-own-memory-model-design.md | 23 ++++++- tslang/include/TypeScript/Defines.h | 6 ++ .../MLIRLogic/MLIRDeclarationPrinter.h | 5 +- .../TypeScript/MLIRLogic/MLIRGenStore.h | 2 + tslang/lib/TypeScript/DeclarationPrinter.cpp | 18 ++++-- tslang/lib/TypeScript/LowerToLLVM.cpp | 2 +- tslang/lib/TypeScript/MLIRGenFunctions.cpp | 2 +- tslang/lib/TypeScript/MLIRGenImpl.h | 27 +++++++- .../TypeScript/OwnedReturnConsumptionPass.cpp | 59 ++++++++++++++++- .../lib/TypeScript/OwnershipSignaturePass.cpp | 64 ++++++++++++++++--- .../lib/TypeScript/OwnershipSignaturePass.h | 3 + tslang/test/tester/CMakeLists.txt | 29 ++++++++- tslang/test/tester/own-shared-record.cmake | 46 +++++++++++++ .../own/own_err_record_exported_unsaid.ts | 15 +++++ .../tester/own/own_err_record_not_record.ts | 16 +++++ .../own/own_err_record_without_lends.ts | 20 ++++++ tslang/test/tester/own/own_lends.ts | 1 + tslang/test/tester/tests/export_own_record.ts | 29 +++++++++ tslang/test/tester/tests/import_own_record.ts | 25 ++++++++ .../tests/import_own_record_outlives.ts | 11 ++++ 20 files changed, 378 insertions(+), 25 deletions(-) create mode 100644 tslang/test/tester/own-shared-record.cmake create mode 100644 tslang/test/tester/own/own_err_record_exported_unsaid.ts create mode 100644 tslang/test/tester/own/own_err_record_not_record.ts create mode 100644 tslang/test/tester/own/own_err_record_without_lends.ts create mode 100644 tslang/test/tester/tests/export_own_record.ts create mode 100644 tslang/test/tester/tests/import_own_record.ts create mode 100644 tslang/test/tester/tests/import_own_record_outlives.ts diff --git a/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md b/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md index 77b533738..0fc894dff 100644 --- a/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md +++ b/tslang/docs/superpowers/specs/2026-09-24-own-memory-model-design.md @@ -3887,9 +3887,26 @@ and the same errors as a call's result that borrows its arguments.) parameter's, `this`'s, or a read out of one: those parameters are the set. A record on one path and anything else on another (a plain borrow, a fresh object) gives no facts. `closest` returns `bestHit`, which holds `sc.ts[i].hit(ray)`'s records: its result borrows `ray` and `sc`. - `@lends` may name several parameters: `@lends("a", "b") pairOf(a, b): Pair { return { a: a, b: b } - }` (`own_lends`). A declaration's `@lends`, seen from another module without its body, is a plain - borrow: a record's box then leaks at that caller, and nothing is freed twice. + `@lends` may name several parameters: `@lends("a", "b") @record pairOf(a, b): Pair { return { a: a, + b: b } }` (`own_lends`). +- **`@record` (USER decision 2026-10-23).** A declaration seen from another module has no body to + say whether the result is a record, so `@record` beside `@lends` says it + (`OWN_DECLARED_RECORD_ATTR_NAME`, a unit attribute). The declaration printer emits it after + `@lends`; the importer's declaration gets it, and its calls get `__own_result_record` + (`meetFacts`, and `calleesDeclare` for a call no function resolves). Under own the importer's + OwnedReturnConsumptionPass counts such a call's result owned (`calleeDeclaresRecord`: by name, + through a dynamic import's global, or as a method), so the box is released where the caller + lets it go. Before, the importer took the result for a plain borrow and the box leaked; nothing + was freed twice. Guessing record-ness from the result type, or from `@lends` naming several + parameters, is not safe: a plain borrow of an interface argument looks the same, and a caller + told it owns a record would free that argument. The defining module checks the marker against + its body (`checkDeclared`): `@record` without `@lends` is an error, `@record` on a body that + returns a borrow as it is is an error, and a function another module can call (exported, in a + module that is not an executable) whose body returns a record has to say `@record`. A function + no other module calls need not: its callers go by its body. An override says the same + `@record` as what it overrides. Tests: `export_own_record` / `import_own_record` (-shared, every + model), `import_own_record_outlives`, `own-shared-record.cmake` (the importer releases each box), + `own_err_record_without_lends`, `own_err_record_not_record`, `own_err_record_exported_unsaid`. - **Families** meet by agreement, as before (`meetFacts`): members whose sets or record-ness differ are demoted. The union of the sets would be sound for records (the caller would bound the result by more arguments), but not across a record and a plain borrow, whose callers release different diff --git a/tslang/include/TypeScript/Defines.h b/tslang/include/TypeScript/Defines.h index 7d4a5f10d..dc8aab927 100644 --- a/tslang/include/TypeScript/Defines.h +++ b/tslang/include/TypeScript/Defines.h @@ -299,6 +299,12 @@ #define OWN_DECLARED_KEEPS_ATTR_NAME "ts.own_declared_keeps" #define OWN_DECLARED_BORROWS_ATTR_NAME "ts.own_declared_borrows" #define OWN_DECLARED_LENDS_ATTR_NAME "ts.own_declared_lends" +// `@record`, beside `@lends`: the result is a borrowing record (own spec 32) - a box the caller owns, +// holding a borrow of each argument `@lends` names - not a borrow of one of them. An importer, which +// sees the declaration without its body, learns it only from this; a function another module can call +// that returns such a record has to say it. A unit attribute on the FuncOp. +#define RECORD_DECORATOR "record" +#define OWN_DECLARED_RECORD_ATTR_NAME "ts.own_declared_record" // the names of the arguments of a function that says any, for -mm=own's errors about it #define OWN_DECLARED_PARAM_NAMES_ATTR_NAME "ts.own_declared_param_names" diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h b/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h index ae1364bdf..3b9e139ff 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h @@ -24,7 +24,8 @@ namespace typescript void printEnum(StringRef, NamespaceInfo::TypePtr, mlir::DictionaryAttr); void printVariableDeclaration(StringRef, NamespaceInfo::TypePtr, mlir::Type, bool, StringRef = StringRef()); void print(StringRef, NamespaceInfo::TypePtr, mlir_ts::FunctionType, StringRef = StringRef(), - ArrayRef ownKeeps = {}, ArrayRef ownBorrows = {}, ArrayRef ownLends = {}); + ArrayRef ownKeeps = {}, ArrayRef ownBorrows = {}, ArrayRef ownLends = {}, + bool ownRecord = false); void print(ClassInfo::TypePtr); void print(InterfaceInfo::TypePtr); void printGenericClass(NamespaceInfo::TypePtr, StringRef); @@ -40,7 +41,7 @@ namespace typescript bool filterName(StringRef); bool filterField(mlir::Attribute); void printParams(ArrayRef, mlir::Type); - void printOwnMarkers(ArrayRef, ArrayRef, ArrayRef, bool, unsigned); + void printOwnMarkers(ArrayRef, ArrayRef, ArrayRef, bool, bool, unsigned); void printFunction(StringRef, ArrayRef, mlir::Type); void printMethod(bool, StringRef, ArrayRef, mlir::Type, mlir::Type); void printAccessor(bool, StringRef, StringRef, mlir_ts::AccessLevel, ArrayRef, mlir::Type, mlir::Type, StringRef); diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h index c13644b33..29b17de72 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h @@ -170,6 +170,8 @@ struct MethodInfo std::vector ownKeeps; std::vector ownBorrows; uint64_t ownLendsMask = 0; + // `@record` (OWN_DECLARED_RECORD_ATTR_NAME): what `@lends` names, the result is a record of + bool ownRecord = false; }; struct GenericMethodInfo diff --git a/tslang/lib/TypeScript/DeclarationPrinter.cpp b/tslang/lib/TypeScript/DeclarationPrinter.cpp index 57bf0d2da..57d435cc9 100644 --- a/tslang/lib/TypeScript/DeclarationPrinter.cpp +++ b/tslang/lib/TypeScript/DeclarationPrinter.cpp @@ -229,9 +229,10 @@ namespace typescript } // `@keeps("p1")`, `@borrows("this", "p1")`: by the names printParams gives the parameters, `this` - // the receiver of a method, so the importer reads the same argument indices back + // the receiver of a method, so the importer reads the same argument indices back. `@record` after + // `@lends`: the importer's only way to know the result is a record it owns (own spec 32). void MLIRDeclarationPrinter::printOwnMarkers(ArrayRef keeps, ArrayRef borrows, ArrayRef lends, - bool hasThis, unsigned indent) + bool record, bool hasThis, unsigned indent) { auto printMarker = [&](StringRef name, ArrayRef indices) { if (indices.empty()) @@ -263,6 +264,12 @@ namespace typescript printMarker(KEEPS_DECORATOR, keeps); printMarker(BORROWS_DECORATOR, borrows); printMarker(LENDS_DECORATOR, lends); + if (record && !lends.empty()) + { + os.indent(indent); + os << "@" RECORD_DECORATOR; + newline(); + } } void MLIRDeclarationPrinter::printFunction(StringRef name, ArrayRef params, mlir::Type returnType) @@ -440,7 +447,8 @@ namespace typescript } void MLIRDeclarationPrinter::print(StringRef name, NamespaceInfo::TypePtr elementNamespace, mlir_ts::FunctionType funcType, StringRef dllName, - ArrayRef ownKeeps, ArrayRef ownBorrows, ArrayRef ownLends) + ArrayRef ownKeeps, ArrayRef ownBorrows, ArrayRef ownLends, + bool ownRecord) { printNamespaceBegin(elementNamespace); @@ -452,7 +460,7 @@ namespace typescript newline(); } - printOwnMarkers(ownKeeps, ownBorrows, ownLends, /*hasThis=*/false, 0); + printOwnMarkers(ownKeeps, ownBorrows, ownLends, ownRecord, /*hasThis=*/false, 0); printFunction( name, @@ -710,7 +718,7 @@ namespace typescript } } - printOwnMarkers(method.ownKeeps, method.ownBorrows, lends, /*hasThis=*/!method.isStatic, 4); + printOwnMarkers(method.ownKeeps, method.ownBorrows, lends, method.ownRecord, /*hasThis=*/!method.isStatic, 4); // the slot of a @cpp class's virtual method: without it the importer takes the method // for a non-virtual one diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index fe9c3de42..c62d8fc07 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -2072,7 +2072,7 @@ struct FuncOpLowering : public TsLlvmPattern // read by -mm=own only if (name == OWN_DECLARED_NO_DROPS_ATTR_NAME || name == OWN_DECLARED_KEEPS_ATTR_NAME || name == OWN_DECLARED_BORROWS_ATTR_NAME || name == OWN_DECLARED_LENDS_ATTR_NAME || - name == OWN_DECLARED_PARAM_NAMES_ATTR_NAME) { + name == OWN_DECLARED_RECORD_ATTR_NAME || name == OWN_DECLARED_PARAM_NAMES_ATTR_NAME) { continue; } diff --git a/tslang/lib/TypeScript/MLIRGenFunctions.cpp b/tslang/lib/TypeScript/MLIRGenFunctions.cpp index 9e293c0c0..065c9387a 100644 --- a/tslang/lib/TypeScript/MLIRGenFunctions.cpp +++ b/tslang/lib/TypeScript/MLIRGenFunctions.cpp @@ -1288,7 +1288,7 @@ namespace mlirgen if (auto globalOp = theModule.lookupSymbol(globalName)) { for (auto *name : {OWN_DECLARED_KEEPS_ATTR_NAME, OWN_DECLARED_BORROWS_ATTR_NAME, OWN_DECLARED_LENDS_ATTR_NAME, - OWN_DECLARED_PARAM_NAMES_ATTR_NAME}) + OWN_DECLARED_RECORD_ATTR_NAME, OWN_DECLARED_PARAM_NAMES_ATTR_NAME}) { if (auto attr = declared->getAttr(name)) { diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index d2a4f340a..fa81fb6cc 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -3293,6 +3293,28 @@ class MLIRGenImpl { auto result = mlir::success(); iterateDecorators(functionLikeDeclarationBaseAST, genContext, [&](StringRef name, SmallVector args) { + // `@record` names nothing: it says what `@lends` lends into (OWN_DECLARED_RECORD_ATTR_NAME) + if (name == RECORD_DECORATOR) + { + if (llvm::any_of(attrs, [](mlir::NamedAttribute &attr) { return attr.getName() == OWN_DECLARED_RECORD_ATTR_NAME; })) + { + emitError(location) << "'@" RECORD_DECORATOR "' is given twice"; + result = mlir::failure(); + return; + } + + if (!args.empty()) + { + emitError(location) << "'@" RECORD_DECORATOR "' names no parameter: '@" LENDS_DECORATOR + "' names those the record borrows"; + result = mlir::failure(); + return; + } + + attrs.push_back({builder.getStringAttr(OWN_DECLARED_RECORD_ATTR_NAME), builder.getUnitAttr()}); + return; + } + if (name != KEEPS_DECORATOR && name != BORROWS_DECORATOR && name != LENDS_DECORATOR) { return; @@ -12807,6 +12829,8 @@ class MLIRGenImpl } } } + + methodInfo.ownRecord = funcOp->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME); } if (propertyName.size() > 0) @@ -14182,7 +14206,8 @@ class MLIRGenImpl return attr ? attr.asArrayRef() : ArrayRef(); }; dp.print(funcProto->getNameWithoutNamespace(), elementNamespace, funcProto->getFuncType(), dllName, - marker(OWN_DECLARED_KEEPS_ATTR_NAME), marker(OWN_DECLARED_BORROWS_ATTR_NAME), marker(OWN_DECLARED_LENDS_ATTR_NAME)); + marker(OWN_DECLARED_KEEPS_ATTR_NAME), marker(OWN_DECLARED_BORROWS_ATTR_NAME), marker(OWN_DECLARED_LENDS_ATTR_NAME), + funcOp && funcOp->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME)); declExports << ss.str().str(); } diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index b86781937..17eae3895 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -153,9 +153,23 @@ class OwnedReturnConsumptionPass }); }); + // -mm=own: a function another module defines, whose declaration says `@record`, returns a box + // its caller owns (own spec 32) - the one result of an import this pass can count as owned + auto own = compileOptions.memoryModel == MemoryModelOwn; + auto anyDeclaredRecord = false; + if (own) + { + module.walk([&](mlir::Operation *op) { + if (mlir::isa(op) && op->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME)) + { + anyDeclaredRecord = true; + } + }); + } + // `new C()` is marked where it is built, so there can be discarded temporaries to give // back even when no function here is classified as returning owned. - if (returnsOwned.empty()) + if (returnsOwned.empty() && !anyDeclaredRecord) { claimGlobalInitializers(mth, module); releaseDiscardedTemporaries(mth, module); @@ -176,7 +190,8 @@ class OwnedReturnConsumptionPass } auto callee = calleeNameOf(callOp, fieldFunctions); - auto calleeReturnsOwned = !callee.empty() && returnsOwned.contains(callee); + auto calleeReturnsOwned = (!callee.empty() && returnsOwned.contains(callee)) || + (anyDeclaredRecord && calleeDeclaresRecord(module, callOp)); // A call through a plain value - a callback handed to `reduce`, a function-typed // field - names nothing at all, and neither vtables nor member names help. What does @@ -2137,6 +2152,46 @@ class OwnedReturnConsumptionPass // canonicalizer (SimplifyIndirectCallWithKnownCallee) already rewrites into direct calls, // which is the argument that they name one callee: it is the same judgement, made here // before canonicalization has run. + // Does the callee say `@record` (OWN_DECLARED_RECORD_ATTR_NAME)? By name, through the global a + // dynamically imported function is read from, or as a method - virtual or not: an override says + // the same markers as what it overrides (OwnershipSignaturePass::checkDeclared). + static bool calleeDeclaresRecord(mlir::ModuleOp module, mlir_ts::CallIndirectOp callOp) + { + if (callOp.getNumOperands() == 0) + { + return false; + } + + auto callee = callOp.getOperand(0); + mlir::StringRef name; + if (auto symbolRefOp = callee.getDefiningOp()) + { + name = symbolRefOp.getIdentifier(); + } + else if (auto loadOp = callee.getDefiningOp()) + { + if (auto addressOfOp = loadOp.getReference().getDefiningOp()) + { + name = addressOfOp.getGlobalName(); + } + } + else if (auto getMethodOp = callee.getDefiningOp()) + { + auto *boundOp = getMethodOp.getBoundFunc().getDefiningOp(); + if (auto thisSymbolRefOp = mlir::dyn_cast_or_null(boundOp)) + { + name = thisSymbolRefOp.getIdentifier(); + } + else if (auto thisVirtualSymbolRefOp = mlir::dyn_cast_or_null(boundOp)) + { + name = thisVirtualSymbolRefOp.getIdentifier(); + } + } + + auto *symbol = name.empty() ? nullptr : mlir::SymbolTable::lookupSymbolIn(module, name); + return symbol && symbol->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME); + } + static mlir::StringRef calleeNameOf(mlir_ts::CallIndirectOp callOp, const own_facts::FieldFunctions *fieldFunctions) { if (callOp.getNumOperands() == 0) diff --git a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp index e6b3b889f..c86777a50 100644 --- a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp +++ b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp @@ -272,6 +272,19 @@ namespace own_signature return sorted; } + bool OwnershipSignaturePass::calleesDeclare(const Callees &callees, llvm::StringRef name) + { + return !callees.named.empty() && llvm::all_of(callees.named, [&](mlir::StringAttr symbol) { + mlir::Operation *declaring = functions.lookup(symbol.getValue()); + if (!declaring) + { + declaring = globals.lookup(symbol.getValue()); + } + + return declaring && declaring->hasAttr(name); + }); + } + llvm::SmallVector OwnershipSignaturePass::declaredByCallees(const Callees &callees, llvm::StringRef name) { llvm::SmallVector agreed; @@ -380,6 +393,15 @@ namespace own_signature continue; } + if (funcOp->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME) && !funcOp->hasAttr(OWN_DECLARED_LENDS_ATTR_NAME)) + { + funcOp.emitError("'@" RECORD_DECORATOR "' of '") + << funcOp.getSymName() + << "' says its result is a borrowing record, but no '@" LENDS_DECORATOR "' names what it borrows"; + signalPassFailure(); + continue; + } + if (declaresCallerFacts(funcOp)) { auto local = factsLocal.lookup(funcOp); @@ -424,6 +446,28 @@ namespace own_signature signalPassFailure(); } + else if (!lends.empty() && funcOp->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME) != local.record) + { + // An importer knows the result is a record only from `@record` (spec 32): told a plain + // borrow, it never frees the box; told a record, it frees an argument it does not own. + // Callers in this module go by the body, so a function no other module calls need + // not say it. + if (funcOp->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME)) + { + funcOp.emitError("'@" RECORD_DECORATOR "' of '") + << funcOp.getSymName() << "' says its result is a record that borrows " + << describeParams(funcOp, lends) << ", but its body returns a borrow of it as it is"; + signalPassFailure(); + } + else if (!executable && (funcOp->hasAttr("export") || funcOp->hasAttr(DLL_EXPORT))) + { + funcOp.emitError("'") + << funcOp.getSymName() << "' returns a record that borrows " << describeParams(funcOp, lends) + << ", which another module can learn only from '@" RECORD_DECORATOR "' beside its '@" LENDS_DECORATOR + "'"; + signalPassFailure(); + } + } } if (funcOp->hasAttr(OWN_DECLARED_BORROWS_ATTR_NAME)) @@ -473,13 +517,14 @@ namespace own_signature continue; } - for (auto *name : {OWN_DECLARED_KEEPS_ATTR_NAME, OWN_DECLARED_BORROWS_ATTR_NAME, OWN_DECLARED_LENDS_ATTR_NAME}) + for (auto *name : {OWN_DECLARED_KEEPS_ATTR_NAME, OWN_DECLARED_BORROWS_ATTR_NAME, OWN_DECLARED_LENDS_ATTR_NAME, + OWN_DECLARED_RECORD_ATTR_NAME}) { if (funcOp->hasAttr(name) != declaring->hasAttr(name) || declared(funcOp, name) != declared(declaring, name)) { funcOp.emitError("'") << funcOp.getSymName() << "' overrides '" << declaring.getSymName() << "', and has to say the same '@" KEEPS_DECORATOR "', '@" BORROWS_DECORATOR - "' and '@" LENDS_DECORATOR "'"; + "', '@" LENDS_DECORATOR "' and '@" RECORD_DECORATOR "'"; signalPassFailure(); break; } @@ -782,10 +827,9 @@ namespace own_signature auto facts = callFacts(call.callees); if (facts.empty()) { - // a call this pass cannot resolve still has what its callees' signatures say - // A declaration's `@lends` is a plain borrow to its callers: its body, which would - // say whether the result is a record the caller owns (spec 32), is not here. A - // record's box then leaks, and nothing is freed twice. + // a call this pass cannot resolve still has what its callees' signatures say. + // A declaration's `@lends` is a plain borrow to its callers, and a record the + // caller owns (spec 32) with `@record`: its body is not here to say which. auto keeps = declaredByCallees(call.callees, OWN_DECLARED_KEEPS_ATTR_NAME); facts.owned.assign(keeps.begin(), keeps.end()); if (auto lends = declaredByCallees(call.callees, OWN_DECLARED_LENDS_ATTR_NAME); !lends.empty()) @@ -793,6 +837,7 @@ namespace own_signature llvm::sort(lends); lends.erase(std::unique(lends.begin(), lends.end()), lends.end()); facts.borrows = lends; + facts.record = calleesDeclare(call.callees, OWN_DECLARED_RECORD_ATTR_NAME); } } @@ -838,9 +883,12 @@ namespace own_signature said.borrows = declaredLends(entry.second); said.owned.assign(keeps.begin(), keeps.end()); // a body here that returns a borrowing record of what `@lends` names: callers in this - // module own the box (spec 32); checkDeclared holds the body to the parameters + // module own the box (spec 32); checkDeclared holds the body to the parameters. A + // declaration has no body: its `@record` says so. auto local = factsLocal.lookup(entry.second); - said.record = !said.borrows.empty() && local.record && local.borrows == said.borrows; + said.record = !said.borrows.empty() && + (entry.second.isDeclaration() ? entry.second->hasAttr(OWN_DECLARED_RECORD_ATTR_NAME) + : local.record && local.borrows == said.borrows); if (factsThroughInterface(entry.second, said)) { continue; diff --git a/tslang/lib/TypeScript/OwnershipSignaturePass.h b/tslang/lib/TypeScript/OwnershipSignaturePass.h index 6f60cb09f..0139b33e2 100644 --- a/tslang/lib/TypeScript/OwnershipSignaturePass.h +++ b/tslang/lib/TypeScript/OwnershipSignaturePass.h @@ -446,6 +446,9 @@ class OwnershipSignaturePass : public mlir::PassWrapper declaredByCallees(const Callees &callees, llvm::StringRef name); + // Does every function a call names declare the unit marker `name` (`@record`)? + bool calleesDeclare(const Callees &callees, llvm::StringRef name); + // The parameters, for an error, by the names MLIRGen recorded beside the marker. static std::string describeParams(mlir_ts::FuncOp funcOp, llvm::ArrayRef indices); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 5f52cdbc2..6293bb582 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -1608,6 +1608,28 @@ add_test(NAME test-compile-shared-import-own-markers-lent-outlives COMMAND test-runner -shared -mm=own "${PROJECT_SOURCE_DIR}/test/tester/tests/import_own_markers_lent_outlives.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_own_markers.ts") set_tests_properties(test-compile-shared-import-own-markers-lent-outlives PROPERTIES PASS_REGULAR_EXPRESSION "borrows a call's result but is used here after it may be released or overwritten" FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed") +# @record across the boundary (own spec 32): the importer owns the box a borrowing record's +# function returns, and knows it only from the declaration's @record +foreach(record_mm gc rc own) + tslang_add_test(NAME test-compile-shared-export-import-own-record-${record_mm} COMMAND test-runner -shared -mm=${record_mm} "${PROJECT_SOURCE_DIR}/test/tester/tests/import_own_record.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_own_record.ts") + tslang_add_test(NAME test-jit-shared-export-import-own-record-${record_mm} COMMAND test-runner -jit -shared -mm=${record_mm} "${PROJECT_SOURCE_DIR}/test/tester/tests/import_own_record.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_own_record.ts") +endforeach() +# ...a read of the record after an argument it borrows is given a new value is refused +add_test(NAME test-compile-shared-import-own-record-outlives + COMMAND test-runner -shared -mm=own "${PROJECT_SOURCE_DIR}/test/tester/tests/import_own_record_outlives.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_own_record.ts") +set_tests_properties(test-compile-shared-import-own-record-outlives PROPERTIES PASS_REGULAR_EXPRESSION "borrows a call's result but is used here after it may be released or overwritten" + FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed") +# ...and the importer releases each box, which no run can see (own-shared-record.cmake) +if (WIN32) + add_test(NAME test-own-shared-record-released + COMMAND ${CMAKE_COMMAND} + "-DTSLANG=$" + "-DSOURCE_DIR=${PROJECT_SOURCE_DIR}/test/tester/tests" + "-DWORK_DIR=${CMAKE_CURRENT_BINARY_DIR}/own-shared-record" + "-DLLVM_LIB=${LLVM_LIBRARY_DIR}" + "-DTSLANG_LIB=${CMAKE_BINARY_DIR}/lib" + -P "${CMAKE_CURRENT_SOURCE_DIR}/own-shared-record.cmake") +endif() # the markers mean nothing to the other models foreach(markers_mm gc rc) tslang_add_test(NAME test-jit-${markers_mm}-own-generator-locals COMMAND test-runner -jit -mm=${markers_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/own_generator_locals.ts") @@ -4170,7 +4192,7 @@ set(own_error_cases "own_err_borrows_not_generator|'@borrows' of 'copyOf' names 'w', but it neither makes a generator whose state borrows its arguments nor returns one" "own_err_borrows_mixed_return|'@borrows' of 'pick' names 'w', but it neither makes a generator whose state borrows its arguments nor returns one" "own_err_marker_unknown_param|'@keeps' names 'd', which is not a parameter of this function" - "own_err_marker_override|'Derived.set' overrides 'Base.set', and has to say the same '@keeps', '@borrows' and '@lends'" + "own_err_marker_override|'Derived.set' overrides 'Base.set', and has to say the same '@keeps', '@borrows', '@lends' and '@record'" "own_err_lends_missing|'firstOf' could return a borrow or keep a parameter, but it can be called from another module. '@lends' or '@keeps' would say it in its signature" "own_err_lends_mismatch|'@lends' of 'pick' names 'a', but its body returns a borrow of 'b'" "own_err_lends_not_borrow|'@lends' of 'made' names 'l', but its body does not return a borrow of it" @@ -4228,7 +4250,10 @@ set(own_error_cases "own_err_borrowing_record_open|:17:31: error: '[^']*' borrows '[^']*' and cannot be stored, returned or captured" "own_err_borrowing_record_borrowed_again|:29:11: error: '[^']*' borrows the result of 'A[.]hit' but is used here after it may be released or overwritten" "own_err_destructure_element_reassigned|:10:11: error: '[^']*' borrows an element but is used here after it may be released or overwritten" - "own_err_destructure_element_returned|:9:5: error: '[^']*' borrows an element and cannot be stored, returned or captured") + "own_err_destructure_element_returned|:9:5: error: '[^']*' borrows an element and cannot be stored, returned or captured" + "own_err_record_without_lends|:11:1: error: '@record' of 'pairOf' says its result is a borrowing record, but no '@lends' names what it borrows" + "own_err_record_not_record|:7:1: error: '@record' of 'first' says its result is a record that borrows 'a', but its body returns a borrow of it as it is" + "own_err_record_exported_unsaid|:12:1: error: 'pairOf' returns a record that borrows 'a', 'b', which another module can learn only from '@record' beside its '@lends'") foreach(own_error_case ${own_error_cases}) string(FIND "${own_error_case}" "|" own_error_sep) string(SUBSTRING "${own_error_case}" 0 ${own_error_sep} own_error_name) diff --git a/tslang/test/tester/own-shared-record.cmake b/tslang/test/tester/own-shared-record.cmake new file mode 100644 index 000000000..e07377fe6 --- /dev/null +++ b/tslang/test/tester/own-shared-record.cmake @@ -0,0 +1,46 @@ +# -mm=own across a DLL boundary: a function another module defines that says `@record` returns a box +# its caller owns (own spec 32), which the importer learns only from the declaration it re-parses. +# test-runner's -shared mode runs the pair; this checks what a run cannot see - that the importer +# releases the box. Without `@record` it took the result for a plain borrow and never freed it. + +cmake_minimum_required(VERSION 3.17.3) + +foreach(var TSLANG SOURCE_DIR WORK_DIR LLVM_LIB TSLANG_LIB) + if(NOT DEFINED ${var}) + message(FATAL_ERROR "${var} is required") + endif() +endforeach() + +set(ENV{GC_LIB_PATH} "") +set(ENV{TSLANG_LIB_PATH} "") + +file(REMOVE_RECURSE "${WORK_DIR}") +file(MAKE_DIRECTORY "${WORK_DIR}") + +execute_process(COMMAND "${TSLANG}" --emit=dll -mm=own --no-default-lib + "--llvm-lib-path=${LLVM_LIB}" "--tslang-lib-path=${TSLANG_LIB}" + "${SOURCE_DIR}/export_own_record.ts" -o export_own_record.dll + WORKING_DIRECTORY "${WORK_DIR}" + OUTPUT_VARIABLE out + ERROR_VARIABLE err + RESULT_VARIABLE status) +if(NOT status EQUAL 0) + message(FATAL_ERROR "--emit=dll export_own_record: exit ${status}\n${out}\n${err}") +endif() + +# `import './export_own_record'` finds the DLL in the working directory before the source +execute_process(COMMAND "${TSLANG}" --emit=mlir-affine -mm=own --no-default-lib "${SOURCE_DIR}/import_own_record.ts" + WORKING_DIRECTORY "${WORK_DIR}" + OUTPUT_VARIABLE out + ERROR_VARIABLE err + RESULT_VARIABLE status) +if(NOT status EQUAL 0 OR "${out}${err}" MATCHES "error:|Stack dump") + message(FATAL_ERROR "import_own_record: exit ${status}\n${out}\n${err}") +endif() + +# one release of a Pair box in each loop +string(REGEX MATCHALL "\"ts.Release\"[(]%[0-9]+[)] : [(]!ts.iface<@Pair>[)]" released "${out}${err}") +list(LENGTH released count) +if(NOT count EQUAL 2) + message(FATAL_ERROR "import_own_record: ${count} releases of a Pair box, expected 2\n${out}${err}") +endif() diff --git a/tslang/test/tester/own/own_err_record_exported_unsaid.ts b/tslang/test/tester/own/own_err_record_exported_unsaid.ts new file mode 100644 index 000000000..29adbfab3 --- /dev/null +++ b/tslang/test/tester/own/own_err_record_exported_unsaid.ts @@ -0,0 +1,15 @@ +// An exported function returning a borrowing record has to say `@record`: another module, which sees +// only its declaration, would take the result for a plain borrow and never free the box. +export class Node { + constructor(public v: number) {} +} + +export interface Pair { + a: Node; + b: Node; +} + +@lends("a", "b") +export function pairOf(a: Node, b: Node): Pair { + return { a: a, b: b }; +} diff --git a/tslang/test/tester/own/own_err_record_not_record.ts b/tslang/test/tester/own/own_err_record_not_record.ts new file mode 100644 index 000000000..ca3b51602 --- /dev/null +++ b/tslang/test/tester/own/own_err_record_not_record.ts @@ -0,0 +1,16 @@ +// `@record` says the caller owns a box, but `first` returns its argument itself: a caller told so +// would free the argument. +class Node { + constructor(public v: number) {} +} + +@lends("a") +@record +function first(a: Node): Node { + return a; +} + +function main() { + const a = new Node(1); + print(first(a).v); +} diff --git a/tslang/test/tester/own/own_err_record_without_lends.ts b/tslang/test/tester/own/own_err_record_without_lends.ts new file mode 100644 index 000000000..79dd5b0d0 --- /dev/null +++ b/tslang/test/tester/own/own_err_record_without_lends.ts @@ -0,0 +1,20 @@ +// `@record` says what `@lends` lends into: alone it names nothing. +class Node { + constructor(public v: number) {} +} + +interface Pair { + a: Node; + b: Node; +} + +@record +function pairOf(a: Node, b: Node): Pair { + return { a: a, b: b }; +} + +function main() { + const a = new Node(1); + const b = new Node(2); + print(pairOf(a, b).a.v); +} diff --git a/tslang/test/tester/own/own_lends.ts b/tslang/test/tester/own/own_lends.ts index 9434662bf..e363a9771 100644 --- a/tslang/test/tester/own/own_lends.ts +++ b/tslang/test/tester/own/own_lends.ts @@ -44,6 +44,7 @@ export interface Pair { } @lends("a", "b") +@record export function pairOf(a: Node, b: Node): Pair { return { a: a, b: b }; } diff --git a/tslang/test/tester/tests/export_own_record.ts b/tslang/test/tester/tests/export_own_record.ts new file mode 100644 index 000000000..035047165 --- /dev/null +++ b/tslang/test/tester/tests/export_own_record.ts @@ -0,0 +1,29 @@ +// The library side of @record across a module boundary (own spec 32): `pairOf` and `Maker.pairWith` +// return a borrowing record - a box their caller owns, holding a borrow of each argument `@lends` +// names. The importer sees only these declarations; `@record` tells it the box is its to release. +// See import_own_record.ts. +export class Node { + constructor(public v: number) {} +} + +export interface Pair { + a: Node; + b: Node; +} + +@lends("a", "b") +@record +export function pairOf(a: Node, b: Node): Pair { + return { a: a, b: b }; +} + +@final +export class Maker { + base = new Node(5); + + @lends("this", "n") + @record + pairWith(n: Node): Pair { + return { a: this.base, b: n }; + } +} diff --git a/tslang/test/tester/tests/import_own_record.ts b/tslang/test/tester/tests/import_own_record.ts new file mode 100644 index 000000000..c72e98179 --- /dev/null +++ b/tslang/test/tester/tests/import_own_record.ts @@ -0,0 +1,25 @@ +// A borrowing record from another module (export_own_record.ts): the importer owns each box +// `pairOf` and `pairWith` return, and releases it at the end of each iteration, leaving the borrowed +// nodes to their owners. Each loop calls one of them: a call to another module's method may destroy +// what it can reach, so a record cannot be read across one. Under gc and rc the markers mean nothing. +import './export_own_record' + +function main() { + const a = new Node(1); + const b = new Node(2); + let sum = 0; + for (let i = 0; i < 1000; i++) { + const p = pairOf(a, b); + sum += p.a.v + p.b.v; + } + + const m = new Maker(); + for (let i = 0; i < 1000; i++) { + const q = m.pairWith(a); + sum += q.a.v + q.b.v; + } + + assert(sum == 9000, "records across the boundary"); + assert(a.v == 1 && b.v == 2 && m.base.v == 5, "the borrowed nodes outlive the records"); + print("done."); +} diff --git a/tslang/test/tester/tests/import_own_record_outlives.ts b/tslang/test/tester/tests/import_own_record_outlives.ts new file mode 100644 index 000000000..b39754c00 --- /dev/null +++ b/tslang/test/tester/tests/import_own_record_outlives.ts @@ -0,0 +1,11 @@ +// -mm=own: the record `pairOf` returns (export_own_record.ts) borrows `a`, so a read of it after `a` +// is given a new value is refused. +import './export_own_record' + +function main() { + let a = new Node(1); + const b = new Node(2); + const p = pairOf(a, b); + a = new Node(3); + print(p.a.v); +}