Skip to content
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

Evolving the API of the wasmtime crate #708

Closed
27 tasks done
alexcrichton opened this issue Dec 12, 2019 · 6 comments
Closed
27 tasks done

Evolving the API of the wasmtime crate #708

alexcrichton opened this issue Dec 12, 2019 · 6 comments
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself

Comments

@alexcrichton
Copy link
Member

alexcrichton commented Dec 12, 2019

I've been reviewing the wasmtime crate from a Rust API perspective and ended up realizing that there's actually quite a few changes that I would like to make to the crate. I think that these changes are far too large so simply send in a PR, so I wanted to make sure that we had some discussion of this first!

In this issue I hope to lay out a vision for an end-state wasmtime crate and what the API might look like. I'm assuming that we can incrementally reach this end-goal over time and the exact route through which we get here isn't too too important. In any case, I'm curious if others have thoughts on all this!

Some of these items below may warrant their own separate issue as well, but I wanted to make sure that I had this all written down in one location first

High-level changes

  • All wasmtime-* crates should be private dependencies
  • Store should implement Default
  • Document everything
  • Update Send and Sync of all types
  • Internal reference counting instead of HostRef, HostRef is hidden
  • No methods should use raw identifiers
  • Instance is no longer reference counted to ensure that it's Send meaning globals/tables/functions are no longer reference counted either.

Config

  • Should remove methods referencing foreign wasmtime-* types
  • Should assert Send and Sync

Engine

  • Should implement Default (uses default Config)
  • Should assert Send and Sync
  • Should internally reference count (implements Clone)

Store

Module

