From 9faeef31180b3323310c3c572f1e0cd4428f4ed8 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sun, 19 Jul 2026 17:51:19 +0100 Subject: [PATCH] Preserve source field order in the interface-cast object clone When a tuple/object value can't be cast to an interface as-is (field types need coercion), castTupleToInterface / castObjectToInterface build a clone - and both took the clone's field list from InterfaceInfo::getTupleTypeFields, which orders members methods-first. For Counter {count; inc()} the clone became [inc@0, count@8], reversed from the source layout [count@0, inc@8] that both the vtable's field-offset slots and any already-compiled method bodies (in particular in the exporting module) address by byte offset. At runtime c.count read the funcptr slot and inc() incremented the funcptr instead of count - silent corruption, no crash. Factor the clone field-list construction into getInterfaceCloneFields: keep the SOURCE tuple's field order, take only the field types from the interface (the coercion purpose of the clone), append interface-only members after the source fields. All downstream member lookups are name-based, so only the byte layout changes. Together with #256 this makes casting a cross-module structurally-typed object VALUE to a method-bearing interface inside the importer work end-to-end for the first time. Add the regression test pair export/import_object_literal_structural_typed.ts (asserts count 0 -> 1 -> 2 through the interface) in both compile-shared and jit-shared modes. Full ctest suite: 722/722 passed (720 + 2 new). Co-Authored-By: Claude Fable 5 --- .../interface-vtable-simplification-design.md | 38 ++++++++++ tslang/lib/TypeScript/MLIRGenCast.cpp | 72 +++++++++++-------- tslang/test/tester/CMakeLists.txt | 2 + .../export_object_literal_structural_typed.ts | 14 ++++ .../import_object_literal_structural_typed.ts | 20 ++++++ 5 files changed, 117 insertions(+), 29 deletions(-) create mode 100644 tslang/test/tester/tests/export_object_literal_structural_typed.ts create mode 100644 tslang/test/tester/tests/import_object_literal_structural_typed.ts diff --git a/tslang/docs/interface-vtable-simplification-design.md b/tslang/docs/interface-vtable-simplification-design.md index 13b2b2145..a8215cd03 100644 --- a/tslang/docs/interface-vtable-simplification-design.md +++ b/tslang/docs/interface-vtable-simplification-design.md @@ -435,3 +435,41 @@ not-yet-investigated bug. No regression test was added for the now-fixed crash (a full end-to-end test would still fail, on this new bug, not the old one); this section records the fix and the newly-found blocker for whoever picks up the clone-bug investigation next. + +### Clone field-order bug FIXED (2026-07-19, follow-up to the above) + +Root cause of the reversed clone: both clone-building blocks (in +`castTupleToInterface` and `castObjectToInterface`, MLIRGenCast.cpp) built +the clone's field list from `InterfaceInfo::getTupleTypeFields`, which +emits **methods first, then fields** (MLIRGenStore.h) - so for +`Counter {count; inc()}` the clone's layout became `[inc@0, count@8]`, +reversed from the source tuple's `[count@0, inc@8]`. Two consumers still +addressed fields by the source layout: the interface vtable's field-offset +slots and - fatally, unfixable from the importer - the exporting module's +already-compiled `inc` body, which hard-codes `count` at offset 0. Hence +`c.count` read the funcptr slot and `inc()` incremented it. + +Fix: new file-local helper `getInterfaceCloneFields` (MLIRGenCast.cpp), +used by both clone sites - the clone now preserves the SOURCE tuple's field +order, taking only the field TYPES from the interface (the type-coercion +purpose of the clone, e.g. si32 -> number or func-typed field vs method +funcType), with interface-only members appended after the source fields. +All member lookups downstream are name-based, so only the byte layout was +at stake. + +With this plus PR #256, the full cross-module scenario finally works: +regression test pair added - +`export_object_literal_structural_typed.ts` (structurally-typed +method-bearing export, NOT interface-typed at the definition site) / +`import_object_literal_structural_typed.ts` (casts the imported value to +the interface in the importer, asserts count 0 -> 1 -> 2 through the +interface), wired as +`test-{compile,jit}-shared-export-import-object-literal-structural-typed`. +Note the cast still clones (warning stands): mutations through the +interface don't write back to the exporter's global - that's the +documented value-semantics of this cast, not a bug in this arc. Known +remaining sharp edge (pre-existing, unchanged): a clone whose field TYPES +are size-changing coercions (e.g. si32 -> f64 number) still shifts offsets +relative to already-compiled method bodies expecting the original layout; +that can only bite literals whose inferred field types differ in size from +the interface's, and is out of scope here. 722/722 suite (720 + 2 new). diff --git a/tslang/lib/TypeScript/MLIRGenCast.cpp b/tslang/lib/TypeScript/MLIRGenCast.cpp index 5a137e0b5..d78553977 100644 --- a/tslang/lib/TypeScript/MLIRGenCast.cpp +++ b/tslang/lib/TypeScript/MLIRGenCast.cpp @@ -7,7 +7,47 @@ namespace typescript namespace mlirgen { - ValueOrLogicalResult MLIRGenImpl::selectFieldsValues(mlir::Location location, SmallVector &values, mlir::Value value, + // Field list for the clone made when a tuple/object value can't be cast to an + // interface as-is (field types need coercion, e.g. si32 -> number, or a + // func-typed field vs a method's funcType). The clone MUST keep the SOURCE + // tuple's field order: the interface vtable's field-offset slots and any + // methods compiled against the original object layout (in particular in + // another module) address fields by byte offset, so reordering to interface + // member order (methods-first, per InterfaceInfo::getTupleTypeFields) + // silently reads/writes the wrong slots at runtime. Only the TYPES are taken + // from the interface; interface-only members are appended after the source + // fields. + static mlir::LogicalResult getInterfaceCloneFields(mlir::ArrayRef srcFields, + InterfaceInfo::TypePtr interfaceInfo, mlir::MLIRContext *context, + SmallVector &fields) + { + SmallVector interfaceFields; + if (mlir::failed(interfaceInfo->getTupleTypeFields(interfaceFields, context))) + { + return mlir::failure(); + } + + for (auto &origField : srcFields) + { + auto interfaceField = + std::find_if(interfaceFields.begin(), interfaceFields.end(), + [&](auto &item) { return item.id == origField.id; }); + fields.push_back(interfaceField != interfaceFields.end() ? *interfaceField : origField); + } + + for (auto &interfaceField : interfaceFields) + { + if (std::find_if(fields.begin(), fields.end(), + [&](auto &item) { return item.id == interfaceField.id; }) == fields.end()) + { + fields.push_back(interfaceField); + } + } + + return mlir::success(); + } + + ValueOrLogicalResult MLIRGenImpl::selectFieldsValues(mlir::Location location, SmallVector &values, mlir::Value value, ::llvm::ArrayRef<::mlir::typescript::FieldInfo> fields, bool filterSpecialCases, const GenContext &genContext, bool errorAsWarning) { auto count = 0; @@ -1579,24 +1619,11 @@ namespace mlirgen if (mlir::failed(mth.canCastTupleToInterface(location, srcTuple, interfaceInfo, true))) { SmallVector fields; - if (mlir::failed(interfaceInfo->getTupleTypeFields(fields, builder.getContext()))) + if (mlir::failed(getInterfaceCloneFields(srcTuple.getFields(), interfaceInfo, builder.getContext(), fields))) { return mlir::failure(); } - // append all fields from original tuple - for (auto origField : srcTuple.getFields()) { - if (std::find_if( - fields.begin(), - fields.end(), - [&] (auto& item) { - return item.id == origField.id; - }) == fields.end()) - { - fields.push_back(origField); - } - } - auto newInterfaceTupleType = getTupleType(fields); CAST(inEffective, location, newInterfaceTupleType, inEffective, genContext); tupleType = newInterfaceTupleType; @@ -1638,24 +1665,11 @@ namespace mlirgen if (mlir::failed(mth.canCastTupleToInterface(location, storageTuple, interfaceInfo, true))) { SmallVector fields; - if (mlir::failed(interfaceInfo->getTupleTypeFields(fields, builder.getContext()))) + if (mlir::failed(getInterfaceCloneFields(storageTuple.getFields(), interfaceInfo, builder.getContext(), fields))) { return mlir::failure(); } - // append all fields from original tuple - for (auto origField : storageTuple.getFields()) { - if (std::find_if( - fields.begin(), - fields.end(), - [&] (auto& item) { - return item.id == origField.id; - }) == fields.end()) - { - fields.push_back(origField); - } - } - auto newInterfaceTupleType = getTupleType(fields); CAST_A(unboxed, location, newInterfaceTupleType, in, genContext); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 86923ab4c..e342d9d1b 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -847,6 +847,7 @@ add_test(NAME test-compile-shared-decl-emit-class COMMAND test-runner -shared "$ add_test(NAME test-compile-shared-export-import-class-interface COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_interface.ts") add_test(NAME test-compile-shared-export-import-object-literal-with-class-types COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_class_types.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_class_types.ts") add_test(NAME test-compile-shared-export-import-object-literal-with-interface COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_interface.ts") +add_test(NAME test-compile-shared-export-import-object-literal-structural-typed COMMAND test-runner -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed.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") @@ -862,6 +863,7 @@ add_test(NAME test-jit-shared-decl-emit-class COMMAND test-runner -jit -shared " add_test(NAME test-jit-shared-export-import-class-interface COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_interface.ts") add_test(NAME test-jit-shared-export-import-object-literal-with-class-types COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_class_types.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_class_types.ts") add_test(NAME test-jit-shared-export-import-object-literal-with-interface COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_with_interface.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_with_interface.ts") +add_test(NAME test-jit-shared-export-import-object-literal-structural-typed COMMAND test-runner -jit -shared -gctors-as-method "${PROJECT_SOURCE_DIR}/test/tester/tests/import_object_literal_structural_typed.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_object_literal_structural_typed.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.ts b/tslang/test/tester/tests/export_object_literal_structural_typed.ts new file mode 100644 index 000000000..dd1517e07 --- /dev/null +++ b/tslang/test/tester/tests/export_object_literal_structural_typed.ts @@ -0,0 +1,14 @@ +namespace A { + + export interface Counter { + count: number; + inc(): void; + } + + // deliberately typed with a STRUCTURAL type, not the interface: the + // interface cast then happens in the IMPORTING module against the + // @dllimport-reconstructed tuple type (see + // import_object_literal_structural_typed.ts), unlike + // export_object_literal_with_interface.ts where the cast happens here. + export var counterObj: { count: number; inc(): void } = { count: 0, inc() { this.count = this.count + 1; } }; +} diff --git a/tslang/test/tester/tests/import_object_literal_structural_typed.ts b/tslang/test/tester/tests/import_object_literal_structural_typed.ts new file mode 100644 index 000000000..f45913d86 --- /dev/null +++ b/tslang/test/tester/tests/import_object_literal_structural_typed.ts @@ -0,0 +1,20 @@ +import './export_object_literal_structural_typed' + +// Casting the imported structurally-typed VALUE to a method-bearing interface +// in the importing module. Regression coverage for two bugs: +// - the hybrid_func -> func cast crash in CastLogicHelper (PR #256); +// - the interface-cast clone reordering fields to interface (methods-first) +// order, which silently misread `count` through the vtable's field offset +// and made inc() corrupt the funcptr slot instead of incrementing. +// Note: this cast clones the object (see the compiler warning), so mutations +// through `c` are visible via `c` but not via A.counterObj. +var c: A.Counter = A.counterObj; +print(c.count); +assert(c.count == 0); +c.inc(); +print(c.count); +assert(c.count == 1); +c.inc(); +print(c.count); +assert(c.count == 2); +print("done.");