Extract "world elaboration" to a separate function #1800
Merged
Conversation
This file contains 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
This commit extracts the process of elaborating a worlds imports/exports to a dedicated function. This function ensures the transitive closure of all imports/exports are listed in the world in proper topograhical order. This additionally validates that the world has a coherent definition given WIT today, notably that imports don't accidentally try to use exported types. This functionality was all present previously during the process of taking a WIT document and creating `Resolve`. This moves the logic around and refactors it slightly given its new surroundings. This reorders a few imports/exports in worlds from what was previously present but the underlying meaning of worlds should be the same. The goal of this commit is to enable `Resolve`-mutating transformations to not need to preserve all these invariants each time a world is modified. Instead a world can be modified and then this function can be called to "clean up" the world afterwards.
This commit updates the `importize` operation to be a much simpler "just move the exports to the imports" operation where care is only taken to preserve imported types and to additionally error on overlap of imported types and exported functions/interfaces. The previous `elaborate_world` method helps make this method much simpler than before.
|
The impact of this new function shows up a bit in the second commit here where |
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
Sep 18, 2024
This commit is the first half and the meat of the implementation of bytecodealliance#1774 where, by default, WIT processing tools will now merge world imports when possible based on semver versions. The precise shape of how this is done has gone through a few iterations and this is the end result I've ended up settling on. This should handle all the various cases we're interested in and continue to produce valid worlds within a `Resolve` (with the help of `elaborate_world` added in bytecodealliance#1800). CLI tooling has been updated with flags to configure this behavior but the behavior is now enabled by default.
dicej
approved these changes
Sep 18, 2024
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
Sep 23, 2024
This commit is the first half and the meat of the implementation of bytecodealliance#1774 where, by default, WIT processing tools will now merge world imports when possible based on semver versions. The precise shape of how this is done has gone through a few iterations and this is the end result I've ended up settling on. This should handle all the various cases we're interested in and continue to produce valid worlds within a `Resolve` (with the help of `elaborate_world` added in bytecodealliance#1800). CLI tooling has been updated with flags to configure this behavior but the behavior is now enabled by default.
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
Sep 23, 2024
This commit is the first half and the meat of the implementation of bytecodealliance#1774 where, by default, WIT processing tools will now merge world imports when possible based on semver versions. The precise shape of how this is done has gone through a few iterations and this is the end result I've ended up settling on. This should handle all the various cases we're interested in and continue to produce valid worlds within a `Resolve` (with the help of `elaborate_world` added in bytecodealliance#1800). CLI tooling has been updated with flags to configure this behavior but the behavior is now enabled by default.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 24, 2024
* Implement merging semver-compatible interfaces in imports This commit is the first half and the meat of the implementation of #1774 where, by default, WIT processing tools will now merge world imports when possible based on semver versions. The precise shape of how this is done has gone through a few iterations and this is the end result I've ended up settling on. This should handle all the various cases we're interested in and continue to produce valid worlds within a `Resolve` (with the help of `elaborate_world` added in #1800). CLI tooling has been updated with flags to configure this behavior but the behavior is now enabled by default. * Add broken test case for wit-component * Match wasm/component import names based on versions This commit implements the final bit of #1774 where the `wit-component` crate will now match imports based on semver version in addition to the previous exact-name matching that was performed previously. * Don't panic on duplicate shims in wit-component This is now possible with import version matching so refactor the internal data structures to better support insertion of possibly duplicate shims. Most of wit-component was already ready for this refactoring, it was just the initial generation of shims that needed to be reorganized slightly. * wip * Elaborate all worlds after semver merging Any interface could be modified, so elaborate all of them to fixup anything that needs new imports. * Only merge once at the end, not continuously This commit updates how the semver-merging bits of `Resolve` work by moving it towards the end of the encoding process rather than once-per-world-merged. That helps both deduplicate work and fix some asserts I'm seeing that are tripping if a `Resolve` is import-merged and then merged again elsewhere. This additionally simplifies some APIs because the boolean of what to do isn't threaded to quite so many locations. * Fix test * Update interface deps of exports too Dependencies might depend on replaced interfaces so the dependency edges there need to be updated in the same manner as imports. * Fix fuzzer build Also ensure to at least try to test the new merging function.
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.
This commit extracts the process of elaborating a worlds imports/exports
to a dedicated function. This function ensures the transitive closure of
all imports/exports are listed in the world in proper topograhical
order. This additionally validates that the world has a coherent
definition given WIT today, notably that imports don't accidentally try
to use exported types.
This functionality was all present previously during the process of
taking a WIT document and creating
Resolve. This moves the logicaround and refactors it slightly given its new surroundings. This
reorders a few imports/exports in worlds from what was previously
present but the underlying meaning of worlds should be the same.
The goal of this commit is to enable
Resolve-mutating transformationsto not need to preserve all these invariants each time a world is
modified. Instead a world can be modified and then this function can be
called to "clean up" the world afterwards.