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

Rust import: Traits on generated bindings #291

Closed
philpax opened this issue Jul 25, 2022 · 12 comments
Closed

Rust import: Traits on generated bindings #291

philpax opened this issue Jul 25, 2022 · 12 comments
Labels
syntax Related to syntax of `*.wit` files themselves

Comments

@philpax
Copy link
Contributor

philpax commented Jul 25, 2022

Hello again!

I have two types:

record entity-id {
    namespace: u8,
    id: u64,
    gen: s32,
}

enum event-type {
    game-start,
    frame,
    player-spawn,
}

For the former, I'd like to derive/implement PartialEq (so that I can compare IDs), and for the latter, I'd like to derive/implement Hash (so that I can use it as a hashmap key). Unfortunately, the current binding generation model doesn't really allow for this as you can't implement traits on foreign types in Rust.

I was wondering if there was a solution planned for this/the general problem of implementing additional behaviours on top of the binding types. Is my best bet just to newtype away? If so, how should I handle the bound functions that consume these types like entity-freeze: func(object: entity-id) - wrap them in functions that convert to/from the newtype?

@esoterra
Copy link
Contributor

I've thought some about it. My preferred solution is to switch from using a proc macro in Rust to using an attribute macro, and then adding annotations that let you attach extra derives to things.

e.g.

#[wit-bindgen]
#[export("game.wit")]
#[add_derive("entity-id", PartialEq)]
struct Game;

@philpax
Copy link
Contributor Author

philpax commented Jul 26, 2022

That sounds like a promising start, but I see two potential problems / things to account for:

  1. User-defined traits would be nice to have - for example, a custom Display (the host code uses a colon-separated representation for EntityId), or operator overloading for Vec3 (f32 x, y, z). With that being said, for Vec3 specifically, I'm thinking I'll hide the interface types and just add some casting to/from glam's Vec3.
  2. Using attributes might get unwieldy if you have a lot of additions you need to make to the generated code, like having to add derives to every type you define.

My suggestion to address both issues would be an "augmentations" code block that could be passed as the last argument of the proc macro:

wit_bindgen_rust::import!("src/main.wit", {
  #[derive(Hash, PartialEq)]
  struct EntityId;

  impl Display for EntityId { /* ... */ }
});

The only bit of magic would be associating the derives with the relevant structs.

That being said, I'm leaning more towards using the generated types purely at the interface layer, and building my own interface atop that, even though it's a bit of extra work. Is there any prior art / thoughts on how bindgen should be used 'canonically', or are things still in flux?

@philpax
Copy link
Contributor Author

philpax commented Aug 12, 2022

Another thing I've just realised would be nice to have is adding doc comments to types, but I'm not sure if that would be best handled as an "augmentation" or within the .wit file itself.

@Liamolucko
Copy link
Contributor

Another thing I've just realised would be nice to have is adding doc comments to types, but I'm not sure if that would be best handled as an "augmentation" or within the .wit file itself.

wit-bindgen already supports doc comments in .wit files, using triple-slash comments. They should probably be added to WIT.md.

@philpax
Copy link
Contributor Author

philpax commented Aug 13, 2022

Ah, my apologies, you're absolutely right! Should've tried that first 🤦

@esoterra
Copy link
Contributor

@philpax the solution you proposed doesn't mesh very well with how wit-bindgen is currently structured. We're not really taking in your Rust code and extending it, we're parsing a *.wit file and then dumping the results where you called the macro. These features will be much easier (and thus more likely) to implement if they are framed as things that configure the existing code generator instead of things that require the code generators to be rewritten.

@philpax
Copy link
Contributor Author

philpax commented Aug 15, 2022

Yes, I know - I'm not suggesting extending existing Rust code with the bindings from the wit, I'm suggesting extending the output of the generator with language-specific augmentations. That is, this example

wit_bindgen_rust::import!("src/main.wit", {
  #[derive(Hash, PartialEq)]
  struct EntityId;
});

tells the code generator to augment its output for EntityId with #[derive(Hash, PartialEq)], and anything that doesn't directly change how a language item is defined would be dumped into the output.

I think that would be easier to understand/manipulate and more flexible for the user than an attribute-based solution, but I also recognise that there would be difficulties associated with this in the implementation (you would basically need to be able to parse a subset of Rust).

An alternative would be to have the bindgen macros not create modules (so that users could augment the content of the generated module directly), but you'd still need a solution for the derive. If this was the case, I'd be fine with the attribute solution, as it should hopefully not occur too often.

@alexcrichton
Copy link
Member

If you're using the proc-macro support for wit_bindgen_rust then the type generated isn't a foreign type, it's still crate-local, so I think that PartialEq implementations (and other custom trait implementations) can still be added. I don't think that they can be added via #[derive] since you don't have access to the actual struct definition, but at least to me that seems like a reasonable tradeoff in complexity for generated bindings.

Support for something like this I think is probably best done through the wit-syntax as @Liamolucko mentions where there's something like a language-specific attribute where the Rust generator would recognize "oh add derive(PartialEq)" or something like that.

@alexcrichton alexcrichton added the syntax Related to syntax of `*.wit` files themselves label Sep 6, 2022
@Gearme
Copy link
Contributor

Gearme commented Dec 1, 2022

For a slightly wider context of the issue at hand: @theduke may be working on a proposal to address this (component-model issue #58). Seems to me that with this being a syntax issue, it might be better discussed further in the component-model repo.

@t3hmrman
Copy link

Note that support for custom derives has landed thanks to @thomastaylor312 : #678

@philpax
Copy link
Contributor Author

philpax commented Sep 27, 2023

Awesome! Thanks to Thomas for implementing this :D

@MendyBerger
Copy link

Default doesn't work though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
syntax Related to syntax of `*.wit` files themselves
Projects
None yet
Development

No branches or pull requests

8 participants