Round-trip boxed-ness across @dllimport boundary via a sibling @boxed decorator#261
Merged
ASDAlexander77 merged 4 commits intoJul 20, 2026
Merged
Conversation
…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.
…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>
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.
… 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.
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
Follow-up to "Bug 1" (untyped cross-module object-literal export → bare
objecton reimport, documented indocs/interface-vtable-simplification-design.md). Includes the earlier printer fix (ObjectType prints its real structural shape instead of bare"object") which fixes the originally-reported crash (mlir::cast<mlir_ts::TupleType>assert atMLIRGenInterfaces.cpp:312) — but verifying end-to-end found that alone isn't enough.The core problem
An untyped, method-bearing object-literal export (
export var counterObj = {...}) is auto-boxed asObjectType(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". (Also tested per a suggested workaround: introducing an explicitobject-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/@volatiledecorator pattern (VariableClass,iterateDecorators):DeclarationPrinter.cpp:printVariableDeclarationemits a sibling@boxeddecorator when the variable's real type isObjectTypewrapping a concreteTuple/ConstTuple/ObjectStoragetype.VariableClassgets anisBoxedflag, set from the@boxeddecorator via the sameiterateDecoratorsloop that already handles@dllimport.MLIRGenVariables.cpp: thedeclarationMode-onlyinitFuncpath (evaluateTypeAndInit) wraps the resolved type ingetObjectType(...)whenisBoxedis set. Deliberately not changed: the generalgetType()handling forTypeLiteral(used everywhere — function params, return types, etc.) — boxing there unconditionally would regress Preserve source field order in the interface-cast object clone #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 — zero regressions, including #257's unboxed case and the already-working same-module boxed interface exports.
What's NOT fixed
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 it's a constructor-ordering issue or something else specific to that shape. Full mechanism, evidence, and the precise next debugging step (a WinDbg breakpoint comparing the global's raw memory before vs. aftermain()starts) are documented indocs/interface-vtable-simplification-design.md.No regression test added — the scenario this was built for still fails, just later and for a different reason.
🤖 Generated with Claude Code