Skip to content

Commit f6775a3

Browse files
authored
Replace GetHost with a function pointer, add HasData (#10770)
* Replace `GetHost` with a function pointer, add `HasData` This commit is a refactoring to the fundamentals of the `bindgen!` macro and the functions that it generates. Prior to this change the fundamental entrypoint generated by `bindgen!` was a function `add_to_linker_get_host` which takes a value of type `G: GetHost`. This `GetHost` implementation is effectively an alias for a closure whose return value is able to close over the parameter given lfietime-wise. The `GetHost` abstraction was added to Wasmtime originally to enable using any type that implements `Host` traits, not just `&mut U` as was originally supported. The definition of `GetHost` was _just_ right to enable a type such as `MyThing<&mut T>` to implement `Host` and a closure could be provided that could return it. At the time that `GetHost` was added it was known to be problematic from an understandability point of view, namely: * It has a non-obvious definition. * It's pretty advanced Rust voodoo to understand what it's actually doing * Using `GetHost` required lots of `for<'a> ...` in places which is unfamiliar syntax for many. * `GetHost` values couldn't be type-erased (e.g. put in a trait object) as we couldn't figure out the lifetime syntax to do so. Despite these issues it was the only known solution at hand so we landed it and kept the previous `add_to_linker` style (`&mut T -> &mut U`) as a convenience. While this has worked reasonable well (most folks just try to not look at `GetHost`) it has reached a breaking point in the WASIp3 work. In the WASIp3 work it's effectively now going to be required that the `G: GetHost` value is packaged up and actually stored inside of accessors provided to host functions. This means that `GetHost` values now need to not only be taken in `add_to_linker` but additionally provided to the rest of the system through an "accessor". This was made possible in #10746 by moving the `GetHost` type into Wasmtime itself (as opposed to generated code where it lived prior). While this worked with WASIp3 and it was possible to plumb `G: GetHost` safely around, this ended up surfacing more issues. Namely all "concurrent" host functions started getting significantly more complicated `where` clauses and type signatures. At the end of the day I felt that we had reached the end of the road to `GetHost` and wanted to search for alternatives, hence this change. The fundamental purpose of `GetHost` was to be able to express, in a generic fashion: * Give me a closure that takes `&mut T` and returns `D`. * The `D` type can close over the lifetime in `&mut T`. * The `D` type must implement `bindgen!`-generated traits. A realization I had was that we could model this with a generic associated type in Rust. Rust support for generic associated types is relatively new and not something I've used much before, but it ended up being a perfect model for this. The definition of the new `HasData` trait is deceptively simple: trait HasData { type Data<'a>; } What this enables us to do though is to generate `add_to_linker` functions that look like this: fn add_to_linker<T, D>( linker: &mut Linker<T>, getter: fn(&mut T) -> D::Data<'_>, ) -> Result<()> where D: HasData, for<'a> D::Data<'a>: Host; This definition here models `G: GetHost` as a literal function pointer, and the ability to close over the `&mut T` lifetime with type (not just `&mut U`) is expressed through the type constructor `type Data<'a>`). Ideally we could take a generic generic associated type (I'm not even sure what to call that), but that's not something Rust has today. Overall this felt like a much simpler way of modeling `GetHost` and its requirements. This plumbed well throughout the WASIp3 work and the signatures for concurrent functions felt much more appropriate in terms of complexity after this change. Taking this change to the limit means that `GetHost` in its entirety could be purged since all usages of it could be replaced with `fn(&mut T) -> D::Data<'a>`, a hopefully much more understandable type. This change is not all rainbows however, there are some gotchas that remain: * One is that all `add_to_linker` generated functions have a `D: HasData` type parameter. This type parameter cannot be inferred and must always be explicitly specified, and it's not easy to know what to supply here without reading documentation. Actually supplying the type parameter is quite easy once you know what to do (and what to fill in), but it may involve defining a small struct with a custom `HasData` implementation which can be non-obvious. * Another is that the `G: GetHost` value was previously a full Rust closure, but now it's transitioning to a function pointer. This is done in preparation for WASIp3 work where the function needs to be passed around, and doing that behind a generic parameter is more effort than it's worth. This means that embedders relying on the true closure-like nature here will have to update to using a function pointer instead. * The function pointer is stored in locations that require `'static`, and while `fn(T)` might be expected to be `'static` regardless of `T` is is, in fact, not. This means that practically `add_to_linker` requires `T: 'static`. Relative to just before this change this is a possible regression in functionality, but there orthogonal reasons beyond just this that we want to start requiring `T: 'static` anyway. That means that this isn't actually a regression relative to #10760, a related change. The first point is partially ameliorated with WASIp3 work insofar that the `D` type parameter will start serving as a location to specify where concurrent implementations are found. These concurrent methods don't take `&mut self` but instead are implemented for `T: HasData` types. In that sense it's more justified to have this weird type parameter, but in the meantime without this support it'll feel a bit odd to have this little type parameter hanging off the side. This change has been integrated into the WASIp3 prototyping repository with success. This has additionally been integrated into the Spin embedding which has one of the more complicated reliances on `*_get_host` functions known. Given that it's expected that while this is not necessarily a trivial change to rebase over it should at least be possible. Finally the `HasData` trait here has been included with what I'm hoping is a sufficient amount of documentation to at least give folks a spring board to understand it. If folks have confusion about this `D` type parameter my hope is they'll make their way to `HasData` which showcases various patterns for "librarifying" host implementations of WIT interfaces. These patterns are all used throughout Wasmtime and WASI currently in crates and tests and such. * Update expanded test expectations
1 parent f81c0dc commit f6775a3

File tree

125 files changed

+1848
-12631
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

125 files changed

+1848
-12631
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/component-macro/tests/codegen.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ macro_rules! gentest {
1212
async: true,
1313
});
1414
}
15-
mod concurrent {
16-
wasmtime::component::bindgen!({
17-
path: $path,
18-
async: true,
19-
concurrent_imports: true,
20-
concurrent_exports: true,
21-
});
22-
}
15+
// TODO: re-enable this when wasip3 is merged back into this repo
16+
// mod concurrent {
17+
// wasmtime::component::bindgen!({
18+
// path: $path,
19+
// async: true,
20+
// concurrent_imports: true,
21+
// concurrent_exports: true,
22+
// });
23+
// }
2324
mod tracing {
2425
wasmtime::component::bindgen!({
2526
path: $path,

crates/component-macro/tests/expanded.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@ macro_rules! genexpand {
1515
stringify: true,
1616
}))?;
1717

18-
process_expanded($path, "_concurrent", wasmtime::component::bindgen!({
19-
path: $path,
20-
async: true,
21-
concurrent_imports: true,
22-
concurrent_exports: true,
23-
stringify: true,
24-
}))?;
18+
// TODO: re-enable this when wasip3 is merged back into this repo
19+
// process_expanded($path, "_concurrent", wasmtime::component::bindgen!({
20+
// path: $path,
21+
// async: true,
22+
// concurrent_imports: true,
23+
// concurrent_exports: true,
24+
// stringify: true,
25+
// }))?;
2526

2627
process_expanded($path, "_tracing_async", wasmtime::component::bindgen!({
2728
path: $path,

crates/component-macro/tests/expanded/char.rs

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,16 @@ const _: () = {
144144
let indices = TheWorldIndices::new(&instance.instance_pre(&store))?;
145145
indices.load(&mut store, instance)
146146
}
147-
pub fn add_to_linker<T, U>(
147+
pub fn add_to_linker<T, D>(
148148
linker: &mut wasmtime::component::Linker<T>,
149-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
149+
get: fn(&mut T) -> D::Data<'_>,
150150
) -> wasmtime::Result<()>
151151
where
152+
D: wasmtime::component::HasData,
153+
for<'a> D::Data<'a>: foo::foo::chars::Host,
152154
T: 'static,
153-
U: foo::foo::chars::Host,
154155
{
155-
foo::foo::chars::add_to_linker(linker, get)?;
156+
foo::foo::chars::add_to_linker::<T, D>(linker, get)?;
156157
Ok(())
157158
}
158159
pub fn foo_foo_chars(&self) -> &exports::foo::foo::chars::Guest {
@@ -172,13 +173,14 @@ pub mod foo {
172173
/// A function that returns a character
173174
fn return_char(&mut self) -> char;
174175
}
175-
pub fn add_to_linker_get_host<T, G>(
176+
pub fn add_to_linker<T, D>(
176177
linker: &mut wasmtime::component::Linker<T>,
177-
host_getter: G,
178+
host_getter: fn(&mut T) -> D::Data<'_>,
178179
) -> wasmtime::Result<()>
179180
where
181+
D: wasmtime::component::HasData,
182+
for<'a> D::Data<'a>: Host,
180183
T: 'static,
181-
G: for<'a> wasmtime::component::GetHost<&'a mut T, Host: Host>,
182184
{
183185
let mut inst = linker.instance("foo:foo/chars")?;
184186
inst.func_wrap(
@@ -202,16 +204,6 @@ pub mod foo {
202204
)?;
203205
Ok(())
204206
}
205-
pub fn add_to_linker<T, U>(
206-
linker: &mut wasmtime::component::Linker<T>,
207-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
208-
) -> wasmtime::Result<()>
209-
where
210-
T: 'static,
211-
U: Host,
212-
{
213-
add_to_linker_get_host(linker, get)
214-
}
215207
impl<_T: Host + ?Sized> Host for &mut _T {
216208
/// A function that accepts a character
217209
fn take_char(&mut self, x: char) -> () {

crates/component-macro/tests/expanded/char_async.rs

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -150,16 +150,16 @@ const _: () = {
150150
let indices = TheWorldIndices::new(&instance.instance_pre(&store))?;
151151
indices.load(&mut store, instance)
152152
}
153-
pub fn add_to_linker<T, U>(
153+
pub fn add_to_linker<T, D>(
154154
linker: &mut wasmtime::component::Linker<T>,
155-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
155+
get: fn(&mut T) -> D::Data<'_>,
156156
) -> wasmtime::Result<()>
157157
where
158-
T: 'static,
159-
T: Send,
160-
U: foo::foo::chars::Host + Send,
158+
D: wasmtime::component::HasData,
159+
for<'a> D::Data<'a>: foo::foo::chars::Host + Send,
160+
T: Send + 'static,
161161
{
162-
foo::foo::chars::add_to_linker(linker, get)?;
162+
foo::foo::chars::add_to_linker::<T, D>(linker, get)?;
163163
Ok(())
164164
}
165165
pub fn foo_foo_chars(&self) -> &exports::foo::foo::chars::Guest {
@@ -180,14 +180,14 @@ pub mod foo {
180180
/// A function that returns a character
181181
async fn return_char(&mut self) -> char;
182182
}
183-
pub fn add_to_linker_get_host<T, G>(
183+
pub fn add_to_linker<T, D>(
184184
linker: &mut wasmtime::component::Linker<T>,
185-
host_getter: G,
185+
host_getter: fn(&mut T) -> D::Data<'_>,
186186
) -> wasmtime::Result<()>
187187
where
188-
T: 'static,
189-
G: for<'a> wasmtime::component::GetHost<&'a mut T, Host: Host + Send>,
190-
T: Send,
188+
D: wasmtime::component::HasData,
189+
for<'a> D::Data<'a>: Host + Send,
190+
T: Send + 'static,
191191
{
192192
let mut inst = linker.instance("foo:foo/chars")?;
193193
inst.func_wrap_async(
@@ -215,17 +215,6 @@ pub mod foo {
215215
)?;
216216
Ok(())
217217
}
218-
pub fn add_to_linker<T, U>(
219-
linker: &mut wasmtime::component::Linker<T>,
220-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
221-
) -> wasmtime::Result<()>
222-
where
223-
T: 'static,
224-
U: Host + Send,
225-
T: Send,
226-
{
227-
add_to_linker_get_host(linker, get)
228-
}
229218
impl<_T: Host + ?Sized + Send> Host for &mut _T {
230219
/// A function that accepts a character
231220
async fn take_char(&mut self, x: char) -> () {

crates/component-macro/tests/expanded/char_tracing_async.rs

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -150,16 +150,16 @@ const _: () = {
150150
let indices = TheWorldIndices::new(&instance.instance_pre(&store))?;
151151
indices.load(&mut store, instance)
152152
}
153-
pub fn add_to_linker<T, U>(
153+
pub fn add_to_linker<T, D>(
154154
linker: &mut wasmtime::component::Linker<T>,
155-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
155+
get: fn(&mut T) -> D::Data<'_>,
156156
) -> wasmtime::Result<()>
157157
where
158-
T: 'static,
159-
T: Send,
160-
U: foo::foo::chars::Host + Send,
158+
D: wasmtime::component::HasData,
159+
for<'a> D::Data<'a>: foo::foo::chars::Host + Send,
160+
T: Send + 'static,
161161
{
162-
foo::foo::chars::add_to_linker(linker, get)?;
162+
foo::foo::chars::add_to_linker::<T, D>(linker, get)?;
163163
Ok(())
164164
}
165165
pub fn foo_foo_chars(&self) -> &exports::foo::foo::chars::Guest {
@@ -180,14 +180,14 @@ pub mod foo {
180180
/// A function that returns a character
181181
async fn return_char(&mut self) -> char;
182182
}
183-
pub fn add_to_linker_get_host<T, G>(
183+
pub fn add_to_linker<T, D>(
184184
linker: &mut wasmtime::component::Linker<T>,
185-
host_getter: G,
185+
host_getter: fn(&mut T) -> D::Data<'_>,
186186
) -> wasmtime::Result<()>
187187
where
188-
T: 'static,
189-
G: for<'a> wasmtime::component::GetHost<&'a mut T, Host: Host + Send>,
190-
T: Send,
188+
D: wasmtime::component::HasData,
189+
for<'a> D::Data<'a>: Host + Send,
190+
T: Send + 'static,
191191
{
192192
let mut inst = linker.instance("foo:foo/chars")?;
193193
inst.func_wrap_async(
@@ -244,17 +244,6 @@ pub mod foo {
244244
)?;
245245
Ok(())
246246
}
247-
pub fn add_to_linker<T, U>(
248-
linker: &mut wasmtime::component::Linker<T>,
249-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
250-
) -> wasmtime::Result<()>
251-
where
252-
T: 'static,
253-
U: Host + Send,
254-
T: Send,
255-
{
256-
add_to_linker_get_host(linker, get)
257-
}
258247
impl<_T: Host + ?Sized + Send> Host for &mut _T {
259248
/// A function that accepts a character
260249
async fn take_char(&mut self, x: char) -> () {

crates/component-macro/tests/expanded/conventions.rs

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,16 @@ const _: () = {
146146
let indices = TheWorldIndices::new(&instance.instance_pre(&store))?;
147147
indices.load(&mut store, instance)
148148
}
149-
pub fn add_to_linker<T, U>(
149+
pub fn add_to_linker<T, D>(
150150
linker: &mut wasmtime::component::Linker<T>,
151-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
151+
get: fn(&mut T) -> D::Data<'_>,
152152
) -> wasmtime::Result<()>
153153
where
154+
D: wasmtime::component::HasData,
155+
for<'a> D::Data<'a>: foo::foo::conventions::Host,
154156
T: 'static,
155-
U: foo::foo::conventions::Host,
156157
{
157-
foo::foo::conventions::add_to_linker(linker, get)?;
158+
foo::foo::conventions::add_to_linker::<T, D>(linker, get)?;
158159
Ok(())
159160
}
160161
pub fn foo_foo_conventions(&self) -> &exports::foo::foo::conventions::Guest {
@@ -220,13 +221,14 @@ pub mod foo {
220221
/// Identifiers with the same name as keywords are quoted.
221222
fn bool(&mut self) -> ();
222223
}
223-
pub fn add_to_linker_get_host<T, G>(
224+
pub fn add_to_linker<T, D>(
224225
linker: &mut wasmtime::component::Linker<T>,
225-
host_getter: G,
226+
host_getter: fn(&mut T) -> D::Data<'_>,
226227
) -> wasmtime::Result<()>
227228
where
229+
D: wasmtime::component::HasData,
230+
for<'a> D::Data<'a>: Host,
228231
T: 'static,
229-
G: for<'a> wasmtime::component::GetHost<&'a mut T, Host: Host>,
230232
{
231233
let mut inst = linker.instance("foo:foo/conventions")?;
232234
inst.func_wrap(
@@ -330,16 +332,6 @@ pub mod foo {
330332
)?;
331333
Ok(())
332334
}
333-
pub fn add_to_linker<T, U>(
334-
linker: &mut wasmtime::component::Linker<T>,
335-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
336-
) -> wasmtime::Result<()>
337-
where
338-
T: 'static,
339-
U: Host,
340-
{
341-
add_to_linker_get_host(linker, get)
342-
}
343335
impl<_T: Host + ?Sized> Host for &mut _T {
344336
fn kebab_case(&mut self) -> () {
345337
Host::kebab_case(*self)

crates/component-macro/tests/expanded/conventions_async.rs

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -152,16 +152,16 @@ const _: () = {
152152
let indices = TheWorldIndices::new(&instance.instance_pre(&store))?;
153153
indices.load(&mut store, instance)
154154
}
155-
pub fn add_to_linker<T, U>(
155+
pub fn add_to_linker<T, D>(
156156
linker: &mut wasmtime::component::Linker<T>,
157-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
157+
get: fn(&mut T) -> D::Data<'_>,
158158
) -> wasmtime::Result<()>
159159
where
160-
T: 'static,
161-
T: Send,
162-
U: foo::foo::conventions::Host + Send,
160+
D: wasmtime::component::HasData,
161+
for<'a> D::Data<'a>: foo::foo::conventions::Host + Send,
162+
T: Send + 'static,
163163
{
164-
foo::foo::conventions::add_to_linker(linker, get)?;
164+
foo::foo::conventions::add_to_linker::<T, D>(linker, get)?;
165165
Ok(())
166166
}
167167
pub fn foo_foo_conventions(&self) -> &exports::foo::foo::conventions::Guest {
@@ -228,14 +228,14 @@ pub mod foo {
228228
/// Identifiers with the same name as keywords are quoted.
229229
async fn bool(&mut self) -> ();
230230
}
231-
pub fn add_to_linker_get_host<T, G>(
231+
pub fn add_to_linker<T, D>(
232232
linker: &mut wasmtime::component::Linker<T>,
233-
host_getter: G,
233+
host_getter: fn(&mut T) -> D::Data<'_>,
234234
) -> wasmtime::Result<()>
235235
where
236-
T: 'static,
237-
G: for<'a> wasmtime::component::GetHost<&'a mut T, Host: Host + Send>,
238-
T: Send,
236+
D: wasmtime::component::HasData,
237+
for<'a> D::Data<'a>: Host + Send,
238+
T: Send + 'static,
239239
{
240240
let mut inst = linker.instance("foo:foo/conventions")?;
241241
inst.func_wrap_async(
@@ -363,17 +363,6 @@ pub mod foo {
363363
)?;
364364
Ok(())
365365
}
366-
pub fn add_to_linker<T, U>(
367-
linker: &mut wasmtime::component::Linker<T>,
368-
get: impl Fn(&mut T) -> &mut U + Send + Sync + Copy + 'static,
369-
) -> wasmtime::Result<()>
370-
where
371-
T: 'static,
372-
U: Host + Send,
373-
T: Send,
374-
{
375-
add_to_linker_get_host(linker, get)
376-
}
377366
impl<_T: Host + ?Sized + Send> Host for &mut _T {
378367
async fn kebab_case(&mut self) -> () {
379368
Host::kebab_case(*self).await

0 commit comments

Comments
 (0)