alexcrichton opened PR #1384 from linking
to master
:
This commit adds a new type to the
wasmtime
crate, aLinker
. This
linker is intended to vastly simplify callingInstance::new
by easily
performing name resolution and incrementally defining state over time.
The goal here is to start down a path of making linking wasm modules in
wasmtime
a first-class and ergonomic operation. This is highly likely
to evolve over time and get tweaked through releases as we iterate
towards a design well-suited forwasmtime
, but this is intended to at
least be the initial foundation for such functionality.This commit additionally also adds a C API for the linker and switches
the existing linking examples to using this linker in both Rust and C.One piece of future work I'd like to tackle next is to integrate WASI
into thewasmtime
crate in a more first-class manner. This [Linker
]
type provides a great location to hook into the instantiation process to
easily instantiate modules with WASI imports. That's a relatively large
refactoring for now though and I figured it'd be best left for a
different time.Closes #727
alexcrichton updated PR #1384 from linking
to master
:
This commit adds a new type to the
wasmtime
crate, aLinker
. This
linker is intended to vastly simplify callingInstance::new
by easily
performing name resolution and incrementally defining state over time.
The goal here is to start down a path of making linking wasm modules in
wasmtime
a first-class and ergonomic operation. This is highly likely
to evolve over time and get tweaked through releases as we iterate
towards a design well-suited forwasmtime
, but this is intended to at
least be the initial foundation for such functionality.This commit additionally also adds a C API for the linker and switches
the existing linking examples to using this linker in both Rust and C.One piece of future work I'd like to tackle next is to integrate WASI
into thewasmtime
crate in a more first-class manner. This [Linker
]
type provides a great location to hook into the instantiation process to
easily instantiate modules with WASI imports. That's a relatively large
refactoring for now though and I figured it'd be best left for a
different time.Closes #727
peterhuene submitted PR Review.
peterhuene created PR Review Comment:
It's a little odd to me that
Linker::instantiate
needs to be&mut self
. Perhaps anLinker::insert_key
andLinker::get_key
method, where the former does whatLinker::import_key
is doing now and the latter returnsOption<ImportKey>
after checking if the given strings are present in the data structures without interning? That would remove the requirement of this being&mut self
.
peterhuene created PR Review Comment:
Should this (and the corresponding Rust impl) be
const wasm_instance_t*
?
peterhuene submitted PR Review.
peterhuene created PR Review Comment:
Should this (and the corresponding Rust impl) be
const wasi_instance_t*
?
peterhuene created PR Review Comment:
WASM_API_EXTERN own wasmtime_linker_t* wasmtime_linker_new(wasm_store_t* store);
peterhuene created PR Review Comment:
Assuming
Linker::instantiate
can become&self
, this (and the corresponding Rust impl) should probably beconst wasmtime_linker_t *
.
peterhuene created PR Review Comment:
Extreme nit:
record_instantiate
implies a record is kept some somewhere to me. Perhapshandle_instantiation
since it takesResult<Instance>
?
peterhuene created PR Review Comment:
Should this (and the corresponding Rust impl) be
const wasm_module_t*
?
peterhuene edited PR Review Comment.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
In general builders in Rust have
&mut self
even on the build methods, primarily for future-flexibility. That being said given the way I envision this being used I think we'll want a guarantee that&mut self
isn't needed and that mutations don't happen. In that sense I agree, let's shoot for&self
here.
alexcrichton updated PR #1384 from linking
to master
:
This commit adds a new type to the
wasmtime
crate, aLinker
. This
linker is intended to vastly simplify callingInstance::new
by easily
performing name resolution and incrementally defining state over time.
The goal here is to start down a path of making linking wasm modules in
wasmtime
a first-class and ergonomic operation. This is highly likely
to evolve over time and get tweaked through releases as we iterate
towards a design well-suited forwasmtime
, but this is intended to at
least be the initial foundation for such functionality.This commit additionally also adds a C API for the linker and switches
the existing linking examples to using this linker in both Rust and C.One piece of future work I'd like to tackle next is to integrate WASI
into thewasmtime
crate in a more first-class manner. This [Linker
]
type provides a great location to hook into the instantiation process to
easily instantiate modules with WASI imports. That's a relatively large
refactoring for now though and I figured it'd be best left for a
different time.Closes #727
peterhuene submitted PR Review.
alexcrichton updated PR #1384 from linking
to master
:
This commit adds a new type to the
wasmtime
crate, aLinker
. This
linker is intended to vastly simplify callingInstance::new
by easily
performing name resolution and incrementally defining state over time.
The goal here is to start down a path of making linking wasm modules in
wasmtime
a first-class and ergonomic operation. This is highly likely
to evolve over time and get tweaked through releases as we iterate
towards a design well-suited forwasmtime
, but this is intended to at
least be the initial foundation for such functionality.This commit additionally also adds a C API for the linker and switches
the existing linking examples to using this linker in both Rust and C.One piece of future work I'd like to tackle next is to integrate WASI
into thewasmtime
crate in a more first-class manner. This [Linker
]
type provides a great location to hook into the instantiation process to
easily instantiate modules with WASI imports. That's a relatively large
refactoring for now though and I figured it'd be best left for a
different time.Closes #727
alexcrichton merged PR #1384.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
This should have a
?
to handle errors.
alexcrichton created PR Review Comment:
Oh good eye, I've added a commit to https://github.com/bytecodealliance/wasmtime/pull/1391 which denies warnings in doctests to catch this
alexcrichton submitted PR Review.
Last updated: Dec 23 2024 at 13:07 UTC