Fix class-implements-interface optional-member crash and add coverage#279
Merged
ASDAlexander77 merged 2 commits intoJul 22, 2026
Merged
Conversation
class X implements Interface crashed the compiler at declaration time (no instantiation needed) whenever Interface had an optional member X didn't declare - mlirGenClassVirtualTableDefinitionForInterface never checked the isMissing flag that getVirtualTable already provides, unlike its object-literal sibling (mlirGenObjectVirtualTableDefinitionForInterface), which already handles this correctly. Fixed by mirroring that existing handling. Also adds cross-module (export/import) test coverage for class+interface combinations that had no prior coverage: multilevel interface extends, optional interface members, abstract class implementing an interface, and structural (no explicit `implements`) interface satisfaction. Several -shared tiers are added but left disabled in CMakeLists.txt, documented with the root cause: a class that is itself a dynamic import gets a constant (unresolvable) symbol reference for methods reachable only through its own interface vtable, when cast to that interface from the importing module - a real, separate, deeper bug left for a follow-up.
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
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
class X implements Interfacecrashed the compiler at DECLARATION time (no instantiation needed) wheneverInterfacehad an optional member (opt?: number) thatXdidn't declare.mlirGenClassVirtualTableDefinitionForInterfacenever checked theisMissingflag thatgetVirtualTablealready provides for this legitimate case, unlike its object-literal sibling (mlirGenObjectVirtualTableDefinitionForInterface), which already handles it correctly with a-1sentinel placeholder. Fixed by mirroring that existing, proven handling.export/import) test coverage for class+interface combinations with no prior coverage: multilevel interface extends implemented by a class, optional interface members implemented by a class, an abstract class implementing an interface, and structural (no explicitimplements) interface satisfaction (a different code path:mlirGenCreateInterfaceVTableForClass, triggered at cast time, vs. the declaration-timemlirGenClassHeritageClauseImplementspath).mlirGenClassVirtualTableDefinitionForInterfacealways emitted a constantSymbolRefOpfor the implementing method, unlike the class's ordinary vtable builder, which already resolves a dynamic-import-owned method at runtime instead (SearchForAddressOfSymbolOp). Mirroring that fix exposed two further, general (not interface-specific) lowering-pass bugs - bothGlobalOpLoweringandcreateFunctionFromRegionwere giving a synthesized__cctorconstructor function the wrong debug-info location when its initializer is lazily rebuilt from inside an unrelated function - both fixed. This re-enables the multilevel, optional, and structural-interface-sharedtests.this.area()) returns a wrong value when the method is reached through an interface reference (works correctly via direct or base-class-typed calls). Documented in detail inCMakeLists.txtand left for a dedicated follow-up.Test plan
--emit=objctest -C Debug -R "implements-interface|structural-interface"- all enabled tiers pass (10/10)ctest -C Debug -j8- 786/786 passed, no regressions