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

wiggle: support for Rust async #2701

Merged
merged 7 commits into from
Mar 5, 2021
Merged

wiggle: support for Rust async #2701

merged 7 commits into from
Mar 5, 2021

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Mar 3, 2021

Following #2434, wiggle now supports async functions. Users of the wiggle macro can specify which functions should be async with a new async_ section of the settings.

The new integration test of wasmtime-wiggle shows async support working end-to-end: https://github.com/bytecodealliance/wasmtime/pull/2701/files#diff-9b1eee5f3bd355b0c43aa2ec5d647ddcc085d50f81324540cdbbce39ca6de21b

I rewrote a bunch of the doc test to document the new functionality, plus other earlier changes that had been left out: https://github.com/bytecodealliance/wasmtime/blob/1e4cde62ab510f6c5b551ab73203b48bff707635/crates/wiggle/macro/src/lib.rs

During this work I realized that the wiggle::from_witx macro did not need to take the user's so-called ctx type as an argument to the macro. Instead, all code generated by wiggle is generic - it will accept any type which implements the required traits. (Async lifetime issues meant it was easier to make this particular type generic rather than concrete, because lifetime parameters can't be elided in some async fn contexts.) The wasmtime-wiggle integration does still require the user to specify a concrete ctx type.

Over in crates/wasmtime, I added a missing function Linker::instantiate_async, for use with async stores.

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Mar 3, 2021
@github-actions
Copy link

github-actions bot commented Mar 3, 2021

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

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

Learn more.

@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Mar 4, 2021
@github-actions
Copy link

github-actions bot commented Mar 4, 2021

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

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

Learn more.

@pchickey pchickey marked this pull request as ready for review March 5, 2021 01:36
* ctx parameter no longer accepted by wiggle::from_witx macro.
* optional async_ parameter specifies which functions are async.
* re-export async_trait::async_trait, so users don't have to take a dep.
wiggle-macro doc tests weren't being run, so docs had gotten out of
sync.
} else if lookahead.peek(kw::errors) {
input.parse::<kw::errors>()?;
input.parse::<Token![:]>()?;
Ok(ConfigField::Error(input.parse()?))
} else if lookahead.peek(kw::async_) {
input.parse::<kw::async_>()?;
Copy link
Member

Choose a reason for hiding this comment

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

Could this parse the async keyword itself to avoid the underscore? (given this is a macro I'd assume we could do whatever syntax we wanted here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I don't know why I never thought of that.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants