Skip to content

Fix cross-module class extends under -shared: inline dlsym resolution + decl-printer fixes - #277

Merged
ASDAlexander77 merged 1 commit into
fix/crt-assert-report-modefrom
fix/cross-module-dynamic-import-method-resolution
Jul 21, 2026
Merged

Fix cross-module class extends under -shared: inline dlsym resolution + decl-printer fixes#277
ASDAlexander77 merged 1 commit into
fix/crt-assert-report-modefrom
fix/cross-module-dynamic-import-method-resolution

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

Stacked on #276 (base branch is fix/crt-assert-report-mode) — merge #276 first, then this retargets to main automatically.

Summary

Makes a derived class extending a base from a dynamically imported module (-shared, real DLL boundary) work end-to-end. All six formerly disabled tests — test-{compile,jit}-shared-export-import-class-extends{,-multilevel,-implements-diamond} — now pass and are enabled.

Implements exactly the recommendation from docs/cross-module-dynamic-import-instanceof-design.md (§9, written after the previous session's three reverted attempts): a dedicated, self-contained resolution path with zero interaction with fullNameGlobalsMap scoping. Three co-operating fixes, each exposing the next once the previous error stopped masking it:

  1. ClassMethodAccess inline dlsym fallback (MLIRGenImpl.h): a dynamic-import class member with neither a locally defined FuncOp (bodyless declarations are now explicitly excluded — they lower to unlinkable external references) nor a registered dlsym-global resolves in place via SearchForAddressOfSymbolOp + cast. No global registration at all — valid in both discovery and real passes by construction. This fixed the M.Animal..instanceOf can't be resolved error and cross-module super.method(...) calls generally.

  2. Vtable slots owned by a dynamic-import base resolve at runtime (MLIRGenClasses.cpp): the extends-recursive vtable contains slots for inherited members (incl. the base's .rtti/.size statics under ADD_STATIC_MEMBERS_TO_VTABLE) whose symbols live in the DLL — a constant SymbolRefOp to those was a hard link error (lld: undefined symbol: M.Animal..size referenced by .data). Ownership is determined structurally (walk self + transitive bases, exact symbol match). GlobalOpLowering already routes such initializers through __cctor; no lowering changes.

  3. Two DeclarationPrinter bugs (only reachable once 1+2 let the multilevel test — the first with class B extends A inside the exported decl text — get far enough):

    • The extends clause printed the class's own name instead of the base's (unused loop variable): class B extends M.B. The importer built a self-cycle in baseClasses and stack-overflowed in getVirtualTable's unguarded recursion (0xC00000FD, root-caused via ProcDump + WinDbg).
    • The synthetic base-class storage field (memory layout, not a source member: M.A: [.vtbl:Opaque, a:number];) leaked into the printed decl as a real field, shifting every subsequent field's offset in the importer — silent data corruption: the DLL's methods saw b=22 while the importer read c.b == 0 one slot past it.

Per the design doc's hard-won lesson, not touched: fullNameGlobalsMap scoping, the generic dynamicImport branch's name computation, decorator synthesis. Doc updated with the resolution (§10).

Test plan

  • All six -shared class-extends tests pass (compile + JIT × basic/multilevel/diamond) and are enabled in CMakeLists.txt
  • Full ctest suite green: 767/767 (761 prior + 6 newly enabled) — the previous attempts' regressions were only visible suite-wide, so this was the gating check

🤖 Generated with Claude Code

… + decl-printer fixes

Makes a derived class extending a base from a dynamically imported module
(-shared, real DLL boundary) work end-to-end - all six formerly disabled
tests (basic/multilevel/diamond x compile/JIT) now pass and are enabled.
Three co-operating fixes, each exposing the next:

1. ClassMethodAccess (isDynamicImport instance branch): when a member has
   neither a locally defined FuncOp (bodyless declarations are now explicitly
   excluded - they lower to unlinkable external references) nor a registered
   dlsym-global, resolve it in place via SearchForAddressOfSymbolOp + cast -
   no global registration at all, sidestepping the fullNameGlobalsMap scope
   fragility that sank the previous session's three attempts (see
   docs/cross-module-dynamic-import-instanceof-design.md, now updated with
   the resolution in section 10).

2. mlirGenClassVirtualTableDefinition: vtable slots whose symbols are owned
   by a dynamic-import base (inherited virtual methods; .rtti/.size statics
   under ADD_STATIC_MEMBERS_TO_VTABLE) emit runtime symbol resolution instead
   of constant SymbolRefOp - a link-time address of a DLL-resident symbol
   does not exist without an import library. GlobalOpLowering already routes
   such initializers through the __cctor path.

3. DeclarationPrinter: the extends clause printed the class's OWN name
   instead of the base's (unused loop variable - "class B extends M.B"),
   which made the importer build a self-cycle in baseClasses and
   stack-overflow in getVirtualTable's unguarded recursion; and the synthetic
   base-class storage field (memory layout, not a source member) leaked into
   the printed decl as a real field, shifting every subsequent field's offset
   in the importer - the DLL's methods saw b=22 while the importer read
   c.b==0 from one slot past it.

Full suite green: 767/767 (761 prior + 6 newly enabled).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 merged commit c08f5f4 into fix/crt-assert-report-mode Jul 21, 2026
@ASDAlexander77
ASDAlexander77 deleted the fix/cross-module-dynamic-import-method-resolution branch July 21, 2026 23:48
ASDAlexander77 added a commit that referenced this pull request Jul 22, 2026
…vestigation (#276)

* Fix CRT assert dialogs blocking debug tslang.exe runs; document cross-module .instanceOf investigation

_CrtSetReportFile(_CRT_ASSERT, ...) has no effect unless the report mode for
that category is also set to _CRTDBG_MODE_FILE - without it, the default mode
(_CRTDBG_MODE_WNDW) still pops a blocking "Assertion failed" MessageBox on
every assert(), which looks like a hang in a non-interactive session. One
missing _CrtSetReportMode call fixes it.

Also documents a three-attempt investigation into the disabled cross-module
`-shared` class-extends tests (PR #274's known issue): root-caused why
.instanceOf can't be resolved (two real bugs found and fixed in isolation),
but every attempt to fix it regressed unrelated tests project-wide (70+ tests,
then 10 more on a narrower retry) via a fragile scoped-hash-table interaction
that isn't safe to patch blindly. All three attempts reverted; the actual fix
is left to a future, more careful session per the recommendation in the new
doc.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* Fix cross-module class extends under -shared: inline dlsym resolution + decl-printer fixes (#277)

Makes a derived class extending a base from a dynamically imported module
(-shared, real DLL boundary) work end-to-end - all six formerly disabled
tests (basic/multilevel/diamond x compile/JIT) now pass and are enabled.
Three co-operating fixes, each exposing the next:

1. ClassMethodAccess (isDynamicImport instance branch): when a member has
   neither a locally defined FuncOp (bodyless declarations are now explicitly
   excluded - they lower to unlinkable external references) nor a registered
   dlsym-global, resolve it in place via SearchForAddressOfSymbolOp + cast -
   no global registration at all, sidestepping the fullNameGlobalsMap scope
   fragility that sank the previous session's three attempts (see
   docs/cross-module-dynamic-import-instanceof-design.md, now updated with
   the resolution in section 10).

2. mlirGenClassVirtualTableDefinition: vtable slots whose symbols are owned
   by a dynamic-import base (inherited virtual methods; .rtti/.size statics
   under ADD_STATIC_MEMBERS_TO_VTABLE) emit runtime symbol resolution instead
   of constant SymbolRefOp - a link-time address of a DLL-resident symbol
   does not exist without an import library. GlobalOpLowering already routes
   such initializers through the __cctor path.

3. DeclarationPrinter: the extends clause printed the class's OWN name
   instead of the base's (unused loop variable - "class B extends M.B"),
   which made the importer build a self-cycle in baseClasses and
   stack-overflow in getVirtualTable's unguarded recursion; and the synthetic
   base-class storage field (memory layout, not a source member) leaked into
   the printed decl as a real field, shifting every subsequent field's offset
   in the importer - the DLL's methods saw b=22 while the importer read
   c.b==0 from one slot past it.

Full suite green: 767/767 (761 prior + 6 newly enabled).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

---------

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