Skip to content

Auto-generate the hostcalls module of wasi-common#846

Merged
alexcrichton merged 9 commits intobytecodealliance:masterfrom
alexcrichton:more-auto-generate-witx
Jan 22, 2020
Merged

Auto-generate the hostcalls module of wasi-common#846
alexcrichton merged 9 commits intobytecodealliance:masterfrom
alexcrichton:more-auto-generate-witx

Conversation

@alexcrichton
Copy link
Member

This commit updates the wasi-common crate to remove a souce of duplication from the *.witx files to what's listed in the source. Ideally we're now getting to the point where adding a WASI API means you update the *.witx file, then you write an implementation, then you're done. No further shims you need to write!

Some changes in this PR are:

  • This is built on Auto-generate shims for old wasi_unstable module #845
  • The hostcalls module, both for the current snapshot and old snapshot, are entirely generated from the *.witx files.
  • The code generated should almost match what we had before, except that &mut WasiCtx is required everywhere instead of sometimes using &WasiCtx and sometimes &mut WasiCtx. Note that the handwritten bindings continue to use a mixture.
  • The wasi-common-cbindgen crate has been removed since its functionality is easy enough to fold into the existing macro.

The general goal here is to continue to reduce duplication between all the WASI API definitions, ensuring that there's one source of truth for the signature and once source of truth for the implementation, and everything in the middle is auto-generated as necessary.

@alexcrichton alexcrichton requested a review from kubkon January 22, 2020 16:58
Copy link
Member

@kubkon kubkon left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of questions. Also, just an FYI so that we don't duplicate the effort that @pchickey and myself are currently working on an upgraded version of wig, the wiggle crate which should help us use the polyfill functionality of the witx crate to generate wasi_unstable and any future snapshots in a way that almost completely removes any unnecessary duplication in hostcalls implementation, etc. It should also help us auto-generate the memory.rs module.

@kubkon kubkon added the wasi:impl Issues pertaining to WASI implementation in Wasmtime label Jan 22, 2020
@kubkon
Copy link
Member

kubkon commented Jan 22, 2020

LGTM, just a couple of questions. Also, just an FYI so that we don't duplicate the effort that @pchickey and myself are currently working on an upgraded version of wig, the wiggle crate which should help us use the polyfill functionality of the witx crate to generate wasi_unstable and any future snapshots in a way that almost completely removes any unnecessary duplication in hostcalls implementation, etc. It should also help us auto-generate the memory.rs module.

Oh, and the link to wiggle.

This commit is effectively just doing what bytecodealliance#707 already did, but
applying it to the `snapshot_0` module as well. The end result is the
same, where we cut down on all the boilerplate in `snapshot_0` and bring
it in line with the main `wasi_snapshot_preview1` implementation. The
goal here is to make it easier to change the two in tandem since they're
both doing the same thing.
This commit migrates the `hostcalls` module to being auto-generated by a
macro rather than duplicating a handwritten signature for each wasi
syscall.
Similar to the previous commit, but for `snapshot_0`
This is no longer needed with the hostcalls macro now, we can easily
fold the definition of the cbindgen macro into the same crate.
@alexcrichton alexcrichton force-pushed the more-auto-generate-witx branch from 3477bf8 to 2a90a98 Compare January 22, 2020 20:17
@alexcrichton
Copy link
Member Author

Also, just an FYI so that we don't duplicate the effort that @pchickey and myself are currently working on

Nice! I'm happy to go whichever way here, was just eager to help cut down on more boilerplate, and I also needed this for a refactoring of the wasmtime-wasi crate to use the wasmtime crate

@alexcrichton alexcrichton merged commit 5953215 into bytecodealliance:master Jan 22, 2020
@alexcrichton alexcrichton deleted the more-auto-generate-witx branch January 22, 2020 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasi:impl Issues pertaining to WASI implementation in Wasmtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants