Fix blocking CRT assert dialogs; document cross-module .instanceOf investigation#276
Merged
Conversation
…-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>
Merged
2 tasks
… + 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>
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
tslang.cpp's debug-CRT assertion setup:_CrtSetReportFile(_CRT_ASSERT, ...)was set but the report mode was never switched to_CRTDBG_MODE_FILE, so the default_CRTDBG_MODE_WNDWstill popped a blocking "Assertion failed" MessageBox on everyassert()in a debug build - indistinguishable from a hang in a non-interactive session. One missing_CrtSetReportModecall fixes it.docs/cross-module-dynamic-import-instanceof-design.md, a full write-up of a three-attempt investigation into the disabled cross-module-sharedclass-extends tests (PR Fix cross-module class extends crash and add regression coverage #274's known issue). Root-caused two real bugs in why.instanceOfcan't be resolved across a real DLL boundary, but every fix attempt (including two independent, narrower retries) regressed unrelated tests project-wide via a fragileScopedHashTablescoping interaction. All three attempts were reverted after full-suitectestruns caught the regressions; only this doc and the unrelated assert-dialog fix are included here. The actual fix is left to a future session per the doc's recommendation.Test plan
ctestsuite: 761/761 passing on this branch (confirms the revert leftmain's behavior unchanged; the assert-dialog fix is dev-experience only, no compiler-semantics change)