Skip to content

Fix cross-module diamond-extends interface silently corrupting data - #268

Merged
ASDAlexander77 merged 1 commit into
mainfrom
fix-diamond-extends-interface-vtable
Jul 20, 2026
Merged

Fix cross-module diamond-extends interface silently corrupting data#268
ASDAlexander77 merged 1 commit into
mainfrom
fix-diamond-extends-interface-vtable

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

Summary

  • interface Combined extends Left, Right (2+ extends targets, i.e. diamond) silently corrupted data cross-module, from two independent bugs:
    • mlirGen(InterfaceDeclaration) can re-run for an already-declared interface; the heritage-clause loop unconditionally appended to extends on every run instead of guarding like every other member-add (getFieldIndex/getMethodIndex checks), so the combined vtable's slot count multiplied without bound.
    • The per-object vtable patch loop used a method's standalone virtualIndex instead of its offset within the combined vtable, so every method from the second (or later) extends target got patched into the wrong slot.
  • Extends the same-module diamond/multilevel coverage from Verify multi-level and diamond extends interface hierarchies work correctly #267 across the module boundary, where it was never actually exercised.

Test plan

  • New regression tests: isolated cross-module diamond extends (export/import_object_literal_structural_typed_extends_interface_diamond.ts) and combined multilevel+diamond (export/import_object_literal_structural_typed_extends_interface_multilevel.ts), both compile+JIT variants, registered in test/tester/CMakeLists.txt.
  • Full JIT suite: 372/372 passing.
  • Full AOT compile suite: 376/376 passing.
  • Confirmed both bugs via --emit=mlir dump before/after each fix (vtable slot count and patched slot indices).

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

interface Combined extends Left, Right (2+ extends targets) had two bugs:
mlirGen(InterfaceDeclaration) re-running for an already-declared interface
duplicated its extends list unboundedly (every other member-add is
idempotent, this push_back wasn't), and the per-object vtable patch loop
used a method's standalone virtualIndex instead of its offset within the
combined vtable, mis-patching every method from the second extends target
onward. Adds cross-module regression coverage for diamond extends (isolated)
and multilevel+diamond together, extending the same-module coverage from
#267 across the module boundary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 merged commit b0716c2 into main Jul 20, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the fix-diamond-extends-interface-vtable branch July 20, 2026 19:42
ASDAlexander77 added a commit that referenced this pull request Jul 20, 2026
Adds a cross-module test pair for `interface Combined extends A, B, C`
to stress the vtable-offset accumulation and extends-dedup guard from
PR #268 (which only had 2-target diamond coverage) at N=3. Verified
via live debug trace that the fix generalizes correctly; full
extends-interface test family (12 tests) passes.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
ASDAlexander77 added a commit that referenced this pull request Jul 20, 2026
Adds a cross-module test pair for `interface Combined extends A, B, C`
to stress the vtable-offset accumulation and extends-dedup guard from
PR #268 (which only had 2-target diamond coverage) at N=3. Verified
via live debug trace that the fix generalizes correctly; full
extends-interface test family (12 tests) passes.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant