Don't generate *Param and *Result types in Rust by default
#547
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
Previously when a single WIT type was used in a "borrowed" position, or a parameter to an import, and an "owned" position then the type would have two copies of its definition generated if it internally contained a list. This was intended to signify that for import parameters ownership wasn't necessary so borrowed values could be passed in. In practice though I don't think this was the right default to have. This surprisingly generates two types when one might expect one and otherwise if a type if received from an export and expected to be passed to an import then it's not easy to do so since it must be converted to the borrowed type. This commit moves the preexisting behavior of generating duplicate types behind a new `duplicate_if_necessary` option. The default behavior is to no longer generate two types, but instead just one. Imports now generate their types with a `&` in front if it's otherwise owned internally to signify that ownership isn't required. Closes bytecodealliance#535
pchickey
approved these changes
Mar 28, 2023
|
Fantastic, thank you - I'm looking forward to trying this out! |
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Apr 10, 2023
This commit is a mirror of bytecodealliance/wit-bindgen#547 into the `bindgen!` macro for Wasmtime. The new default is to generate only one Rust type per WIT type input, regardless of if the representation can be slightly more optimal in niche cases with more borrows. This should make the macro easier to work with in the limit ideally. Closes bytecodealliance#6124
alexcrichton
added a commit
to bytecodealliance/wasmtime
that referenced
this pull request
Apr 10, 2023
This commit is a mirror of bytecodealliance/wit-bindgen#547 into the `bindgen!` macro for Wasmtime. The new default is to generate only one Rust type per WIT type input, regardless of if the representation can be slightly more optimal in niche cases with more borrows. This should make the macro easier to work with in the limit ideally. Closes #6124
brendandburns
pushed a commit
to brendandburns/wasmtime
that referenced
this pull request
Apr 13, 2023
…ance#6189) This commit is a mirror of bytecodealliance/wit-bindgen#547 into the `bindgen!` macro for Wasmtime. The new default is to generate only one Rust type per WIT type input, regardless of if the representation can be slightly more optimal in niche cases with more borrows. This should make the macro easier to work with in the limit ideally. Closes bytecodealliance#6124
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.
Previously when a single WIT type was used in a "borrowed" position, or a parameter to an import, and an "owned" position then the type would have two copies of its definition generated if it internally contained a list. This was intended to signify that for import parameters ownership wasn't necessary so borrowed values could be passed in.
In practice though I don't think this was the right default to have. This surprisingly generates two types when one might expect one and otherwise if a type if received from an export and expected to be passed to an import then it's not easy to do so since it must be converted to the borrowed type.
This commit moves the preexisting behavior of generating duplicate types behind a new
duplicate_if_necessaryoption. The default behavior is to no longer generate two types, but instead just one. Imports now generate their types with a&in front if it's otherwise owned internally to signify that ownership isn't required.Closes #535