wasm-tools icon indicating copy to clipboard operation
wasm-tools copied to clipboard

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

Open ydnar opened this issue 1 year ago • 9 comments

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.

ydnar avatar Apr 15 '24 23:04 ydnar

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

Mossaka avatar Apr 19 '24 19:04 Mossaka

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!

alexcrichton avatar Apr 19 '24 20:04 alexcrichton

Exported types referencing imported types should be interesting!

ydnar avatar Apr 20 '24 17:04 ydnar

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

ydnar avatar Apr 20 '24 21:04 ydnar

This seems relevant to the discussion here:

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

Mossaka avatar Apr 21 '24 23:04 Mossaka

@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.

alexcrichton avatar Apr 22 '24 14:04 alexcrichton

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.

cpetig avatar Apr 22 '24 22:04 cpetig

Indeed yeah this change should make that trivial

alexcrichton avatar Apr 23 '24 14:04 alexcrichton

@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.

ydnar avatar Apr 25 '24 14:04 ydnar