Improving Instance::new experience

[alexcrichton]

I think this is a large enough proposal and change that I'd like to split this off of #708. This issue is specifically about improving the experience of using Instance::new, which currently looks like this:

impl Instance {
    pub fn new(
        store: &HostRef<Store>,
        module: &HostRef<Module>,
        externs: &[Extern]
    ) -> Result<Instance>;
}

After #708 that will instead look like:

impl Instance {
    pub fn new(module: &Module, externs: &[Extern]) -> Result<Instance>;
}

Somewhat radically, I would propose that this is actually what we want on Instance! The idea here is that the list of externs is the same length of the list in module.imports(), and externs line up 1:1 with module.imports().

Now that being said this is a pretty bad API to use, and doesn't help instantiating things like WASI or dealing with name resolution at all. I think instead I think we should provide an API similar to the following (bikeshedding everything of course):

impl Instance {
    fn builder(module: &Module) -> InstanceBuilder;
    // ...
}

struct InstanceBuilder { ... }

impl InstanceBuilder {
    // Provide an import by module/name, panics if `module` or `name` is taken
    fn provide(&mut self, module: &str, name: &str, item: impl Into<Extern>) -> &mut Self;

    // Hook up an entire instance under the name `module`. Panics if `module` already used
    fn provide_instance(&mut self, module: &str, instance: &Instance) -> &mut Self;

    // Provides `wasi_unstable` module
    fn provide_wasi_snapshot0(&mut self, config: &WasiSnapshot0Config) -> &mut Self;

    // Provides `wasi_snapshot_preview1` module
    fn provide_wasi_snapshot1(&mut self, config: &WasiSnapshot1Config) -> &mut Self;

    // Does all the name resolution.
    fn build(&mut self) -> Result<Instance>;
}

Generally the idea here is that we keep Instance::new as the "low level" API of instantiating a module. That's for use when name resolution won't cut it for one reason or another. Something like InstanceBuilder will allow providing lots of extensions, as necessary, for hooking into name resolution and such. That way usage might look like:

let module = ...;
let instance = Instance::builder(&module)
    .provide_wasi_snapshot0(&Default::default())
    .provide("my-custom-api", "add", |a: i32, b: i32| a + b)
    .build()?;
// ...

The Into<Extern> part may be pretty tricky and still take some frobbing to get right. Super-long term I think it'd be awesome to do something like:

#[wasmtime::host_module]
trait MyWasmHostModule {
    fn function1(&self);
    fn function2(&self, input: &str) -> String;
}

// generates ...
// trait MyWasmHostModuleExt { ... }
// impl MyWasmHostModuleExt for wasmtime::InstanceBuilder {
//    fn provide_my_wasm_host_module(&mut self, instance: impl MyWasmHostModule)
//       ...
//    }
// }

impl MyWasmHostModule for MyStruct {
    // ...
}

Instace::builder(&module)
    .provide_my_wasm_host_module(&MyStruct)
    .build()?;

(but that's naturally much further in the future)

[eminence]

That builder looks pretty nice. The order you call each provides function will matter, since earlier calls take precedence over later calls.

What if you wanted to load in an entire module (like a wasi module), but then override a specific import? Maybe we could let later calls to provide() override earlier calls? So you could do something like:

let instance = Instance::builder(&module)
    .provide_wasi_snapshot1(&Default::default())
    .provide("wasi_snapshot_preview1", "environ_get", my_environ_get)
    .build()?;

Maybe a use case for this would be if you wanted to customize what data gets provided to the module (so in this example with environ_get, maybe you want to write a custom version that filters out potentially sensitive envvars).

If this behavior isn't wanted, perhaps all of the checking for duplicates could be deferred until build(), which could also take a config option to indicate if overriding is allowed:

impl InstanceBuilder {
   // Does all the name resolution.
   fn build(&mut self, allow_overrides: bool) -> Result<Instance>;
}

let instance = Instance::builder(&module)
    .provide("env", "add", |a: i32, b: i32| a + b)
    .provide("env", "add", |a: i32, b: i32| a + b)
    .build(false)?; // returns `Err`

[alexcrichton]

Yeah that was one case I was worried about, and I was thinking we'd just panic for now to be conservative if any imports look like they overlap one way or another.

[joshtriplett]

I really like this proposal! This would substantially simplify embedding and providing functions, both by taking all the necessary information from the function itself, and by automatically handling type mismatches in arguments. (We'll need some more advanced approaches to create an Export that let you provide functions that can handle different sets of arguments, but this handles the common case beautifully.)

Regarding duplicates, in the short term I'd suggest returning an error for duplicate exports, and perhaps providing a function to explicitly delete or replace an export by name if you want to intentionally override one.

[autodidaddict]

To address @eminence 's question about being able to override individual syscalls, what about the concept of middleware? In other words, inside the builder you can add a middleware trait object and it will get invoked before and potentially after each call (in either direction). This way we could choose to completely override/block a syscall, or we could add additional functionality, logging, tracing, etc.

Maybe...

let instance = Instance::builder()
  .foo()
  .something()
  .with_middleware(FileBlocker::new())
  .build()?;

[pepyakin]

Sometimes, it is useful to have an escape hatch for resolving imports when instantiating modules.

For example, wasm allows a module to import several functions with the same signature. I.e. something like this:

(import "env" "print" (param i32))
(import "env" "print" (param i32 i32))

Another example, is that sometimes you want to have full control over resolving of imports. Basically, having something like Resolver. The latest example I bumped into is when I need to resolve non-existent functions to some stub.

[alexcrichton]

I've posted a draft of this to #1384