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

Update interface types #1289

Closed
wants to merge 22 commits into from
Closed

Update interface types #1289

wants to merge 22 commits into from

Conversation

silvanshade
Copy link
Contributor

This PR follows the discussion from #677 (comment) and continues Alex's work on #1013 (comment).

@github-actions github-actions bot added wasmtime:api Related to the API of the `wasmtime` crate itself fuzzing Issues related to our fuzzing infrastructure labels Mar 11, 2020
@github-actions
Copy link

Subscribe to Label Action

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

Users Subscribed to "fuzzing"
Users Subscribed to "wasmtime:api"

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

Learn more.

@bytecodealliance bytecodealliance deleted a comment from github-actions bot Mar 12, 2020
@bytecodealliance bytecodealliance deleted a comment from github-actions bot Mar 12, 2020
abrown pushed a commit to abrown/wasmtime that referenced this pull request Mar 18, 2020
alexcrichton and others added 22 commits April 3, 2020 14:13
This commit is the start of the modernization of the implementation of
interface types in wasmtime. This has a long way to go, and like the
previous iteration, chunks of it are already destined to get thrown
away.

The current state of affairs is that the implementation of interface
types in wasmtime feels very "bolted on" because, well, it was! The
`wasmtime-interface-types` crate is not well integrated with the
`wasmtime` API crate. Additionally it implements an older version of the
interface types proposal dating back to when it was still based on
WebIDL bindings. The goals of this PR are to integrate interface types
into the `wasmtime` API crate as well as update the implementation to
the current proposal, which is more instruction/adapter based.

Support here is built on top of the tools in the [wasm-interface-types]
repository which contain various pieces of functionality like parsing
the text format, parsing the binary format, stringifying the binary
format, and validating the binary format. These tools are intended to be
developed standalone from Wasmtime itself to encourage other users (like
`wasm-bindgen`, a generator for interface types).

This should also fix a number of integration issues around the interface
types demos we have (eventually) since wasm-bindgen is targeting the
wasm-interface-types family of crates rather than the older WebIDL
bindings proposals.

The current status of this commit/PR is that it is a step in the right
direction but incomplete. I started this work a few months ago when I
had some more time and have been occasionally rebasing it on the current
`wasmtime` master branch. Only a few simple instructions are supported
(and it's all via an interpreter, nothing compiled), and "hard" things
like strings have yet to be done.

What is implemented, however, falls into categories like:

* API integration into the `wasmtime` crate is sorted out
  * The `ValType` type is enhanced with interface types.
  * The `Val` type is enhanced with interface types values.
  * The `Config` type has a gate for the interface types proposal like
    it does all other proposals.
* The `wasmtime-interface-types` crate is deleted and all users are
  migrated to the `wasmtime` crate exclusively.
* The test harness has been enhanced with an invented `*.wast` syntax to
  cover interface types. I basically add things like `s8.const foo` as
  well as updated the parser to be able to parse `@interface`
  directives. Again this is sort of an invented format, but the
  intention is that we can easily write `*.wast` tests the same way for
  interface types how we do for the core spec.

I don't think we want to land this as-is because of how little of
interface types it implements. Additionally there's parallel work that's
been happening in Cranelift to jit interface types stubs which we may
wish to sync up with as well. In any case the main purpose of this
commit is to add fuel to the interface types discussion and hopefully
give us something concrete to work with as well.

[wasm-interface-types]: https://github.com/bytecodealliance/wasm-interface-types
@github-actions github-actions bot added wasi Issues pertaining to WASI wasmtime:c-api Issues pertaining to the C API. labels Apr 3, 2020
@github-actions
Copy link

github-actions bot commented Apr 3, 2020

Subscribe to Label Action

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

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

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

Learn more.

@silvanshade
Copy link
Contributor Author

Just want to give an update on this. Looks like I may need to set this aside for awhile since I don't have much time to focus on it and the branch is getting increasingly out of date. I tried to rebase and get things working again but there's still a lot of work to be done with unraveling recent merge conflicts.

I'd like to try to continue working on this eventually if nobody else gets around to it first, but I don't want to hold up progress or anything either. Realistically, it's probably going to be at least a few weeks before I'll have a chance to pick it up again. If someone wants to take over in the meantime, please feel free.

The main changes I made from Alex's original work was just the implementation of the remaining numeric instructions and then a lot of additional tests in integers.wast. It should be pretty easy to fold those changes back into the original branch or into another branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure 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.

3 participants