Instance

  • Constructor shouldn't require Store (inferred from Module)
  • Constructor's return should expose Trap (handled if Trap is an Error)
  • Should expose module and store accessor
  • ~~Should be Send ~~ (moved to Reconciling wasmtime::Instance and Send #793)
    • Internally not reference counted
    • Accessing tables/etc all return a reference, not an owned type
  • find_export_by_name => get_export

Global, Table, Memory

  • Remove all internal reference counts
    • Memory should be fine to leave with an internal Arc
  • Everything should be Send, only Memory should be Sync (moved to Reconciling wasmtime::Instance and Send #793)
  • Table::grow shouldn't need mut
  • Global::set shouldn't need mut
  • type methods renamed to ty
  • Memory::grow shouldn't need mut

These types are only accessed via a reference when fetched through an Instance.

Func

This type I think needs a lot of improvements. I think it's best to separate out these concerns into a separate issue, however. Some high-level unbaked thoughts are:

  • Right now it's very allocation-heavy, probably too much so
  • There should be a way of taking a function pointer (as a usize) and registering it with a type signature. This would skip all trampolines/etc and we'd document the expected ABI. This would be an unsafe method.
  • There should also be a method of extracting an internal function pointer with the right ABI. You would then manually assert the function type as necessary and would unsafely transmute the function pointer returned to a function pointer of the right type.
  • Somehow data payloads are handled for the above "raw" methods, haven't fully thought this through...

MemoryType, TableType, GlobalType

  • Bikeshed on the name Mutability, otherwise seems good!

ImportType, ExportType

  • a little allocation-heavy, but probably fine

FuncType

  • Feels a bit allocation-heavy right now
  • Maybe intern in a Store, but probably fine for now.

Trap

  • Implement the Error trait
  • Ensure it's pointer-sized
  • Ensure it's Clone, Send, and Sync

Val

  • Commit to "ownership semantics"
  • AnyRef will become some custom form of Rc defined by wasmtime itself
    • jit will have its own refcounting management for tables/globals/etc (needs to be implemented)
  • Let's do Box<u128> for now to avoid blowing up the size
  • Enusring "ownership semantics" leaves room for implementing interface types strings eventually
  • Consider making Val an entirely opaque struct, perhaps with a decode method which returns a full enum. We may not want to commit to a public byte-by-byte representation of a Val just yet. This seems like an advanced concern we can punt though.

WASI

This is now split off to #727

  • Somehow we need to add WASI support to the wasmtime crate
  • "bare minimum" is fn wasi_unstable(&WasiConfiguration) -> HashMap<String, Extern>
  • Perhaps related to the name resolution point below?
    • Or perhaps create an Instance somehow?

Name Resolution

This is now split off to #727

Instantiation is a bit wonky where you have to line up imports 1:1 with the expected imports of the module. We should explore ideas where we have a more name resolution based mechanism which leverages the module system. Would perhaps make it much easier to slot in WASI or slot in a module. Pretty tricky API though so we'd have to think elsewhere about this.

@eminence
Copy link
Contributor

One of the things I'm interested in doing is creating a custom sandbox, where I provide some custom functions for a wasm module to import, and some wasi functions (but maybe not all of the wasi functions). I think your wasi_unstable "bare minimum" function handles this case, by allowing me to pick from the returned HashMap and use whatever ones I want during instantiation. I can definitely see how this would interact with name resolution (I'd like to be able to detect if the module I'm running is importing a wasi function I'm unwilling to provide them)

One of the things that confused me about the current version of Callable is how you store your results in a &mut [Val]. Just based off the function signature, one of the things I tried first was:

    fn call(&self, _params: &[Val], results: &mut [Val]) -> Result<(), HostRef<Trap>> {
        if let Some(Val::I32(v)) = results.get_mut(0) {
            *v = 42;
        } // else return trap
    }

The runtime error you get here is fairly inscrutable. I don't have a concrete proposal here, but I wanted to mention this minor papercut.

@cedric-h
Copy link
Contributor

As I was hiding HostRef in the Module API in my PR #696 I noticed that once I removed the #[derive(Clone)] from the ModuleInner, the compiler began to issue a complaint, namely that the store field wasn't actually ever being used. I've considered maybe adding a #[allow(dead_code)] to the field for now, but perhaps I should just go ahead and add a stores accessor (as you suggest up there) instead?

@cedric-h
Copy link
Contributor

cedric-h commented Dec 13, 2019

I've been looking for a scripting language to embed into some of my Rust game development projects, to facilitate scripting (with hot reloading!) custom logic and events for levels. Along the way, I've tried a bunch of different languages and have developed opinions about their APIs. So without further ado: A review of some Rusty embeddable scripting language APIs.

Dyon

https://github.com/PistonDevelopers/dyon/blob/master/examples/call.rs
I very much dislike Dyon's scripting API.
In terms of features, most things are there (Dyon can return Rust structs to your Rust host code) but the parts that you'd like to be able to control, like deciding how Dyon objects are turned into Rust structs, are hidden from you using really arcane macros, and the parts that you don't care so much about, like wrapping everything in Arc and declaring function type signatures by hand using Dyon's enums for that... aren't. It makes exposing a Rust API to Dyon scripts really annoying, and ultimately drove me away from using Dyon ever again.

Forge

https://github.com/zesterer/forge/blob/master/examples/callbacks.rs
I'm actually very fond of Forge's embedding API.
You can directly pass globals and functions to Forge's API using the builder pattern, which is patently Rusty and very ergonomic. I ended up not using Forge simply because the language itself lacks many features, and because the embedding doesn't yet support returning Rust structs from Forge, one of the things I can't do without for my use case.

Rhai

https://github.com/jonathandturner/rhai
Rhai has very good documentation on their embedding API right in their README.MD.
The API is a little bit messy and could potentially be made cleaner by leveraging Rust's type system better, but the core concepts behind it are similar to Forge's (with a useful distinction between adding functions directly to the interpreting Engine and adding them to particular modules). Their embedding API also supports instantiating and manipulating Rust structs from Rhai. I ended up choosing not to use Rhai, however, because of the lack of features in the language (no for in loop) which prevented it from being ergonomic to use.

PyO3

https://pyo3.rs/master/python_from_rust.html
I was looking for an easily embeddable pure Rust solution for my scripting needs, but ultimately I found that the pure Rust ecosystem just wasn't quite there yet.
PyO3's wrapper around Python's C FFI fulfilled all of my needs and then some, and the macros were very helpful and seemed to work with me instead of against me as was the case with Dyon.
I was able to hack together an abuse of Rust's type system which allows me to take a Rust struct that's returned from Python, make sure it's one of the ECS components in my game, and then introduce it into the simulation. So far I've used this to allow Python to override objects in my map files as they're being loaded, so that a Python script can i.e. choose one of several possible enemy formations stored in the map file and choose to load just one. Aside from that, I haven't tried this yet, but all of the machinery is also there for enemy formations and even rooms and such to be generated from scratch (instead of from a map file) in Python.
https://github.com/cedric-h/hauntfall/blob/master/serv/src/config/level.rs

The ultimate solution to this problem, of course, is wasmtime, which is why I'm excited to see the Interface Types proposal implemented; that should remove most of the barriers that prevent its use in my (and the rest of the Rust gamedev community's) projects.

