From 3d2cc716ee5f0fb220b92c4fe8b705c6cc70652c Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Mon, 28 Sep 2026 11:20:08 +0100 Subject: [PATCH] A -shared library's class is imported once, with the library's vtable Two bugs in importing a class from a -shared library, found through the class import cycle whose -shared test was left disabled; neither needs a cycle. - A module whose export depends on a class of another module declares that class in its __decls too (a derived class's base), so an importer of a library holding both read it twice: "redefinition of symbol named 'Base..new'". A second declaration of an imported class already processed in this module is now skipped (ClassInfo::processingDeclaration tells it from the same declaration generated again). - The declaration printer wrote accessors after all methods. The importer builds the vtable in the order it reads the members, so with `get textContent()` written before `addText()` their slots were swapped and the importer's `addText` call ran the getter. Accessors are now printed where their functions sit among the methods. Co-Authored-By: Claude Opus 5.5 --- .../TypeScript/MLIRLogic/MLIRGenStore.h | 2 + tslang/lib/TypeScript/DeclarationPrinter.cpp | 80 ++++++++++++++----- tslang/lib/TypeScript/MLIRGenClasses.cpp | 13 +++ tslang/test/tester/CMakeLists.txt | 13 ++- .../shared-class-members/base_module.ts | 21 +++++ .../shared-class-members/class_members.ts | 21 +++++ .../shared-class-members/derived_module.ts | 7 ++ 7 files changed, 134 insertions(+), 23 deletions(-) create mode 100644 tslang/test/tester/shared-class-members/base_module.ts create mode 100644 tslang/test/tester/shared-class-members/class_members.ts create mode 100644 tslang/test/tester/shared-class-members/derived_module.ts diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h index a6cbabe92..514ec6d3d 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h @@ -781,6 +781,8 @@ struct ClassInfo ProcessingStages processing; // the module `processing` was reached in: a discovery module is thrown away with what it holds mlir::Operation *processingModule = nullptr; + // and the declaration it was reached from + ClassLikeDeclaration processingDeclaration; ClassInfo() : isDeclaration(false), hasNew(false), hasConstructor(false), constructorAccessLevel(mlir_ts::AccessLevel::Public), hasInitializers(false), hasStaticConstructor(false), diff --git a/tslang/lib/TypeScript/DeclarationPrinter.cpp b/tslang/lib/TypeScript/DeclarationPrinter.cpp index 8c8f7b760..d02d56b4a 100644 --- a/tslang/lib/TypeScript/DeclarationPrinter.cpp +++ b/tslang/lib/TypeScript/DeclarationPrinter.cpp @@ -1,6 +1,7 @@ #include "TypeScript/MLIRLogic/MLIRDeclarationPrinter.h" #include "TypeScript/MLIRLogic/MLIRPrinter.h" +#include "llvm/ADT/StringSet.h" #include "llvm/ADT/TypeSwitch.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/Debug.h" @@ -558,6 +559,27 @@ namespace typescript printIndexer(indexInfo.indexSignature.getInput(0), indexInfo.indexSignature.getResult(0)); } + auto printAccessorGet = [&](auto &accessor) { + printAccessor( + accessor.isStatic, "get", accessor.name, accessor.getAccessLevel, + accessor.get.funcType.getParams(), + accessor.get.funcType.getNumResults() > 0 ? accessor.get.funcType.getResult(0) : mlir::Type(), + classType->classType, + accessor.get.dllName); + }; + + auto printAccessorSet = [&](auto &accessor) { + printAccessor( + accessor.isStatic, "set", accessor.name, accessor.setAccessLevel, + accessor.set.funcType.getParams(), + mlir::Type(), + classType->classType, + accessor.set.dllName); + }; + + // accessor halves printed in the methods loop, where their functions sit + llvm::StringSet<> printedAccessorFuncs; + // methods (including static) for (auto method : classType->methods) { @@ -566,13 +588,41 @@ namespace typescript // get/set accessors are ALSO registered here under their mangled // "get_x"/"set_x" funcOp name (so same-file/JIT compiles can find them as - // ordinary methods) - but that mangled form is printed separately below, - // via classType->accessors, using real `get`/`set` syntax. Printing it - // again here as a plain method would make the importer register it as an - // ordinary MethodDeclaration instead of a GetAccessor/SetAccessor, so + // ordinary methods) - but that mangled form is printed with real `get`/`set` + // syntax instead. Printed as a plain method, the importer would register it + // as an ordinary MethodDeclaration instead of a GetAccessor/SetAccessor, so // property-style access (`obj.x`, `super.x`) would never populate the // reconstructed class's accessors list and fail to resolve. - if (llvm::any_of(classType->accessors, [&](auto &accessor) { + // + // And printed here, at its place among the methods, not after all of them: + // the importer builds the vtable in the order it reads the members, and this + // list is the order the exporting module built it in. With the accessors + // last, `get textContent()` written before `addText()` swapped their slots, + // and the importer's `addText` call ran the getter. + auto accessorPrinted = false; + for (auto &accessor : classType->accessors) + { + if (filterName(accessor.name)) + continue; + + if (accessor.get && accessor.get.name == method.funcName) + { + printAccessorGet(accessor); + printedAccessorFuncs.insert(accessor.get.name); + accessorPrinted = true; + break; + } + + if (accessor.set && accessor.set.name == method.funcName) + { + printAccessorSet(accessor); + printedAccessorFuncs.insert(accessor.set.name); + accessorPrinted = true; + break; + } + } + + if (accessorPrinted || llvm::any_of(classType->accessors, [&](auto &accessor) { return (accessor.get && accessor.get.name == method.funcName) || (accessor.set && accessor.set.name == method.funcName); })) @@ -613,30 +663,20 @@ namespace typescript newline(); } - // accessors + // accessors whose functions are not among the methods for (auto accessor : classType->accessors) { if (filterName(accessor.name)) continue; - if (accessor.get) + if (accessor.get && !printedAccessorFuncs.contains(accessor.get.name)) { - printAccessor( - accessor.isStatic, "get", accessor.name, accessor.getAccessLevel, - accessor.get.funcType.getParams(), - accessor.get.funcType.getNumResults() > 0 ? accessor.get.funcType.getResult(0) : mlir::Type(), - classType->classType, - accessor.get.dllName); + printAccessorGet(accessor); } - if (accessor.set) + if (accessor.set && !printedAccessorFuncs.contains(accessor.set.name)) { - printAccessor( - accessor.isStatic, "set", accessor.name, accessor.setAccessLevel, - accessor.set.funcType.getParams(), - mlir::Type(), - classType->classType, - accessor.set.dllName); + printAccessorSet(accessor); } } diff --git a/tslang/lib/TypeScript/MLIRGenClasses.cpp b/tslang/lib/TypeScript/MLIRGenClasses.cpp index 559410bb1..ad5ca40cc 100644 --- a/tslang/lib/TypeScript/MLIRGenClasses.cpp +++ b/tslang/lib/TypeScript/MLIRGenClasses.cpp @@ -125,9 +125,22 @@ namespace mlirgen } } + // One library of several modules can declare an imported class more than once: a module + // whose export depends on a class of another module declares that class too (a derived + // class's base), so the importer reads `Base` from both modules' __decls. The second is + // the same external class again, and generating it again redefined `Base..new`. + if (!isGenericClass && newClassPtr->isImport + && newClassPtr->processingDeclaration && newClassPtr->processingDeclaration != classDeclarationAST + && newClassPtr->processingModule == theModule.getOperation() + && testProcessingState(newClassPtr, ProcessingStages::Processed, genContext)) + { + return {mlir::success(), newClassPtr->classType.getName().getValue()}; + } + if (!genContext.allowPartialResolve) { newClassPtr->processingModule = theModule.getOperation(); + newClassPtr->processingDeclaration = classDeclarationAST; } setProcessingState(newClassPtr, ProcessingStages::Processing, genContext); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index c23b97214..7af2c0554 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -2438,9 +2438,16 @@ tslang_add_import_tests(reference-path-import-and-reference "${reference_path_di tslang_add_import_tests(reference-path-two-imports "${reference_path_dir}" two_imports module_left module_right) tslang_add_import_tests(reference-path-import-diamond "${reference_path_dir}" import_diamond plain_module via_module) tslang_add_import_tests(reference-path-import-cycle "${reference_path_dir}" import_cycle cycle_module_a cycle_module_b) -# Static linking only: a -shared library of these two modules declares Node twice to its importer -# ("redefinition of symbol named 'Node..new'"), a separate gap. -tslang_add_test(NAME test-compile-reference-path-import-cycle-class COMMAND test-runner "${reference_path_dir}/import_cycle_class.ts" "${reference_path_dir}/cycle_class_node.ts" "${reference_path_dir}/cycle_class_text.ts") +# Through a -shared library of these two modules the importer read Node twice ("redefinition of +# symbol named 'Node..new'"), and then called its methods through the wrong vtable slots - see +# shared-class-members below. +tslang_add_import_tests(reference-path-import-cycle-class "${reference_path_dir}" import_cycle_class cycle_class_node cycle_class_text) + +# A -shared library's class, imported: declared twice when another module of the library extends +# it, and with its vtable built in another order than the library's when an accessor comes before +# a method. +set(shared_class_members_dir "${PROJECT_SOURCE_DIR}/test/tester/shared-class-members") +tslang_add_import_tests(shared-class-members "${shared_class_members_dir}" class_members base_module derived_module) # The first attempt at an import in a cycle fails and is tried again; its errors were printed # although the compile succeeded. add_test(NAME test-compile-reference-path-import-cycle-quiet diff --git a/tslang/test/tester/shared-class-members/base_module.ts b/tslang/test/tester/shared-class-members/base_module.ts new file mode 100644 index 000000000..acaf2f5ea --- /dev/null +++ b/tslang/test/tester/shared-class-members/base_module.ts @@ -0,0 +1,21 @@ +// An accessor written before a method: the importer of a -shared library read the class's members +// with the accessors last, built its vtable in that order, and its `add` call ran the getter. +export class Base { + items: number[] = []; + + get count(): number { + return this.items.length; + } + + add(v: number) { + this.items.push(v); + } + + set first(v: number) { + this.items[0] = v; + } + + last() { + return this.items[this.items.length - 1]; + } +} diff --git a/tslang/test/tester/shared-class-members/class_members.ts b/tslang/test/tester/shared-class-members/class_members.ts new file mode 100644 index 000000000..bce351238 --- /dev/null +++ b/tslang/test/tester/shared-class-members/class_members.ts @@ -0,0 +1,21 @@ +import { Base } from "./base_module"; +import { Derived } from "./derived_module"; + +function main() { + const b = new Base(); + b.add(1); + b.add(2); + assert(b.items.length == 2, "add"); + assert(b.count == 2, "count"); + assert(b.last() == 2, "last"); + b.first = 7; + assert(b.items[0] == 7, "first"); + + const d = new Derived(); + d.add(3); + assert(d.count == 1, "derived count"); + assert(d.last() == 3, "derived last"); + assert(d.name == "derived", "derived name"); + + print("done."); +} diff --git a/tslang/test/tester/shared-class-members/derived_module.ts b/tslang/test/tester/shared-class-members/derived_module.ts new file mode 100644 index 000000000..4901f9c11 --- /dev/null +++ b/tslang/test/tester/shared-class-members/derived_module.ts @@ -0,0 +1,7 @@ +// A class of another module of the same library extends Base, so this module's declarations +// carry Base as well; the importer read Base twice ("redefinition of symbol named 'Base..new'"). +import { Base } from "./base_module"; + +export class Derived extends Base { + name = "derived"; +}