Preserve source field order in the interface-cast object clone#257
Merged
Conversation
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 <noreply@anthropic.com>
This was referenced Jul 19, 2026
ASDAlexander77
added a commit
that referenced
this pull request
Jul 20, 2026
…decorator (#261) * WIP: print ObjectType structural shape instead of bare 'object' (Bug 1, paused) MLIRPrinter::printType's ObjectType case unconditionally printed the literal string "object", discarding the storage type - the root cause of `let counterObj : object;` in cross-module @dllimport declarations for an untyped, method-bearing object-literal export. Print the structural shape via the (previously dead-code) printObjectType helper when the storage type is a TupleType/ConstTupleType/ObjectStorageType. This fixes the originally-described crash (mlir::cast<mlir_ts::TupleType> assert at MLIRGenInterfaces.cpp:312) - the declaration now round-trips as `let counterObj : {count:number, inc:() => void};` instead of `object;`. NOT a complete fix, NOT ready for a PR: verifying end-to-end (WinDbg) surfaced a deeper boxed-vs-unboxed representation mismatch across the @dllimport boundary for untyped exports - see docs/interface-vtable-simplification-design.md's "Bug 1 ... PAUSED before PR" section for full detail and next steps. Paused here per explicit direction, pending a follow-up investigation session. * Round-trip boxed-ness across @dllimport boundary via a sibling @boxed decorator Follow-up to Bug 1 (untyped cross-module object-literal export -> bare `object` on reimport, documented in docs/interface-vtable-simplification-design.md). The earlier printer fix (ObjectType prints its real structural shape instead of bare "object") is included here too and fixes the originally-reported crash (mlir::cast<mlir_ts::TupleType> assert at MLIRGenInterfaces.cpp:312) - but verifying end-to-end found that alone isn't enough: an untyped, method-bearing object-literal export is auto-boxed as ObjectType (pointer-indirected) in the exporter, but its declaration text ({count:number, inc:() => void}) is indistinguishable from an explicitly- typed (unboxed) export of the same shape - there's no TS source syntax for "and this should be boxed". Confirmed (per user's suggested test) that introducing an explicit `object`-typed intermediate binding doesn't help either: it widens away the concrete shape entirely and regresses to the ORIGINAL crash instead of avoiding it. Fix: teach the declaration wire format to carry boxed-ness explicitly, mirroring the established @dllimport/@used/@atomic/@volatile decorator pattern (VariableClass, iterateDecorators): - DeclarationPrinter.cpp: printVariableDeclaration emits a sibling @boxed decorator when the variable's real type is ObjectType wrapping a concrete Tuple/ConstTuple/ObjectStorage type. - VariableClass gets an isBoxed flag, set from the @boxed decorator via the same iterateDecorators loop that already handles @dllimport. - MLIRGenVariables.cpp: the declarationMode-only initFunc path (evaluateTypeAndInit) wraps the resolved type in getObjectType(...) when isBoxed is set. Deliberately NOT changed: the general getType() handling for TypeLiteral (used everywhere - function params, return types, etc.) - boxing there unconditionally would regress #257's explicitly-typed export test, which relies on staying unboxed. Verified: the untyped-export scenario no longer hits the boxing-mismatch crash (the interface cast now constructs without crashing). Full ctest suite: 730/730, unchanged - confirms zero regressions, including #257's unboxed case and the already-working same-module boxed interface exports. NOT a complete fix for the untyped-export scenario. A further, narrower issue remains: reading/calling the boxed global directly (no interface cast) still returns garbage or crashes with a null function pointer call. Unlike every other boxed cross-module global this arc has exercised (all 16-byte inline values, e.g. an interface's {vtblPtr, thisVal} pair), this is the first single-pointer-indirection (8-byte, GC-heap-allocated data) boxed global tried cross-module - not yet determined whether this is a constructor-ordering issue or something else specific to that shape. Full mechanism, evidence, and the precise next debugging step are documented in docs/interface-vtable-simplification-design.md for whoever picks this up. No regression test added - the scenario this was built for still fails, just later and for a different reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Replace @boxed decorator hack with @dllimport-implies-boxed rule VariableClass::isBoxed and the sibling @boxed decorator existed only because declaration-mode reconstruction of a {...}-shaped export had no way to know whether the original was a boxed ObjectType or a plain value tuple. Replace that side-channel with a simpler rule: @dllimport means the declaration holds a reference to the exporting module's storage, so declaration-mode boxes tuple-shaped reconstructions whenever isImport is set, no extra decorator needed. 720/720 -> 730/730 suite, zero regressions (existing structural-typed and with-interface shared export/import tests now take the boxed-source cast path and still pass). Added export/import_object_literal_untyped.ts to exercise the original untyped-export scenario directly (field read + method call, not just an interface cast). Both are registered DISABLED: they hang on a deadlock in cross-module boxed-global initialization, a known open issue (see docs/interface-vtable-simplification-design.md) now reachable for the first time because direct access to the boxed global no longer segfaults first. * Fix method-signature round-trip and export linkage for object-literal fields Two independent bugs found while investigating the untyped-export direct- read/call scenario (following up on 5b33995): 1. DeclarationPrinter/MLIRPrinter printed a method-typed tuple field (a FunctionType taking an explicit `this`) using arrow-property syntax ("name: (...) => result"), textually identical to what a genuinely bound closure field prints. On reimport this always parsed back as HybridFunctionType (two-pointer struct) instead of the exporter's real FunctionType (single pointer), silently misaligning the field. Now prints method-signature syntax ("name(...): result") for FunctionType fields specifically inside object-type-literal braces, matching what getMethodSignature resolves back to - scoped to '{...}' only, since method-signature syntax isn't valid inside a '[...]' tuple. 2. An object literal's method, when it's the initializer of an exported var/let, is reachable only indirectly (its function pointer is baked into the exported global's data, never referenced by name) - but codegen gave it ordinary private linkage, matching every other nested/anonymous function. The linker is then free to treat it as dead and strip it, leaving a null/invalid method slot when read cross- module. New GenContext.isExportVarReceiver flag, set the same way isGlobalVarReceiver already is, forces InternalFlags::IsPublic on such methods (same mechanism exported class methods already use). Also attempted to fix isDynamicImport's cross-module global to hold a live RefType instead of a one-time value-copy snapshot (matching how a @dllimport reference should behave) - reverted after it broke the existing structural-typed interface-cast test by exposing a real, pre-existing gap in FunctionType->BoundFunctionType interface-vtable coercion (the same class of issue as CastLogicHelper.h's long-documented, unimplemented "review usage" stubs). Left as isDynamicImport's original value-copy behavior; the reference-semantics fix and the vtable-coercion gap are follow-up work. 730/730 suite (+2 disabled), zero regressions. Investigation also isolated the actual remaining blocker for the disabled untyped-export direct-call test: the exporter's boxed object resolves to a real-looking heap pointer at runtime, but that memory is unmapped by the time the importer dereferences it - a genuine cross-module GC-heap- lifetime bug (see jit-globals-not-gc-roots.md precedent), unrelated to declaration text, linkage, or reference semantics. Tracked separately. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
"Cloned object is used"warning path), bothcastTupleToInterfaceandcastObjectToInterfacebuilt the clone's field list fromInterfaceInfo::getTupleTypeFields, which orders members methods-first. ForCounter {count; inc()}the clone's layout became[inc@0, count@8]— reversed from the source[count@0, inc@8]that the vtable's field-offset slots and the exporting module's already-compiledincbody address by byte offset.c.countread the funcptr slot as a double, andinc()incremented the funcptr slot instead ofcount— silent corruption, no crash.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 coercion purpose of the clone), with interface-only members appended after the source fields. All downstream member lookups are name-based, so only the byte layout changes.Why order must follow the source
The exporting module's method bodies are already compiled against the original layout (e.g.
inchard-codescountat offset 0) — the importer cannot re-layout them. So the clone has no choice but to match the source order; interface member order was never a valid layout for it.Together with #256, casting a cross-module structurally-typed object VALUE to a method-bearing interface inside the importer works end-to-end for the first time (previously: compiler crash before #256, silent corruption after).
Test plan
export_object_literal_structural_typed.ts/import_object_literal_structural_typed.ts: structurally-typed (not interface-typed) method-bearing export, cast to the interface in the importer, assertingcountgoes 0 → 1 → 2 through interface method calls. Wired astest-{compile,jit}-shared-export-import-object-literal-structural-typed; both pass.Known remaining sharp edge (pre-existing, documented in the design doc, out of scope): size-changing type coercions in the clone (e.g. inferred
si32field vs interfacenumber) still shift offsets relative to already-compiled method bodies.🤖 Generated with Claude Code