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

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

Open
ydnar opened this issue Apr 15, 2024 · 9 comments

Comments

@ydnar
Copy link
Contributor

ydnar commented Apr 15, 2024

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
Copy link
Member

Mossaka commented Apr 19, 2024

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

@alexcrichton
Copy link
Member

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
Copy link
Contributor Author

ydnar commented Apr 20, 2024

Exported types referencing imported types should be interesting!

@ydnar
Copy link
Contributor Author

ydnar commented Apr 20, 2024

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
Copy link
Member

Mossaka commented Apr 21, 2024

This seems relevant to the discussion here:

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

@alexcrichton
Copy link
Member

@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
Copy link
Contributor

cpetig commented Apr 22, 2024

I feel this change could potentially also facilitate the solution we postponed as too complicated in #1438 (comment) - here only borrowed and exported resource handles should be considered as a pointer, all else as an s32.

@alexcrichton
Copy link
Member

Indeed yeah this change should make that trivial

@ydnar
Copy link
Contributor Author

ydnar commented Apr 25, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants