A -shared library's class is imported once, with the library's vtable

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),

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);
             }
         }
 

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);

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

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];
+    }
+}

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.");
+}

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";
+}