bytecodealliance / wasm-tools

CLI and Rust libraries for low-level manipulation of WebAssembly modules
Apache License 2.0
1.34k stars 245 forks source link

proposal: wit-parser: create distinct imported and exported TypeDefs, Functions, and Interfaces in a Resolve #1497

Open ydnar opened 6 months ago

ydnar commented 6 months ago

We propose to move logic from wit-bindgen into the wit-parser crate that determines whether an interface, type, or function in a Resolve structure is either imported or exported, and label those structures as such.

  1. Add an attribute to the Interface, Function, and TypeDef structs to indicate whether it is imported or exported.
  2. The tree of imports and exports in a World would be duplicated (if necessary) and have the relevant import/export attribute.
  3. Modify the JSON serialization of Resolve (e.g. wasm-tools component wit -j ...) to include the import/export attribute.

Context

This is a followup from a conversation in Zulip regarding imported and exported types in a Resolve.

Currently, any types (represented as a TypeDef), functions (Function), and interfaces (Interface) are represented once in the Resolve data structure (and its JSON representation). This means that each bindings generator that depends on the wit-parser crate must reimplement the heuristic to determine when to use an imported type versus an exported type.

Per @alexcrichton:

For given interface utils, and a type fd defined in a different interface types:

What ends up happening here is a bit subtle and it's generally related to WIT conventions. The CM itself has no ambiguity, the problem arises when WIT is mapped back to the component model. To answer your question the cases are:

  • If utils is imported, then it uses the imported fd
  • If utils is exported, and fd is not exported, then it uses an imported fd
  • If utils is exported, and fd is exported, then it will use the exported fd

there's also implicit insertion of interfaces to handle here too, for example wasm-tools component wit ./my-witwill show the "elaborated" version of a world. For example if you do export utils; that'll implicitly insert import fd;. If you have export fd; export utils;, however, then no implicit insertion happens and utils uses the exported fd.

This would simplify the implementation of bindings generators, such as wit-bindgen-go, which could then depend on the import/export resolution in wit-parser.

Mossaka commented 6 months ago

I will take a look at this issue if no one has started looking into it

alexcrichton commented 6 months ago

Thanks @Mossaka!

This is going to touch a lot of places in wasm-tools in some subtle ways, but in the end everything should be pretty straightforward except for the Resolve bits of "actually copy and duplicate everything". One example of being a bit subtle is encoding/decoding which actually have a fair bit of logic to work around the opposite of this issue, specifically generating a single interface instead of two by accident. This might help simplify big chunks of that.

As usual I'm happy to help out with any questions and such, just ping me!

ydnar commented 6 months ago

Exported types referencing imported types should be interesting!

ydnar commented 6 months ago

Would it make sense for each Interface, TypeDef, and Function to have a reference to its symmetric other (import vs export) if it exists?

Mossaka commented 6 months ago

This seems relevant to the discussion here:

https://github.com/bytecodealliance/wit-bindgen/blob/8390dc5621726b4287eea15c3a2b32a49f15e53f/crates/c/src/lib.rs#L582-L600

alexcrichton commented 6 months ago

@ydnar seems reasonable to me yeah, but I think it largely depends on what bindings generators want. If they don't need it it's probably not worth it, but if any needs it then seems good to add.

@Mossaka yeah that should get cleaned up/removed with this issue, in theory.

cpetig commented 6 months ago

I feel this change could potentially also facilitate the solution we postponed as too complicated in https://github.com/bytecodealliance/wasm-tools/pull/1438#issuecomment-1984559922 - here only borrowed and exported resource handles should be considered as a pointer, all else as an s32.

alexcrichton commented 6 months ago

Indeed yeah this change should make that trivial

ydnar commented 6 months ago

@Mossaka you've probably realized or encountered this already: but I think this proposal means that every WorldItem would be duplicated at least once for each World in a Resolve.