Skip to content

Fix determinism bug when mapping type names - #8956

Merged
tlively merged 1 commit into
mainfrom
map-type-names-indices-determinism-fix
Jul 30, 2026
Merged

Fix determinism bug when mapping type names#8956
tlively merged 1 commit into
mainfrom
map-type-names-indices-determinism-fix

Conversation

@tlively

@tlively tlively commented Jul 30, 2026

Copy link
Copy Markdown
Member

When we rewrite module types, we must associate the old type names and indices with the new types they are being mapped to. The code for this was previously sensitive to the iteration order over the old-to-new type map because it modified the module's type names as it traversed them and because visitation order determined which name was kept in cases where multiple old types map to the same new type. Fix both problems by keeping the lesser names and indices in the case of merges and by not updating the module type names until after the new type names have been fully populated.

When we rewrite module types, we must associate the old type names and indices with the new types they are being mapped to. The code for this was previously sensitive to the iteration order over the old-to-new type map because it modified the module's type names as it traversed them and because visitation order determined which name was kept in cases where multiple old types map to the same new type. Fix both problems by keeping the lesser names and indices in the case of merges and by not updating the module type names until after the new type names have been fully populated.
@tlively
tlively requested a review from a team as a code owner July 30, 2026 20:07
@tlively
tlively requested a review from kripken July 30, 2026 20:07
@tlively

tlively commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Split off from #8953, as suggested.

Comment thread src/ir/type-updating.cpp
return seenTypeNames.insert(name).second;
});
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test needed updating after this? Odd...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the only pass that merged types previously was TypeMerging, which I guess must handle this itself somehow.

@tlively
tlively merged commit 16272ba into main Jul 30, 2026
16 checks passed
@tlively
tlively deleted the map-type-names-indices-determinism-fix branch July 30, 2026 20:58
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