-
Notifications
You must be signed in to change notification settings - Fork 255
rust: Reimplement how stream/future payloads work #1528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ricochet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit ba5565b is a significant improvement, dealias begone! I double-checked the tests cover what we hit with the latest rc for WASI.
|
C++ codegen doesn't have async stream/futures support yet. You could extend the list in Looking at this with fresh eyes, I wish I had done this previously: fn should_fail_verify(
&self,
name: &str,
config: &crate::config::WitConfig,
_args: &[String],
) -> bool {
// C++ doesn't support async yet
config.async_
|| config.error_context
|| name == "async-resource-func.wit" // <-- missing async = true
} |
Previously stream/future payload were generated by collecting the set of types used in `future` and `stream` types in a WIT, rendering them to a string, deduplicating based on this string representation, and then generating various impls-with-vtables. This stringification strategy unfortunately falls down in a few situations such as: * Type aliases in WIT render as two names in Rust, but they're using the same Rust type. * Types with the same definition, but in multiple modules, will have two different paths in Rust but alias the same type. * Primitives may be used directly in streams/futures but then additionally used as a WIT type alias. In all of these situations it's effectively exposing how Rust requires at most one-impl-per-type-definition but the stringification/deduping was just a proxy for implementing this restriction and not a precise calculation. Using the work from bytecodealliance/wasm-tools#2447 as well as bytecodealliance#1468 it's possible to do all of this without stringifying. Specifically bytecodealliance#1468, transitively enabled by bytecodealliance/wasm-tools#2447, enables building a set of equal types that the Rust generator knows will all alias the same type definition. Using this it's possible to translate a payload to its "canonical payload" representation ID-wise and perform hashing/deduplication based on that. This in turn solves all of the issues above as well as previous issues such as bytecodealliance#1432 and bytecodealliance#1433 without requiring the workaround in bytecodealliance#1482. The end result is that all of these various bugs should be fixed and the Rust generator should be much more reliable about when exactly a trait impl is emitted vs not. Closes bytecodealliance#1523 Closes bytecodealliance#1524
ba5565b to
f03c4b6
Compare
Previously stream/future payload were generated by collecting the set of
types used in
futureandstreamtypes in a WIT, rendering them to astring, deduplicating based on this string representation, and then
generating various impls-with-vtables. This stringification strategy
unfortunately falls down in a few situations such as:
same Rust type.
different paths in Rust but alias the same type.
additionally used as a WIT type alias.
In all of these situations it's effectively exposing how Rust requires
at most one-impl-per-type-definition but the stringification/deduping
was just a proxy for implementing this restriction and not a precise
calculation. Using the work from bytecodealliance/wasm-tools#2447 as
well as #1468 it's possible to do all of this without stringifying.
Specifically #1468, transitively enabled by
bytecodealliance/wasm-tools#2447, enables building a set of equal types
that the Rust generator knows will all alias the same type definition.
Using this it's possible to translate a payload to its "canonical
payload" representation ID-wise and perform hashing/deduplication based
on that. This in turn solves all of the issues above as well as previous
issues such as #1432 and #1433 without requiring the workaround in #1482.
The end result is that all of these various bugs should be fixed and the
Rust generator should be much more reliable about when exactly a trait
impl is emitted vs not.
Closes #1523
Closes #1524