@joshtriplett
Copy link
Member

Instantiation is a bit wonky where you have to line up imports 1:1 with the expected imports of the module. We should explore ideas where we have a more name resolution based mechanism which leverages the module system. Would perhaps make it much easier to slot in WASI or slot in a module. Pretty tricky API though so we'd have to think elsewhere about this.

I would love to see functions provided from the host/embedder in the style of module exports, with interface types and names, and then have wasmtime handle name resolution.

Once we have linking, what if we have a new type for a "HostModule", with a module name and a set of named exported functions with types?

@alexcrichton
Copy link
Member Author

@eminence good point! I think that the details of Func and how that's created are definitely not ergonomic right now and it's something we want to improve. I think though it's probably best to separate out discussion from this issue since I think that will largely be a separable concern. I'm hoping we can get a lot of the skeleton of the API improved through this issue, and then we can do deep dives like #727 into different APIs.

@cedric-h I don't think we'll want to add allow(dead_code) but I think we largely just need to be careful about our incremental progress. So long as the tests all continue passing I think we're all good :)

@joshtriplett I've started a dedicated discussion for that at #727 since I think it'll be a big topic, want to take a look at the proposal there and see if it sounds like it'll meet your needs?

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 6, 2020
This commit removes the need to use `HostRef<Engine>` in the Rust API.
Usage is retained in the C API in one location, but otherwise `Engine`
can always be used directly.

This is the first step of progress on bytecodealliance#708 for the `Engine` type.
Changes here include:

* `Engine` is now `Clone`, and is documented as being cheap. It's not
  intended that cloning an engine creates a deep copy.
* `Engine` is now both `Send` and `Sync`, and asserted to be so.
* Usage of `Engine` in APIs no longer requires or uses `HostRef`.
alexcrichton added a commit that referenced this issue Jan 6, 2020
This commit removes the need to use `HostRef<Engine>` in the Rust API.
Usage is retained in the C API in one location, but otherwise `Engine`
can always be used directly.

This is the first step of progress on #708 for the `Engine` type.
Changes here include:

* `Engine` is now `Clone`, and is documented as being cheap. It's not
  intended that cloning an engine creates a deep copy.
* `Engine` is now both `Send` and `Sync`, and asserted to be so.
* Usage of `Engine` in APIs no longer requires or uses `HostRef`.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 6, 2020
This commit removes the public API usage of the internal
`CompilationStrategy` enumeration from the `Config` type in the
`wasmtime` crate. To do this the `enum` was copied locally into the
crate and renamed `Strategy`. The high-level description of this change
is:

* The `Config::strategy` method now takes a locally-defined `Strategy`
  enumeration instead of an internal type.

* The contents of `Strategy` are always the same, not relying on Cargo
  features to indicate which variants are present. This avoids
  unnecessary downstream `#[cfg]`.

* A `lightbeam` feature was added to the `wasmtime` crate itself to
  lightbeam compilation support.

* The `Config::strategy` method is now fallible. It returns a runtime
  error if support for the selected strategy wasn't compiled in.

