Stream: git-wasmtime

Topic: wasmtime / issue #5934 implement "part 1" of WIT Templates

Wasmtime GitHub notifications bot (Mar 05 2023 at 22:44):

github-actions[bot] commented on issue #5934:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Mar 06 2023 at 17:06):

dicej commented on issue #5934:

Yes, in that particular test case, the only reason I used bindgen! was to ensure that it doesn't choke on wildcards. In a more realistic scenario, a world could contain a mix of non-wildcard interfaces and wildcard interfaces. You'd still want generated bindings for the former even if you don't need them for the latter.

Also, although https://github.com/WebAssembly/component-model/issues/172 does not include "mixed" interfaces which contain both wildcard and non-wildcard functions, it would be pretty trivial to add support for that. For example:

default world wildcards {
    import imports: interface {
        *: func() -> u32
        foo: func(s: string) -> string
        bar: func() -> u32
    }
}

BTW, my initial implementation (https://github.com/bytecodealliance/wasmtime/pull/5925) _did_ generate code for wildcards, but that code ended up just being wrappers around the Wasmtime API which didn't add much value vs. using the API directly. If you think there _is_ value to be added via code generation, we can revisit that.

Wasmtime GitHub notifications bot (Mar 06 2023 at 18:34):

alexcrichton commented on issue #5934:

In some sense the whole bindgen! macro is a pretty thin wrapper around the component embedding API, but I think the main value from it is that it gets all the types right and you don't have to think about it. From another perspective any type-level bugs are caught by rustc and aren't logic bugs in your own code. Manually calling linker functions, though, you can get past rustc but accidentally deviate from the *.wit which I think is nice to fix if we can. More-or-less I do think that it's wortwhile to generate typed bindings to *-imports/exports to make it idomatic with the rest of *.wit-generated bindings and otherwise make it so you don't have to think about the types.

Wasmtime GitHub notifications bot (Mar 06 2023 at 18:39):

dicej commented on issue #5934:

Fair enough. Would you mind taking a look at the diff for https://github.com/bytecodealliance/wasmtime/pull/5925 and provide feedback on that, then? I can close this PR and reopen that one if desired. I just want to make sure we're aligned about what the generated code should look like first.

Wasmtime GitHub notifications bot (Mar 06 2023 at 19:05):

alexcrichton commented on issue #5934:

For sure yeah, the main thing I'd consider on #5925 is sort of along the same lines of catching issues earlier rather than later. There it looks like the strategy is that *-imports bottom out into a fn call(&mut self, name: &str, ...) trait method but an alternative would be for the host to pass in some sort of configuration "I support call with these names" which then fails instantiation if enough names weren't supplied. My original idea here was to pass in a vector of named trait objects, but that may be too inefficient for the intended use cases (I'm not sure).

I don't feel super strongly about this particular point though myself. It feels a little odd to pass around the component when you're adding something to a linker but that's probably just me needing to get over my preconceptions about this. In any case I do think that #5925 is better in the sense of "everything is type-checked for you" so I think that would be a better starting point than not generating bindings for *

Wasmtime GitHub notifications bot (Mar 07 2023 at 01:51):

dicej commented on issue #5934:

@alexcrichton I've implemented code generation per your feedback. Please let me know what you think.

Wasmtime GitHub notifications bot (Mar 07 2023 at 23:54):

dicej commented on issue #5934:

@alexcrichton Thanks for all your feedback. I think I've addressed everything so far.

Last updated: Nov 22 2024 at 16:03 UTC