From 7c64fb483c27b7e442a6a66e368b727642b31f03 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Mon, 20 Jul 2026 19:50:47 +0100 Subject: [PATCH] Fix cross-module diamond-extends interface silently corrupting data interface Combined extends Left, Right (2+ extends targets) had two bugs: mlirGen(InterfaceDeclaration) re-running for an already-declared interface duplicated its extends list unboundedly (every other member-add is idempotent, this push_back wasn't), and the per-object vtable patch loop used a method's standalone virtualIndex instead of its offset within the combined vtable, mis-patching every method from the second extends target onward. Adds cross-module regression coverage for diamond extends (isolated) and multilevel+diamond together, extending the same-module coverage from #267 across the module boundary. Co-Authored-By: Claude Sonnet 5 --- .../TypeScript/MLIRLogic/MLIRGenStore.h | 32 +++++++-- tslang/lib/TypeScript/MLIRGenInterfaces.cpp | 42 +++++++++--- tslang/test/tester/CMakeLists.txt | 4 ++ ...uctural_typed_extends_interface_diamond.ts | 30 +++++++++ ...ural_typed_extends_interface_multilevel.ts | 67 +++++++++++++++++++ ...uctural_typed_extends_interface_diamond.ts | 17 +++++ ...ural_typed_extends_interface_multilevel.ts | 31 +++++++++ 7 files changed, 209 insertions(+), 14 deletions(-) create mode 100644 tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts create mode 100644 tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts create mode 100644 tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts create mode 100644 tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h index 62903a253..5e50064e6 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h @@ -288,18 +288,40 @@ struct InterfaceInfo // own methods plus every inherited method from `extends`, in the same // extends-first-then-own order as getTupleTypeFields/getVirtualTable - - // needed wherever code must patch/visit every method slot a cast target - // could own, not just this interface's own directly-declared ones. - void getAllMethods(llvm::SmallVector &allMethods) + // paired with the combined vtable slot each one actually occupies IN THIS + // (possibly multi-level/diamond `extends`) interface, since + // method.virtualIndex is only correct standalone (see findField's doc + // comment above); a method inherited through `extends` + // needs vtableOffset (the declaring interface's slot-block position) added to + // it. Needed by mlirGenCreateInterfaceVTableForObject's per-object patch loop, + // which indexes directly into ONE combined vtable for the cast's root + // interface - using method.virtualIndex alone there mis-patches every + // inherited method beyond the first `extends` target (harmless no-op for a + // single `extends` chain, where each level's own slots start where the + // previous level's ended and vtableOffset is cumulative from index 0 - but + // wrong for a diamond, where a second/third `extends` target's methods all + // stack at vtableOffset 0 relative to themselves). + void getAllMethodsWithVTableOffset(llvm::SmallVector> &allMethods, int baseOffset = 0) { + auto offset = baseOffset; for (auto &extent : extends) { - std::get<1>(extent)->getAllMethods(allMethods); + std::get<1>(extent)->getAllMethodsWithVTableOffset(allMethods, offset); + offset += std::get<1>(extent)->getVTableSize(); } + // own methods' virtualIndex (assignCanonicalVirtualIndexes()) is already + // absolute WITHIN THIS interface's own combined vtable (it starts counting + // after this interface's own extends block) - so at THIS level baseOffset + // is 0 and no further adjustment is needed. It's only when THIS interface + // is itself reached as an `extends` target (the recursive call above, + // baseOffset != 0) that its methods - own AND inherited alike, all already + // pushed into `allMethods` by this point - need the caller's baseOffset, + // which the recursive call already applied for inherited ones; own ones + // get it here via baseOffset. for (auto &method : methods) { - allMethods.push_back(&method); + allMethods.push_back({&method, baseOffset}); } } diff --git a/tslang/lib/TypeScript/MLIRGenInterfaces.cpp b/tslang/lib/TypeScript/MLIRGenInterfaces.cpp index 6495f4097..d7cf37bc7 100644 --- a/tslang/lib/TypeScript/MLIRGenInterfaces.cpp +++ b/tslang/lib/TypeScript/MLIRGenInterfaces.cpp @@ -201,8 +201,15 @@ namespace mlirgen // same as an own one; only checking newInterfacePtr->methods here left every // inherited method's slot holding its initial offset-placeholder value // (never a real function pointer), crashing on the first call through it. - llvm::SmallVector allMethods; - newInterfacePtr->getAllMethods(allMethods); + // Paired with each method's actual combined-vtable slot + // (method.virtualIndex + vtableOffset): virtualIndex alone is only correct + // standalone, same caveat as InterfaceInfo::findField's doc comment + // (MLIRGenStore.h) - a method inherited from a SECOND OR LATER `extends` + // target (diamond `extends A, B`) has virtualIndex relative to its OWN + // declaring interface, which collides with the FIRST target's slots unless + // offset by where that target's own block starts in the combined vtable. + llvm::SmallVector> allMethods; + newInterfacePtr->getAllMethodsWithVTableOffset(allMethods); if (allMethods.size() > 0) { mlir_ts::TupleType storeType; @@ -226,8 +233,8 @@ namespace mlirgen // imported object type reconstructed from a @dllimport declaration, with no // local funcOp to name) still need their function pointer read out of the // actual object `in` at cast time. - llvm::SmallVector methodsNeedingPatch; - for (auto* methodPtr : allMethods) + llvm::SmallVector> methodsNeedingPatch; + for (auto& [methodPtr, vtableOffset] : allMethods) { auto& method = *methodPtr; auto fieldId = builder.getStringAttr(method.name); @@ -240,7 +247,7 @@ namespace mlirgen auto fieldInfo = mth.getFieldInfoByIndex(storeType, index); if (lookupObjectLiteralMethodSymbol(fieldInfo.type, fieldId).empty()) { - methodsNeedingPatch.push_back(methodPtr); + methodsNeedingPatch.push_back({methodPtr, vtableOffset}); } } @@ -265,7 +272,7 @@ namespace mlirgen builder.create(location, valueVTable, heapVTable); auto varVTable = builder.create(location, globalVTableRefValue.getType(), heapVTable); - for (auto* methodPtr : methodsNeedingPatch) + for (auto& [methodPtr, vtableOffset] : methodsNeedingPatch) { auto& method = *methodPtr; auto index = mth.getFieldIndexByFieldName(storeType, builder.getStringAttr(method.name)); @@ -282,8 +289,9 @@ namespace mlirgen << "\n\t object method ref: " << V(methodRef) << "\n\n";); // where to save - auto fieldInfoVT = mth.getFieldInfoByIndex(vtableType, method.virtualIndex); - auto methodRefVT = builder.create(location, fieldInfoVT.type, varVTable, method.virtualIndex); + auto combinedVirtualIndex = method.virtualIndex + vtableOffset; + auto fieldInfoVT = mth.getFieldInfoByIndex(vtableType, combinedVirtualIndex); + auto methodRefVT = builder.create(location, fieldInfoVT.type, varVTable, combinedVirtualIndex); LLVM_DEBUG(llvm::dbgs() << "\n!!\n\t vtable method: " << method.name << "\n\t vtable method ref: " << V(methodRefVT) << "\n\n";); @@ -570,7 +578,23 @@ namespace mlirgen auto interfaceInfo = getInterfaceInfoByFullName(interfaceType.getName().getValue()); if (interfaceInfo) { - newInterfacePtr->extends.push_back({-1, interfaceInfo}); + // mlirGen(InterfaceDeclaration) can run more than once for the same + // already-declared interface (e.g. re-visited from another module or a + // discovery pass) - mlirGenInterfaceAddFieldMember/addInterfaceMethod are + // idempotent (they check getFieldIndex/getMethodIndex before pushing), but + // this push_back had no equivalent guard, so a diamond `extends A, B` got + // BOTH targets appended again on every re-run, multiplying the combined + // vtable's slot count (and every findField/findMethod/getVirtualTable + // offset derived from it) without bound. Skip targets already present. + auto alreadyExtends = llvm::any_of(newInterfacePtr->extends, [&](auto &extent) { + return std::get<1>(extent)->fullName == interfaceInfo->fullName; + }); + + if (!alreadyExtends) + { + newInterfacePtr->extends.push_back({-1, interfaceInfo}); + } + success = true; } }) diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index cc8ff9614..157a2cf3d 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -866,6 +866,8 @@ add_test(NAME test-compile-shared-export-import-object-literal-structural-typed- add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-multi-method COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_multi_method.ts") add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-interleaved COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_interleaved.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_interleaved.ts") add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface.ts") +add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-multilevel COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts") +add_test(NAME test-compile-shared-export-import-object-literal-structural-typed-extends-interface-diamond COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts") add_test(NAME test-compile-shared-export-import-vars COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars.ts") add_test(NAME test-compile-shared-export-import-vars-2 COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars2.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars2.ts") add_test(NAME test-compile-shared-export-import-enum COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_enum.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_enum.ts") @@ -888,6 +890,8 @@ add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-para add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-multi-method COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_multi_method.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_multi_method.ts") add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-interleaved COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_interleaved.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_interleaved.ts") add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface.ts") +add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-multilevel COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts") +add_test(NAME test-jit-shared-export-import-object-literal-structural-typed-extends-interface-diamond COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts") add_test(NAME test-jit-shared-export-import-vars COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars.ts") add_test(NAME test-jit-shared-export-import-vars-2 COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_vars2.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_vars2.ts") add_test(NAME test-jit-shared-export-import-enum COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_enum.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_enum.ts") diff --git a/tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts b/tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts new file mode 100644 index 000000000..bd1270ec2 --- /dev/null +++ b/tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_diamond.ts @@ -0,0 +1,30 @@ +namespace M2 { + export interface Left { + left: number; + addLeft(n: number): void; + } + export interface Right { + right: number; + addRight(n: number): void; + } + export interface Combined extends Left, Right { + combined: number; + addCombined(n: number): void; + } + + export var rawCombined: { + left: number; + addLeft(n: number): void; + right: number; + addRight(n: number): void; + combined: number; + addCombined(n: number): void; + } = { + left: 1.0, + addLeft(n: number) { this.left = this.left + n; }, + right: 2.0, + addRight(n: number) { this.right = this.right + n; }, + combined: 3.0, + addCombined(n: number) { this.combined = this.combined + n; }, + }; +} diff --git a/tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts b/tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts new file mode 100644 index 000000000..e08e0281a --- /dev/null +++ b/tslang/test/tester/tests/export_object_literal_structural_typed_extends_interface_multilevel.ts @@ -0,0 +1,67 @@ +namespace M { + + // Extends export_object_literal_structural_typed_extends_interface.ts's + // coverage (single-level: Accumulator extends Base) to a cross-module + // 3-level chain (C extends B extends A) and an interface with TWO + // extends targets (Combined extends Left, Right) - the same coverage + // 00object_annotated_method_extends_interface_multilevel.ts locked in + // same-module, verified here to also survive the module boundary. + + export interface A { + a: number; + addA(n: number): void; + } + export interface B extends A { + b: number; + addB(n: number): void; + } + export interface C extends B { + c: number; + addC(n: number): void; + } + + export var raw: { + a: number; + addA(n: number): void; + b: number; + addB(n: number): void; + c: number; + addC(n: number): void; + } = { + a: 1.0, + addA(n: number) { this.a = this.a + n; }, + b: 2.0, + addB(n: number) { this.b = this.b + n; }, + c: 3.0, + addC(n: number) { this.c = this.c + n; }, + }; + + export interface Left { + left: number; + addLeft(n: number): void; + } + export interface Right { + right: number; + addRight(n: number): void; + } + export interface Combined extends Left, Right { + combined: number; + addCombined(n: number): void; + } + + export var rawCombined: { + left: number; + addLeft(n: number): void; + right: number; + addRight(n: number): void; + combined: number; + addCombined(n: number): void; + } = { + left: 1.0, + addLeft(n: number) { this.left = this.left + n; }, + right: 2.0, + addRight(n: number) { this.right = this.right + n; }, + combined: 3.0, + addCombined(n: number) { this.combined = this.combined + n; }, + }; +} diff --git a/tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts b/tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts new file mode 100644 index 000000000..b2181fc0e --- /dev/null +++ b/tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_diamond.ts @@ -0,0 +1,17 @@ +import './export_object_literal_structural_typed_extends_interface_diamond' + +let combined: M2.Combined = M2.rawCombined; + +combined.addLeft(10); +assert(combined.left == 11); +print(combined.left); + +combined.addRight(20); +assert(combined.right == 22); +print(combined.right); + +combined.addCombined(30); +assert(combined.combined == 33); +print(combined.combined); + +print("done."); diff --git a/tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts b/tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts new file mode 100644 index 000000000..2c8230777 --- /dev/null +++ b/tslang/test/tester/tests/import_object_literal_structural_typed_extends_interface_multilevel.ts @@ -0,0 +1,31 @@ +import './export_object_literal_structural_typed_extends_interface_multilevel' + +let obj: M.C = M.raw; + +obj.addA(10); +assert(obj.a == 11); +print(obj.a); + +obj.addB(20); +assert(obj.b == 22); +print(obj.b); + +obj.addC(30); +assert(obj.c == 33); +print(obj.c); + +let combined: M.Combined = M.rawCombined; + +combined.addLeft(10); +assert(combined.left == 11); +print(combined.left); + +combined.addRight(20); +assert(combined.right == 22); +print(combined.right); + +combined.addCombined(30); +assert(combined.combined == 33); +print(combined.combined); + +print("done.");