* The `Strategy` enum is listed as `#[non_exhaustive]` so we can safely
  add variants over time to it.

This reduces the public crate dependencies of the `wasmtime` crate
itself, removing the need to reach into internal crates even more!

cc bytecodealliance#708
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 7, 2020
This commit removes the public API usage of the internal
`CompilationStrategy` enumeration from the `Config` type in the
`wasmtime` crate. To do this the `enum` was copied locally into the
crate and renamed `Strategy`. The high-level description of this change
is:

* The `Config::strategy` method now takes a locally-defined `Strategy`
  enumeration instead of an internal type.

* The contents of `Strategy` are always the same, not relying on Cargo
  features to indicate which variants are present. This avoids
  unnecessary downstream `#[cfg]`.

* A `lightbeam` feature was added to the `wasmtime` crate itself to
  lightbeam compilation support.

* The `Config::strategy` method is now fallible. It returns a runtime
  error if support for the selected strategy wasn't compiled in.

* The `Strategy` enum is listed as `#[non_exhaustive]` so we can safely
  add variants over time to it.

This reduces the public crate dependencies of the `wasmtime` crate
itself, removing the need to reach into internal crates even more!

cc bytecodealliance#708
alexcrichton added a commit that referenced this issue Jan 7, 2020
* Remove usage of `CompilationStrategy` from `Config`

This commit removes the public API usage of the internal
`CompilationStrategy` enumeration from the `Config` type in the
`wasmtime` crate. To do this the `enum` was copied locally into the
crate and renamed `Strategy`. The high-level description of this change
is:

* The `Config::strategy` method now takes a locally-defined `Strategy`
  enumeration instead of an internal type.

* The contents of `Strategy` are always the same, not relying on Cargo
  features to indicate which variants are present. This avoids
  unnecessary downstream `#[cfg]`.

* A `lightbeam` feature was added to the `wasmtime` crate itself to
  lightbeam compilation support.

* The `Config::strategy` method is now fallible. It returns a runtime
  error if support for the selected strategy wasn't compiled in.

* The `Strategy` enum is listed as `#[non_exhaustive]` so we can safely
  add variants over time to it.

This reduces the public crate dependencies of the `wasmtime` crate
itself, removing the need to reach into internal crates even more!

cc #708

* Fix fuzz targets

* Update nightly used to build releases

* Run rustfmt
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 7, 2020
This commit goes through the public API of the `wasmtime` crate and
removes the need for `HostRef<Store>`, as discussed in bytecodealliance#708. This commit
is accompanied with a few changes:

* The `Store` type now also implements `Default`, creating a new
  `Engine` with default settings and returning that.

* The `Store` type now implements `Clone`, and is documented as being a
  "cheap clone" aka being reference counted. As before there is no
  supported way to create a deep clone of a `Store`.

* All APIs take/return `&Store` or `Store` instead of `HostRef<Store>`,
  and `HostRef<T>` is left as purely a detail of the C API.

* The `global_exports` function is tagged as `#[doc(hidden)]` for now
  while we await its removal.

* The `Store` type is not yet `Send` nor `Sync` due to the usage of
  `global_exports`, but it is intended to become so eventually.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 7, 2020
This commit goes through the public API of the `wasmtime` crate and
removes the need for `HostRef<Store>`, as discussed in bytecodealliance#708. This commit
is accompanied with a few changes:

* The `Store` type now also implements `Default`, creating a new
  `Engine` with default settings and returning that.

* The `Store` type now implements `Clone`, and is documented as being a
  "cheap clone" aka being reference counted. As before there is no
  supported way to create a deep clone of a `Store`.

* All APIs take/return `&Store` or `Store` instead of `HostRef<Store>`,
  and `HostRef<T>` is left as purely a detail of the C API.

* The `global_exports` function is tagged as `#[doc(hidden)]` for now
  while we await its removal.

* The `Store` type is not yet `Send` nor `Sync` due to the usage of
  `global_exports`, but it is intended to become so eventually.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 7, 2020
