Skip to content

Make WIT merging commutative#2451

Open
fibonacci1729 wants to merge 2 commits intobytecodealliance:mainfrom
fibonacci1729:wit-commutativity
Open

Make WIT merging commutative#2451
fibonacci1729 wants to merge 2 commits intobytecodealliance:mainfrom
fibonacci1729:wit-commutativity

Conversation

@fibonacci1729
Copy link
Copy Markdown
Contributor

@fibonacci1729 fibonacci1729 commented Feb 25, 2026

Closes #1897

This PR makes the following changes made to mod.rs to make WIT merging commutative:

  • MergeMap::build_interface — Previously, every type and function in from's interface was required to exist in into's interface (strict equality). Now, one interface can be a superset of the other. If both sides have exclusive items, it fails (genuinely incompatible). If only from has extras, they're skipped in the mapping phase and handled later during the merge.

  • Resolve::merge interface processing — When a mapped interface has extra types/functions from from, those extras are now added to the into interface (types added to the types map, functions added to the functions map with proper span adjustment and remapping).

  • Adds topologically_sort_interfaces method on Resolve that rebuilds the interfaces arena in topological order after a merge (newly-added interfaces may have been added after existing interfaces that now depend on them).

NOTE: I made use of Copilot (Claude Opus 4.6) in implementing the topological sort bits.

Signed-off-by: Brian Hardock <brian.hardock@fermyon.com>
@fibonacci1729 fibonacci1729 marked this pull request as ready for review February 26, 2026 21:05
@fibonacci1729 fibonacci1729 requested a review from a team as a code owner February 26, 2026 21:05
@fibonacci1729 fibonacci1729 requested review from fitzgen and removed request for a team February 26, 2026 21:05
@alexcrichton
Copy link
Copy Markdown
Member

Wanted to say I haven't forgotten this, and thanks! I'll do my best to set aside time tomorrow to review this

@fibonacci1729
Copy link
Copy Markdown
Contributor Author

Oh no rush at all!

Copy link
Copy Markdown
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

I'm a bit wary of the top-o sort of the arena itself, but before reviewing that too too closely I would in theory prefer to have fuzzing/testing/something to show it's a problem before we'd identify it's a problem. To that end a question for you: how much time would you be willing to put into testing/fuzzing this?

I ask because much of the design of wit-component and wit-parser are heavily influenced by the roundtrip_wit.rs fuzzer in this repository. I discovered what felt like an endless long tail of issues only through fuzzing which highlighted all sorts of stuff I forgot when originally writing WIT support. I'd ideally like to lean on that for this as well as much as we can mainly because I don't necessarily trust myself to catch all the corner cases here. I highlighted one case below of a merging which I believe should work but isn't currently caught by the fuzzer (I've been running it locally for awhile now).

I describe below a test script as well as a sort of ideal testing strategy where we start from a single WIT, diverge, and assert the merge then later works. The roundtrip_wit.rs fuzzer is already almost doing this where it's taking worlds in a WIT package, creating a dummy component, deleting arbitrary imports, and then creating components. I've applied this diff locally as an attempt to be able to test like this, although it's turning up things that I think are preexisting issues as well.

Would you be up for helping to work on fuzzing side of this to help ensure this covers all sorts of various edge cases and such?

@fibonacci1729
Copy link
Copy Markdown
Contributor Author

fibonacci1729 commented Apr 7, 2026

@alexcrichton in the last commit here, i added a test to demonstrate why the toposort is needed post merge and updated the fuzzer to run the real-world workflow.

Signed-off-by: Brian Hardock <brian.hardock@fermyon.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.

merging wit is not commutative

2 participants