Fix hybrid-func-to-func cast crash when casting cross-module tuple to interface - #256
Merged
Merged
Conversation
… interface
castTypeScriptTypes handled narrowing a BoundFunctionType value to a plain
FunctionType (the "losing this reference" vtable-slot path) but not the
equivalent HybridFunctionType case - the actual type of a method field read
off a cross-module reconstructed tuple. That value fell through to
castLLVMTypes' dead "value to ref of value" branch and hit
llvm_unreachable("review usage") at CastLogicHelper.h:765, crashing the
compiler on `<Counter>importedObj` in the importing module.
Add the missing branch, mirroring the BoundFunctionType case: extract the
raw function pointer via GetMethodOp and warn about the dropped receiver
(the interface vtable call re-supplies its own thisVal).
Note: this fixes the compile-time crash only. Verifying end-to-end surfaced
a separate pre-existing bug - the object clone built for the interface cast
writes its fields in reversed order, so the vtable's field offsets read the
wrong slots at runtime. Recorded in
docs/interface-vtable-simplification-design.md for a follow-up
investigation; no regression test added since the full scenario still fails
on that second bug.
Full ctest suite: 720/720 passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ASDAlexander77
added a commit
that referenced
this pull request
Jul 19, 2026
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>
ASDAlexander77
added a commit
that referenced
this pull request
Jul 19, 2026
…spatch (#259) * Extend test coverage for object-literal/interface casts and method dispatch Adds four new test cases exercising variations not covered by the recent interface-vtable-cast arc (#256-258), which only ever tested zero-argument, single-method interfaces: - 00object_annotated_method_params.ts: type-literal-annotated object methods with PARAMETERS (that arc's fix only had zero-arg method coverage), including a same-module chained sibling-method call. - 00interface_object_array.ts: several distinct object literals (each its own per-type constant vtable) collected into a single interface-typed array and dispatched through the same call site in a loop - would catch a shared/aliased vtable or wrong-`this` bug that a single-object test can't. - export/import_object_literal_structural_typed_params.ts: extends export/import_object_literal_structural_typed.ts's cross-module coverage along the "zero-arg -> parameterized method" axis. Along the way, found and documented (not fixed) two new, real gaps in docs/interface-vtable-simplification-design.md: - a cross-module cast to a MULTI-method interface breaks for every vtable slot past 0 (wrong value at slot 1, crash at slot 2+) - every prior test in this arc only used single-method interfaces, so this was never exercised before. The committed cross-module test deliberately stays single-method to avoid committing a failing case. - (same-module only, unrelated) a value-returning method can't `return this.siblingMethod(...)` within the same type-literal-annotated object literal - calling the sibling as a bare statement works fine. Avoided in the committed tests. Full ctest suite: 730/730 passed (722 + 8 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Update CMake configuration for Visual Studio 18 and adjust ROOT_PATH in typescript.cmake --------- 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
docs/interface-vtable-simplification-design.md): casting a cross-module, structurally-typed object VALUE to a method-bearing interface inside the importing module hitllvm_unreachable("review usage")atCastLogicHelper.h:765.castTypeScriptTypeshandled narrowing aBoundFunctionTypevalue to a plainFunctionType(the "losing this reference" vtable-slot path) but not the equivalentHybridFunctionTypecase — which is the actual type of a method field read off a@dllimport-reconstructed tuple (!ts.hybrid_func<, !ts.void, false>→!ts.func<!ts.opaque, !ts.void, false>). The value fell through tocastLLVMTypes' dead "value to ref of value" branch.HybridFunctionTypebranch mirroring theBoundFunctionTypeone exactly — extract the raw function pointer viaGetMethodOp, emit the same "losing this reference" warning (the interface vtable call re-supplies its ownthisVal, so dropping the value's own receiver here is the established convention).Important caveat — crash fixed, scenario not yet fully working
Verifying end-to-end (real
-sharedcompile+link+run, WinDbg breakpoints on the actualc.inc()call site with live register/memory inspection) surfaced a separate pre-existing bug: the object clone built when casting the cross-module tuple to an interface writes its fields in reversed order (incfuncptr at offset 0,countat offset 8 — backwards from the declaredstruct<(f64, ptr)>layout the vtable's field offsets are computed against). So the compile-time crash is gone, but at runtimec.inc()silently corrupts the funcptr slot instead of incrementingcount. That clone-path bug (likelyMLIRGenCast.cpp'scastObjectToInterface, behind the "Cloned object is used" warning) is documented in the design doc as its own follow-up investigation.No regression test added for this fix alone — an end-to-end test would still fail on the clone bug, not the fixed crash. The design doc records both the fix and the newly-found blocker.
Test plan
export var counterObj: { count: number; inc(): void } = {...}+<A.Counter>A.counterObjin the importer,-sharedmode) now compiles and links where it previously crashed the compiler.-j8).🤖 Generated with Claude Code