Fix casting object literal that omits an extends-inherited optional field - #271
Merged
Merged
Conversation
…ield An object literal omitting an optional field inherited via `extends` (e.g. `interface Derived extends Base` where `Base` declares `opt?: number`) failed to compile when cast to the derived interface, even though the identical scenario works when the interface declares the optional field directly. Root cause: getInterfaceCloneFields (the fallback clone path used when a tuple-to-interface cast needs field-type coercion) tried to backfill every interface field into the clone's tuple, including ones the source object genuinely lacks. The codebase's actual convention for a genuinely-absent optional field is a shorter storage tuple with the slot omitted entirely - InterfaceSymbolRefOp's `optional` attribute handles the absence via a runtime slot-count check, not a placeholder value in a same-width tuple. Fix: skip appending an absent conditional field in getInterfaceCloneFields's append loop instead of backfilling it, matching the existing convention. Also fixes getTupleTypeFields to propagate field.isConditional instead of hardcoding false (methods already did this correctly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
ASDAlexander77
added a commit
that referenced
this pull request
Jul 20, 2026
…on) and extend test coverage (#272) * Add Claude Code badge to README Credits Claude Code as a development tool, matching the existing "Powered by LLVM" badge style. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Extend interface extends+optional coverage to cross-module cast Adds a cross-module test pair for `interface Derived extends Base` where Base declares an optional field, cast from imported exported objects both with and without the optional field provided. Confirms the fix from #271 (same-module only) also covers the cross-module path, which has historically been more fragile due to vtable cloning and GC-heap boxing - no additional bug found; full interface test suite (69 tests) passes 100%. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix compiler crash casting object literal that omits an extends-inherited optional method An object literal omitting an optional METHOD inherited via `extends` (e.g. `interface Base { opt?(n: number): number }`, `interface Derived extends Base`) crashed the compiler outright with llvm_unreachable("not implemented yet") in mlirGenObjectVirtualTableDefinitionForInterface. The equivalent missing FIELD case already had a placeholder pattern (a -1 sentinel cast to the field's ref type, inserted into the vtable slot), but the missing-method case had never been implemented at all - this was simply unsupported, not merely broken for extends specifically. Fix: mirror the missing-field's -1-sentinel placeholder for a missing method's vtable slot, cast to the method's function-pointer-ref type instead of a field type. New test 00interface_optional_method_extends.ts locks this in (compile+jit, registered in CMakeLists.txt). Note: comparing an optional interface method against `undefined` (`x.optMethod == undefined`) is a separate, deeper, pre-existing bug (hardcoded to a compile-time-constant result in UndefLogicHelper.h, unrelated to extends) - deliberately left unfixed and documented in the test's comments after an attempted fix caused an unresolved control-flow crash; deferred for a future session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix optional interface method vs undefined comparison always returning wrong constant Comparing an optional interface method against `undefined` (`obj.optMethod == undefined`) never worked for any interface, extends or not: UndefLogicHelper.h's processUndefVale only special-cased InterfaceType/ClassType against undefined, so a BoundFunctionType fell through to a hardcoded compile-time-constant false/true regardless of whether the method was actually present. Fix required two coordinated changes: - InterfaceSymbolRefOpLowering (LowerToLLVM.cpp) now branchlessly selects a null `this` pointer (LLVM::SelectOp) when an optional method's vtable slot holds the "missing member" -1 sentinel - a real bound method's `this` is never null, giving the comparison something concrete to check. - UndefLogicHelper.h's new BoundFunctionType branch checks that null-or-not via a directly-emitted LLVM::ICmpOp, not the shared LogicOp<StdIOpTy, V1, v1, ...> helper: v1 is a template parameter baked in from the outer comparison operator that triggered the whole call (e.g. arith::CmpIPredicate::ne for a source-level `!=`), so passing a different SyntaxKind at the call site was silently ignored and inverted the result. An earlier attempt using CodeLogicHelper's conditionalExpressionLowering (block-splitting control flow) caused an unresolved crash; WinDbg dump analysis showed execution running off the end of a JIT-compiled block. Replacing it with the branchless select (already used successfully elsewhere in this file) resolved it cleanly. Restores the `== undefined`/`!= undefined` assertions in 00interface_optional_method_extends.ts now that they pass correctly. Full test suite: 758/758 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 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
extends(e.g.interface Derived extends BasewhereBasedeclaresopt?: number) failed withfield "opt" can't be found in tuple, even though the identical scenario compiles fine when the interface declares the optional field directly (noextends).getInterfaceCloneFields(lib/TypeScript/MLIRGenCast.cpp) — the fallback clone path used when a tuple-to-interface cast needs field-type coercion — tried to backfill every interface field into the clone's tuple, including ones the source object genuinely lacks. But the codebase's existing convention for a genuinely-absent optional field is a shorter storage tuple with the slot omitted entirely;InterfaceSymbolRefOp'soptionalattribute handles the absence via a runtime slot-count check, not a placeholder value in a same-width tuple.getInterfaceCloneFields's append loop instead of backfilling it, matching the existing convention. Also fixesInterfaceInfo::getTupleTypeFields(include/TypeScript/MLIRLogic/MLIRGenStore.h) to propagatefield.isConditionalinstead of hardcodingfalse(the method branch right above it already did this correctly).Test plan
test/tester/tests/00interface_optional_extends.ts(registered intest/tester/CMakeLists.txt, compile + JIT) — casts both a providing and a non-providing object literal to the derived interface, plus a post-cast re-read to also guard against the historical shared-virtualIndex-clobber bug classctest -R "extends-interface|interface-optional"— 18/18 passedctest -R "interface"(broad sanity pass, 67 tests) — 100% passed, no regressions🤖 Generated with Claude Code