This commit goes through the public API of the `wasmtime` crate and
removes the need for `HostRef<Store>`, as discussed in bytecodealliance#708. This commit
is accompanied with a few changes:

* The `Store` type now also implements `Default`, creating a new
  `Engine` with default settings and returning that.

* The `Store` type now implements `Clone`, and is documented as being a
  "cheap clone" aka being reference counted. As before there is no
  supported way to create a deep clone of a `Store`.

* All APIs take/return `&Store` or `Store` instead of `HostRef<Store>`,
  and `HostRef<T>` is left as purely a detail of the C API.

* The `global_exports` function is tagged as `#[doc(hidden)]` for now
  while we await its removal.

* The `Store` type is not yet `Send` nor `Sync` due to the usage of
  `global_exports`, but it is intended to become so eventually.
alexcrichton added a commit that referenced this issue Jan 7, 2020
* Remove the need for `HostRef<Store>`

This commit goes through the public API of the `wasmtime` crate and
removes the need for `HostRef<Store>`, as discussed in #708. This commit
is accompanied with a few changes:

* The `Store` type now also implements `Default`, creating a new
  `Engine` with default settings and returning that.

* The `Store` type now implements `Clone`, and is documented as being a
  "cheap clone" aka being reference counted. As before there is no
  supported way to create a deep clone of a `Store`.

* All APIs take/return `&Store` or `Store` instead of `HostRef<Store>`,
  and `HostRef<T>` is left as purely a detail of the C API.

* The `global_exports` function is tagged as `#[doc(hidden)]` for now
  while we await its removal.

* The `Store` type is not yet `Send` nor `Sync` due to the usage of
  `global_exports`, but it is intended to become so eventually.

* Touch up comments on some examples

* Run rustfmt
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 8, 2020
This commit continues previous work and also bytecodealliance#708 by removing the need
to use `HostRef<Module>` in the API of the `wasmtime` crate. The API
changes performed here are:

* The `Module` type is now itself internally reference counted.
* The `Module::store` function now returns the `Store` that was used to
  create a `Module`
* Documentation for `Module` and its methods have been expanded.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 8, 2020
This commit removes the `unsafe impl Send` for `Trap` by removing the
internal `HostRef` and leaving `HostRef` entirely as an implementation
detail of the C API.

cc bytecodealliance#708
data-pup pushed a commit to data-pup/wasmtime that referenced this issue Jan 8, 2020
This commit removes the need to use `HostRef<Engine>` in the Rust API.
Usage is retained in the C API in one location, but otherwise `Engine`
can always be used directly.

This is the first step of progress on bytecodealliance#708 for the `Engine` type.
Changes here include:

* `Engine` is now `Clone`, and is documented as being cheap. It's not
  intended that cloning an engine creates a deep copy.
* `Engine` is now both `Send` and `Sync`, and asserted to be so.
* Usage of `Engine` in APIs no longer requires or uses `HostRef`.
data-pup pushed a commit to data-pup/wasmtime that referenced this issue Jan 8, 2020
…#764)

* Remove usage of `CompilationStrategy` from `Config`

This commit removes the public API usage of the internal
`CompilationStrategy` enumeration from the `Config` type in the
`wasmtime` crate. To do this the `enum` was copied locally into the
crate and renamed `Strategy`. The high-level description of this change
is:

* The `Config::strategy` method now takes a locally-defined `Strategy`
  enumeration instead of an internal type.

* The contents of `Strategy` are always the same, not relying on Cargo
  features to indicate which variants are present. This avoids
  unnecessary downstream `#[cfg]`.

* A `lightbeam` feature was added to the `wasmtime` crate itself to
  lightbeam compilation support.

* The `Config::strategy` method is now fallible. It returns a runtime
  error if support for the selected strategy wasn't compiled in.

* The `Strategy` enum is listed as `#[non_exhaustive]` so we can safely
  add variants over time to it.

This reduces the public crate dependencies of the `wasmtime` crate
itself, removing the need to reach into internal crates even more!

