Skip to content

Fix a debug assert firing with multiple supertypes - #2689

Merged
fitzgen merged 1 commit into
bytecodealliance:mainfrom
alexcrichton:fix-multiple-supertypes
Sep 29, 2026
Merged

fitzgen merged 1 commit into
bytecodealliance:mainfrom
alexcrichton:fix-multiple-supertypes

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

This commit fixes a debug-assert added in #2652 which could erroneously fire. Notably the interning of rec groups is not written to handle multiple supertypes (as that's an invalid module anyway). I intended to debug-assert that this situation didn't arise within intern'ing under the (erroneous) assumption that such modules were already rejected during validation. Turns out the validation happened in a different order meaning that the assertion could indeed fire and just didn't with the tests included. This adds a small supertype-length check before interning to ensure that the phase there doesn't even see multiple supertypes.

This commit fixes a debug-assert added in bytecodealliance#2652 which could erroneously
fire. Notably the interning of rec groups is not written to handle
multiple supertypes (as that's an invalid module anyway). I intended to
debug-assert that this situation didn't arise within intern'ing under
the (erroneous) assumption that such modules were already rejected
during validation. Turns out the validation happened in a different
order meaning that the assertion could indeed fire and just didn't with
the tests included. This adds a small supertype-length check before
interning to ensure that the phase there doesn't even see multiple supertypes.
@alexcrichton
alexcrichton requested a review from a team as a code owner September 28, 2026 20:26
@fitzgen
fitzgen added this pull request to the merge queue Sep 29, 2026
Merged via the queue into bytecodealliance:main with commit a6d3ac6 Sep 29, 2026
37 checks passed
@alexcrichton
alexcrichton deleted the fix-multiple-supertypes branch September 29, 2026 22:37
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.

2 participants