Support reference types in the C API #1996
Conversation
fitzgen
force-pushed
the
ref-types-in-c-api
branch
from
5ba1e64
to
2986e1e
Compare
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:c-api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
There was a problem hiding this comment.
Thanks for this! (especially the doc updates and examples)
I'm left with a feeling though that this is a bit overly-complicated for externref in the C API. I'm a bit worried about the multiple layers of allocations and indirection with Option and such. I think it's also a bit confusing to try to represent both functions and externref in wasm_ref_t just because that's the way the C API is right now.
A possible alternative that I've been thinking of would look something like this:
- The
wasm_ref_tis still either a funcref or externref, but it's not tagged internally with what it is. - Usage of
*mut wasm_ref_tis given meaning inwasm_val_tbased on the union's tag - Usage of
*mut wasm_ref_tinwasm_table_*APIs is given meaning based on the table's type (this is a vector for UB since we're assuming you got things right). - We could add
wasmtime_table_*functions which operate onwasm_val_t(similar to our own APIs in the Rust API) to "fix" the UB - The representation of
funcrefwould exactly bewasm_func_t, so thewasm_ref_tpointer would simply be a castedwasm_func_t - The representation of
externrefwould ideally be the*mut VMExternDataunderlying pointer, but we'd have to add Rust APIs for this (which while doing that seems plausible to me feels like we can defer that to later and doBoxfor now). - The
wasm_ref_*APIs would probably all continue to just abort the process.
In my opinion the existing APIs in wasm.h are basically not suitable for extenref/funcref right now. They might make more sense in a future world with anyref but it feels premature to design around that possible future.
|
I think I am going to go only one step towards the "raw" |
|
Hm ok that can work, but trying to think through this, presumably the reason is that Given that it seems that to support that API we'd need to have internal tagging. I'll raise an issue on the wasm-c-api repo as well. |
fitzgen
force-pushed
the
ref-types-in-c-api
branch
from
5af32d4
to
4126617
Compare
fitzgen
force-pushed
the
ref-types-in-c-api
branch
from
4126617
to
86328b3
Compare
This required that `wasm_val_t` have a `Drop` implementation, an explicit `Clone` implementation, and no longer be `Copy`, which rippled out through the crate a bit. Additionally, `wasm_func_call` and friends were creating references to uninitialized data for its out pointers and assigning to them. As soon as `wasm_val_t` gained a `Drop` impl and tried to drop the old value of the assignment (which is uninitialized data), then things blew up. The fix is to properly represent the out pointers with `MaybeUninit`, and use `ptr::write` to initialize them without dropping the old data. Part of bytecodealliance#929
This commit adds APIs to create new `externref` values (with and without finalizers) and to get an `externref`'s wrapped data.
…rnref_new_with_finalizer`
The C API prefers not to return structs by value. Same for `wasmtime_externref_new_with_finalizer`.
…erence Same for `wasm_table_grow` and `wasm_table_new` and their `init` values.
fitzgen
force-pushed
the
ref-types-in-c-api
branch
from
86328b3
to
89603bc
Compare
Use `anyhow` for nice errors and provide error context on commands that we run.
It is created by the `run-examples` crate.
The canonical examples tests will be the examples run via `cargo run -p run-examples` from now on. Those tests have two advantages over the ones being deleted: 1. They will automatically pick up new examples and run/test them. 2. They support all of Linux, MacOS, and Windows, while the tests being deleted are Linux only.
fitzgen
force-pushed
the
ref-types-in-c-api
branch
2 times, most recently
from
c53a6a6
to
509092d
Compare
We were accidentally always running the `fib-debug/main` example because of shenanigans with alphabetical ordering and hard coding "main.exe" as the command we run. Now we properly detect which example we built and run the appropriate executable.
fitzgen
force-pushed
the
ref-types-in-c-api
branch
from
509092d
to
3638dba
Compare
See each commit message for details. The first commit in particular has a bit of rippling out due to
wasm_val_tnot being POD anymore.