cc bytecodealliance#708

* Fix fuzz targets

* Update nightly used to build releases

* Run rustfmt
data-pup pushed a commit to data-pup/wasmtime that referenced this issue Jan 8, 2020
* Remove the need for `HostRef<Store>`

This commit goes through the public API of the `wasmtime` crate and
removes the need for `HostRef<Store>`, as discussed in bytecodealliance#708. This commit
is accompanied with a few changes:

* The `Store` type now also implements `Default`, creating a new
  `Engine` with default settings and returning that.

* The `Store` type now implements `Clone`, and is documented as being a
  "cheap clone" aka being reference counted. As before there is no
  supported way to create a deep clone of a `Store`.

* All APIs take/return `&Store` or `Store` instead of `HostRef<Store>`,
  and `HostRef<T>` is left as purely a detail of the C API.

* The `global_exports` function is tagged as `#[doc(hidden)]` for now
  while we await its removal.

* The `Store` type is not yet `Send` nor `Sync` due to the usage of
  `global_exports`, but it is intended to become so eventually.

* Touch up comments on some examples

* Run rustfmt
alexcrichton added a commit that referenced this issue Jan 8, 2020
* Remove unsafety from `Trap` API

This commit removes the `unsafe impl Send` for `Trap` by removing the
internal `HostRef` and leaving `HostRef` entirely as an implementation
detail of the C API.

cc #708

* Run rustfmt
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 8, 2020
This commit continues previous work and also bytecodealliance#708 by removing the need
to use `HostRef<Module>` in the API of the `wasmtime` crate. The API
changes performed here are:

* The `Module` type is now itself internally reference counted.
* The `Module::store` function now returns the `Store` that was used to
  create a `Module`
* Documentation for `Module` and its methods have been expanded.
alexcrichton added a commit that referenced this issue Jan 8, 2020
* Remove the need for `HostRef<Module>`

This commit continues previous work and also #708 by removing the need
to use `HostRef<Module>` in the API of the `wasmtime` crate. The API
changes performed here are:

* The `Module` type is now itself internally reference counted.
* The `Module::store` function now returns the `Store` that was used to
  create a `Module`
* Documentation for `Module` and its methods have been expanded.

* Fix compliation of test programs harness

* Fix the python extension

* Update `CodeMemory` to be `Send + Sync`

This commit updates the `CodeMemory` type in wasmtime to be both `Send`
and `Sync` by updating the implementation of `Mmap` to not store raw
pointers. This avoids the need for an `unsafe impl` and leaves the
unsafety as it is currently.

* Fix a typo
alexcrichton added a commit that referenced this issue Jan 8, 2020
This commit continues previous work and also #708 by removing the need
to use `HostRef<Module>` in the API of the `wasmtime` crate. The API
changes performed here are:

* The `Module` type is now itself internally reference counted.
* The `Module::store` function now returns the `Store` that was used to
  create a `Module`
* Documentation for `Module` and its methods have been expanded.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 9, 2020
This commit removes all remaining usages of `HostRef` in the public API
of the `wasmtime` crate. This involved a number of API decisions such
as:

* None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef`
* All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now.
* Methods called `type` are renamed to `ty` to avoid typing `r#type`.
* Methods requiring mutability for external items now no longer require
  mutability. The mutable reference here is sort of a lie anyway since
  the internals are aliased by the underlying module anyway. This
  affects:
  * `Table::set`
  * `Table::grow`
  * `Memory::grow`
  * `Instance::set_signal_handler`
* The `Val::FuncRef` type is now no longer automatically coerced to
  `AnyRef`. This is technically a breaking change which is pretty bad,
  but I'm hoping that we can live with this interim state while we sort
  out the `AnyRef` story in general.
