bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
14.82k stars 1.24k forks source link

Components: Implementing traits in library without dyn #8764

Closed MendyBerger closed 2 weeks ago

MendyBerger commented 3 weeks ago

Currently, if you wanna implement the traits generated by component::bindgen in a library that other's are gonna use with custom types, you can't do it on T: YourTrait, you have to implement it on dyn YourTrait. At least that's what I gather from what wasi-http is doing.

I'm working on wasi-webgpu, where we'd really like one of our methods to be generic, but generics are not object-safe. Is there any way we can do that?

alexcrichton commented 3 weeks ago

Heh I was actually just talking with others about possibly changing some parts about wasi-http about this.

For your use case would something like this work?

pub struct MyWebgpuWrapper<T>(pub T);

pub trait MyWebgpuView { /* ... */ }

impl<T: MyWebgpuView> bindings::wasi::webgpu::interface::Host for MyWebgpuWrapper<T> { /* ... */ }

pub fn add_to_linker<T: MyWebgpuView>(linker: &mut Linker<T>) -> Result<()> {
    /* ... */
}

Basically having a newtype wrapper locally which is used internally in add_to_linker as a temporary value?

MendyBerger commented 3 weeks ago

Not sure I understand how this would work. Wouldn't this require the consumer of the wasi-webgpu library to implement Host manually? Or do you mean that all of this would be in library code?

alexcrichton commented 3 weeks ago

Oh the wasi-webgpu library would have impls that look like

impl<T: MyWebgpuView> bindings::wasi::webgpu::interface::Host for MyWebgpuWrapper<T>

so users wouldn't need to implement Host themselves. They only need to construct MyWebgpuWrapper<T> which is probably MyWebgpuWrapper(&mut my_stuff)

Also, to confirm, you're looking to provide a small trait of methods and then from that small trait of methods all other methods are implemented? That's what wasmtime-wasi/wasmtime-wasi-http are doing but if you're not leap frogging traits like this then most of this infrastructure isn't necessary

MendyBerger commented 3 weeks ago

Oh! I think I see it now! Host is implemented for a concrete type. And the concrete implementation won't conflict with the auto generated &mut T implementations because of the wrapper. Brilliant idea! Let me check if I can get this to work.

MendyBerger commented 3 weeks ago

@alexcrichton I was able to get it to work, thanks for the help!

I modeled it like wasi-http which seems to be doing the same thing.

It took me a couple of hours to figure this out though. Would it be possible for bindgen! to generate all the plumbing, and just expose a ready to use WasiFooImpl and add_to_linker? Figuring out exactly what needs to go in the add_to_linker like the type_annotate, as well as the requirement to implement WasiFooView on WasiFooImpl, &mut T: WasiFooView, and Box<T: WasiFooView> is a lot of work.

alexcrichton commented 3 weeks ago

Hm unfortuantely I don't think so. The WasiView trait is a handwritten trait and has no means of being automatically derived. Furthermore all implementations of the Host traits are implemented in terms of WasiView and additionally can't be auto-derived. For the same reasons nothing can auto-derive the impls and such too.

I'll also reiterate though that none of this is ideally necessary. I'd much rather have, for example Host for WasiCtx for all of WASI or something like:

pub struct WasiTemp<'a> {
    pub table: &'a mut ResourceTable, 
    pub ctx: &'a mut WasiCtx,
}

impl Host for WasiTemp<'_> { /* ... */ }

Or put another way I'd much rather to have Host for concrete types. The only reason WasiView exists is to mirror the design of WasiHttpView and the only reason that exists is the various trait methods it has for customizing how a request is dispatched for example. If you can get away with it I'd recommend not using a *View triat entirely and instead using a configuration context of some kind and everything is implemented in terms of that context.

pchickey commented 3 weeks ago

I think historically WasiView came before WasiHttpView, opposite of what your example is, but no matter. The real unfortunate necessity for all of this machinery is that all* bindings implementations on a store need to share the same ResourceTable, but have their own Ctx type for "everything else": WasiCtx, WasiHttpCtx, WebGpuCtx etc. The best way we came up with in wasmtime-wasi and wasmtime-wasi-http is to have a trait where you can borrow either the ctx or the table mutably, but it would work just as well with a struct like Alex describes where each of those mut borrows is a member of the struct.

alexcrichton commented 3 weeks ago

Ah yes true! One thing I'll also point out is that we haven't historically done something like WasiTemp<'a> because that wasn't possible until https://github.com/bytecodealliance/wasmtime/pull/8448 landed. Now with that we could in theory move all WASI bits, but the customization bits that use a trait still necessitate otherwise.

That being said a better yet design from what we have now, perhaps for wasi-http, would be to have WasiHttpCtx<T> where T: WasiHttpImplTrait where it's not the same as WasiView but just the customization methods. That way we could in fact use WasiTemp<'a> or something similar.

alexcrichton commented 2 weeks ago

I think this has been answered now so I'm going to close this, but if there are any lingering questions/clarifications feel free to comment and/or open a new issue.