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

Bindgen! gives weird name to an interface well-named in WIT file #9774

Open
ifsheldon opened this issue Dec 10, 2024 · 6 comments
Open

Bindgen! gives weird name to an interface well-named in WIT file #9774

ifsheldon opened this issue Dec 10, 2024 · 6 comments

Comments

@ifsheldon
Copy link
Contributor

I don't know whether this is a bug or expected. I personally would consider it as a minor bug, but I leave it as a feature request.

Feature

I'd like bindgen! generates a sensible name for the bindings of interfaces in a WIT file.

I have this WIT file. It exports an interface called add that contains only one function add.

package component: interfaced-adder;

// See https://github.com/bytecodealliance/cargo-component/issues/360 for why this is needed
// Also according to https://component-model.bytecodealliance.org/creating-and-consuming/composing.html, "Composition happens at the level of interfaces"

interface add {
  add: func(a: s32, b: s32) -> s32;
}

world adder {
  export add;
}

In my Rust host, I need to run an adder component, so I write the host code like

bindgen!({
    path: "../wit-files/interfaced-adder.wit",
    world: "adder",
});

pub fn run_adder_sync(engine: &Engine) {
    let (component, linker, mut store) = get_component_linker_store(
        engine,
        "./target/wasm32-wasip2/release/guest_interfaced_adder_rs.wasm",
        "../target/wasm32-wasip2/release/guest_interfaced_adder_rs.wasm",
    );
    let bindings = Adder::instantiate(&mut store, &component, &linker).unwrap();
    let a = 1;
    let b = 2;
    // TODO: interface0 seems weird, file an issue
    let result = bindings.interface0.call_add(&mut store, a, b).unwrap();
    assert_eq!(result, 3);
}

Instead of a meaningful name like "add", bindgen! somehow generate the binding for interface add with the name "interface0". This seems weird to me. And I don't see why it can't be named as the same as the interface name. bindings.add.call_add seems more natural to me.

Benefit

The current naming is bad for readability and understanding, as no one can see from interface0 that it has any relation to the interface add. interface0 means just the first interface.

Changing the naming can improve readability, understanding and code writing.

Implementation

No, I am not very familiar with bindgen internals.

@alexcrichton
Copy link
Member

I believe interface0 here is an internal name in the struct, and the intended interface should be bindings.component_interfaced_adder_add().call_add() where the first component_interfaced_adder_add() gives you a structure representing the interface add. Does that work here?

In this example it would be this method

@ifsheldon
Copy link
Contributor Author

That works. Thanks! But I still think we should give a better name to this field, even if it's internal.

Besides, the name scheme bindings.<package name>_<world name>_<interface name>() seems a bit too verbose to me, since in the bindgen! we have already specified the WIT file, so we can assume the bindgen users have already known the package and world of their choice. I think simple bindings.<interface name>() would be better.

Also, this method bindings.<package name>_<world name>_<interface name>() has no autogenerated documentation in code, I think a simple oneliner would be great, like Bindings for <package name>/add.

@alexcrichton
Copy link
Member

Auto-docs sounds great yeah, but we can't easily change it to just <interface name> because this world is valid and would then have invalid bindings:

world some-world {
    export foo;
    export foo: interface { ... }
}

interface foo { ... }

While I agree it's wordy, it's needed to disambiguate.

@ifsheldon
Copy link
Contributor Author

because this world is valid and would then have invalid bindings

What!? I don't know that. So WIT specs allow exporting two interfaces that have the same name in the same world? This seems crazy to me. I guess in this case, we would just use foo1 and foo2 since there's no other (sane) ways to programmatically distinguish these two, which I would consider as the fault of this WIT author.

@alexcrichton
Copy link
Member

Well, to clarify, these are two different syntactic forms of exports. One is a "full interface" which is technically named ns:pkg/foo where the other is a "kebab export" where the name is just foo. These are not distinguished as foo1 and foo2 but as ns_pkg_foo() and foo() today. I was trying to explain the origins of why the name is slightly wordy right now.

@ifsheldon
Copy link
Contributor Author

OK, seems I just need to dig into WIT specs........ but first let's get auto-docs working

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

2 participants