* The implementation of the C API was refactored and updated in a few
  locations to account for these changes:
  * Accessing the exports of an instance are now cached to ensure we
    always hand out the same `HostRef` values.
  * `wasm_*_t` for external values no longer have internal cache,
    instead they all wrap `wasm_external_t` and have an unchecked
    accessor for the underlying variant (since the type is proof that
    it's there). This makes casting back and forth much more trivial.

This is all related to bytecodealliance#708 and while there's still more work to be done
in terms of documentation, this is the major bulk of the rest of the
implementation work on bytecodealliance#708 I believe.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 10, 2020
This commit removes all remaining usages of `HostRef` in the public API
of the `wasmtime` crate. This involved a number of API decisions such
as:

* None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef`
* All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now.
* Methods called `type` are renamed to `ty` to avoid typing `r#type`.
* Methods requiring mutability for external items now no longer require
  mutability. The mutable reference here is sort of a lie anyway since
  the internals are aliased by the underlying module anyway. This
  affects:
  * `Table::set`
  * `Table::grow`
  * `Memory::grow`
  * `Instance::set_signal_handler`
* The `Val::FuncRef` type is now no longer automatically coerced to
  `AnyRef`. This is technically a breaking change which is pretty bad,
  but I'm hoping that we can live with this interim state while we sort
  out the `AnyRef` story in general.
* The implementation of the C API was refactored and updated in a few
  locations to account for these changes:
  * Accessing the exports of an instance are now cached to ensure we
    always hand out the same `HostRef` values.
  * `wasm_*_t` for external values no longer have internal cache,
    instead they all wrap `wasm_external_t` and have an unchecked
    accessor for the underlying variant (since the type is proof that
    it's there). This makes casting back and forth much more trivial.

This is all related to bytecodealliance#708 and while there's still more work to be done
in terms of documentation, this is the major bulk of the rest of the
implementation work on bytecodealliance#708 I believe.
alexcrichton added a commit that referenced this issue Jan 10, 2020
* Remove `HostRef` from the `wasmtime` public API

This commit removes all remaining usages of `HostRef` in the public API
of the `wasmtime` crate. This involved a number of API decisions such
as:

* None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef`
* All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now.
* Methods called `type` are renamed to `ty` to avoid typing `r#type`.
* Methods requiring mutability for external items now no longer require
  mutability. The mutable reference here is sort of a lie anyway since
  the internals are aliased by the underlying module anyway. This
  affects:
  * `Table::set`
  * `Table::grow`
  * `Memory::grow`
  * `Instance::set_signal_handler`
* The `Val::FuncRef` type is now no longer automatically coerced to
  `AnyRef`. This is technically a breaking change which is pretty bad,
  but I'm hoping that we can live with this interim state while we sort
  out the `AnyRef` story in general.
* The implementation of the C API was refactored and updated in a few
  locations to account for these changes:
  * Accessing the exports of an instance are now cached to ensure we
    always hand out the same `HostRef` values.
  * `wasm_*_t` for external values no longer have internal cache,
    instead they all wrap `wasm_external_t` and have an unchecked
    accessor for the underlying variant (since the type is proof that
    it's there). This makes casting back and forth much more trivial.

This is all related to #708 and while there's still more work to be done
in terms of documentation, this is the major bulk of the rest of the
implementation work on #708 I believe.

* More API updates

* Run rustfmt

* Fix a doc test

* More test updates
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 13, 2020
This can be inferred from the `Module` argument. Additionally add a
`store` accessor to an `Instance` in case it's needed to instantiate
another `Module`.

cc bytecodealliance#708
@alexcrichton alexcrichton added the wasmtime:api Related to the API of the `wasmtime` crate itself label Jan 13, 2020
alexcrichton added a commit that referenced this issue Jan 13, 2020
* Don't require `Store` in `Instance` constructor

This can be inferred from the `Module` argument. Additionally add a
`store` accessor to an `Instance` in case it's needed to instantiate
another `Module`.

cc #708

* Update more constructors

* Fix a doctest

* Don't ignore store in `wasm_instance_new`

* Run rustfmt
@alexcrichton
Copy link
Member Author

With #814 I believe everything here is either fixed or split out into a separate issue, so I'm going to close!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

No branches or pull requests

4 participants