-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
wasmtime-wasi: Split a new IoView
trait off of WasiView
#10016
Conversation
413dd23
to
afb6228
Compare
T::ctx(&mut self.0) | ||
} | ||
} | ||
|
||
/// Per-[`Store`] state which holds state necessary to implement WASI from this |
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.
These docs are now appended to the ones above, so should these words move above the example perhaps?
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.
I made lots of fixes to the docs during the code motion for #10036 - can we put off the docs review and check it all over there?
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.
Sounds reasonable to me 👍
fn table(&mut self) -> &mut ResourceTable; | ||
} | ||
|
||
pub trait WasiView: IoView { |
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.
Could this retain some of the documentation from before as well? Ideally with an example or links to other examples throughout the crate.
use crate::ctx::WasiCtx; | ||
use wasmtime::component::ResourceTable; | ||
|
||
pub trait IoView: Send { |
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.
To perhaps bikeshed a bit, maybe TableView
?
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.
Also mind ensuring there's documentation for this trait as well? (similar to the WasiView
one below)
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.
I named it Io
instead of Table
view in anticipation of factoring it out for the wasmtime-wasi-io
crate. I don't mind renaming it, but thats why I was thinking Io
over Table
.
This PR prepares the wasi-io aspects of the wasmtime-wasi crate to be split off into its own independent crate, in order for the common base of wasi-io functionality to be reusable in a variety of host implementations which do not intend to reuse all of wasmtime-wasi.
Summary for users
You're an embedder using wasmtime-wasi, and maybe even wasmtime-wasi-http. You defined a struct like:
Prior to this PR, you'd hook your
MyCtx
up to theWasiView
andWasiHttpView
traits like:After this PR, you rewrite that to:
Thats it. For almost all users, you don't have to worry about anything else. All of the other aspects of
wasmtime_wasi{,_http}::add_to_linker_{sync,async}
keep working exactly like before. Semantically, nothing changed. TheWasiView
andWasiHttpView
traits grew a common implies on theIoView
trait, i.e.trait WasiView: IoView {...}
,trait WasiHttpView: IoView {...}
.More details
You probably don't care about these, unless you are doing low level linker stuff and probably can just read the source code to figure it out.
There's a new
IoImpl
struct that the wasi-io Host trait impls, instead ofWasiImpl
. LikeWasiImpl
this struct is#[repr(transparent)]
. Anytime you were constructing aWasiImpl(t)
, you now constructWasiImpl(IoImpl(t))
, e.g. https://github.com/bytecodealliance/wasmtime/pull/10016/files#diff-a6ec073ba5d074b65f0bcbf8df48af2b126d65034398905bf74d8f10a5c5f8b6R288If you're using the wasmtime-wit-bindgen generated add_to_linker_ funcs directly, you'll need a variant on the
type_annotate
hack to get rustc to infer IoView in the right places, e.g. https://github.com/bytecodealliance/wasmtime/pull/10016/files#diff-a6ec073ba5d074b65f0bcbf8df48af2b126d65034398905bf74d8f10a5c5f8b6R287.