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

Add a wasmtime::Linker type #1384

Merged
merged 4 commits into from
Mar 24, 2020
Merged

Conversation

alexcrichton
Copy link
Member

This commit adds a new type to the wasmtime crate, a Linker. This
linker is intended to vastly simplify calling Instance::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 for wasmtime, 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 the wasmtime 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

This commit adds a new type to the `wasmtime` crate, a `Linker`. This
linker is intended to vastly simplify calling `Instance::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 for `wasmtime`, 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 the `wasmtime` 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 bytecodealliance#727
@github-actions github-actions bot added wasi Issues pertaining to WASI wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Mar 23, 2020
@github-actions
Copy link

Subscribe to Label Action

This issue or pull request has been labeled: "wasi", "wasmtime:api", "wasmtime:c-api"

Users Subscribed to "wasmtime:api"
Users Subscribed to "wasmtime:c-api"

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

Copy link
Member

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great and I really like how it simplifies things. I like this better than taking a list of instances to Module.Instantiate in the C# API, so I'll create a first-class Linker representation instead.

Just some minor feedback, but otherwise 👍 from me.

crates/c-api/include/wasmtime.h Outdated Show resolved Hide resolved
crates/c-api/include/wasmtime.h Outdated Show resolved Hide resolved
crates/c-api/include/wasmtime.h Outdated Show resolved Hide resolved
crates/api/src/linker.rs Outdated Show resolved Hide resolved
crates/c-api/include/wasmtime.h Outdated Show resolved Hide resolved
crates/c-api/include/wasmtime.h Outdated Show resolved Hide resolved
crates/c-api/src/lib.rs Outdated Show resolved Hide resolved
@alexcrichton
Copy link
Member Author

Updated!

@alexcrichton alexcrichton merged commit 0d4bde4 into bytecodealliance:master Mar 24, 2020
@alexcrichton alexcrichton deleted the linking branch March 24, 2020 02:02
/// let mut linker = Linker::new(&store);
/// let ty = GlobalType::new(ValType::I32, Mutability::Const);
/// let global = Global::new(&store, ty, Val::I32(0x1234))?;
/// linker.define("host", "offset", global);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have a ? to handle errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good eye, I've added a commit to #1391 which denies warnings in doctests to catch this

@KeenS KeenS mentioned this pull request May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improving Instance::new experience
3 participants