wit-bindgen icon indicating copy to clipboard operation
wit-bindgen copied to clipboard

Types appearing both in module and in exports

Open arlyon opened this issue 11 months ago • 7 comments

Consider this wit

package litehouse:plugin;

interface plugin {
    variant update {
      time(u64),

      temperature(float64),
      wind-speed(float64),

      current(float64),
      voltage(u16),
      power(u16),
      on-off(bool),
    }
}

world plugin-host {
  use plugin.{update};
  import send-update: func(nickname: string, update: update);

  export plugin;
}

I want to simultaneously expose the interface to the host as well as require the host to expose a function for the guest to use. The generated code however produces two types, bindings::litehouse::plugin::plugin::Update and bindings::exports::litehouse::plugin::plugin::Update, one presumably stems from the use and one from the export. These generated types are (to my eyes) identical. Any Update coming from the guest has the first, and any Update going into the guest has the latter. I would assume we would be able to, in the case of an interface being exported, defer all references of it to its exported types. Is there a technical reason for not doing this? If it is some technical issue, then a From / Into each way would be handy.

I will note it looks like all types from the interface are generated in both bindings and bindings::exports despite only importing one of them. I would also imagine those would be pruned to just the ones that are explicitly used plus types they reference.

arlyon avatar Mar 20 '24 23:03 arlyon

Thanks for the report! This is currently intentional, but as you've pointed out, not necessary in this case either. The intentional part is that when the interface is both imported and exported that two separate modules are generated and each has copies of all the types of the interface. This is required for resources, for example, because it's assumed that the exported resource will have a different identity than the imported resource.

Without resources though, yes, it's possible to share types. Even types that don't transitively contain resources can be deduplicated. This deduplication has not yet been implemented in any bindings generator currently.

alexcrichton avatar Mar 21 '24 17:03 alexcrichton

I’ve implemented type deduplication in wit-bindgen-go for mutually imported/exported types that do not include any resource handles.

I ended up running into a related issue, where the resource-drop for a resource type that’s both imported and exported will have the same mangled name:

// resource drop for imported type
__attribute__((__import_module__("example:uses/a"), __import_name__("[resource-drop]res")))
extern void __wasm_import_example_uses_a_res_drop(int32_t handle);

void example_uses_a_res_drop_own(example_uses_a_own_res_t handle) {
  __wasm_import_example_uses_a_res_drop(handle.__handle);
}

// resource drop for exported type
__attribute__((__import_module__("example:uses/a"), __import_name__("[resource-drop]res")))
extern void __wasm_import_exports_example_uses_a_res_drop(int32_t handle);

void exports_example_uses_a_res_drop_own(exports_example_uses_a_own_res_t handle) {
  __wasm_import_exports_example_uses_a_res_drop(handle.__handle);
}

ydnar avatar May 08 '24 17:05 ydnar

Nice! I think that the dtor issue may have been fixed in https://github.com/bytecodealliance/wit-bindgen/pull/946, but mind confirming?

alexcrichton avatar May 08 '24 17:05 alexcrichton

I'll try this out, thanks!

Does this also apply to resource.new/resource.rep?

ydnar avatar May 09 '24 00:05 ydnar

I don't think so, but I'd have to dig in to be sure. I say this though because resource new/rep only work on exported resources, so I don't think the [export] is needed to disambiguate there (as it's always exports)

alexcrichton avatar May 09 '24 15:05 alexcrichton

Is there value in consistency for imports on exported types?

ydnar avatar May 11 '24 16:05 ydnar

I don't have much of a preference myself, but it seems reasonable to at least accept both perhaps

alexcrichton avatar May 13 '24 15:05 alexcrichton