[feature request] API for basic manipulation of generated items

[crumblingstatue]

I am the current maintainer of rust-sfml.

Currently, we are using pre-generated bindings to CSFML as a bridge between Rust and the C++ API. We suffer performance penalties with this, as the CSFML API hides the C++ types behind a heap allocation, so this way we'll never be as performant as the original C++ API.

I am considering using bindgen to generate bindings directly against the C++ API, but using the generated bindings as-is would make my API less ergonomic / idiomatic.

I'll give a few examples:

Example 1

Here are the generated bindings for sf::ContextSettings:

#[repr(C)]
#[derive(Debug, Copy)]
pub struct sf_ContextSettings {
    pub depthBits: ::std::os::raw::c_uint,
    pub stencilBits: ::std::os::raw::c_uint,
    pub antialiasingLevel: ::std::os::raw::c_uint,
    pub majorVersion: ::std::os::raw::c_uint,
    pub minorVersion: ::std::os::raw::c_uint,
    pub attributeFlags: sf_Uint32,
    pub sRgbCapable: bool,
}

1. If possible, I would like to derive more traits for it, instead of having to manually add impl blocks.

2. I want to rename this to just ContextSettings, the sf_ prefix is just needless noise, due to the module support in Rust. Re-exports make this possible, but it wouldn't hurt to have a direct ability to rename types.

3. I want the fields to follow the idiomatic Rust naming convention.

4. I want to add documentation for both the type, and its fields.

I don't consider wrapping this in a wrapper type a solution, as that would make the API for the end-user less ergonomic (getter/setter methods vs. direct field access).

What I currently do is manually define this type in Rust, and transmute between this and the expected CSFML type. Obviously, automated bindings are much more robust than fragile hand-maintained bindings.

#[repr(C)]
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Copy)]
pub struct ContextSettings {
    /// Bits of the depth buffer.
    pub depth_bits: c_uint,
    /// Bits of the stencil buffer.
    pub stencil_bits: c_uint,
    /// Level of antialiasing.
    pub antialiasing_level: c_uint,
    /// Major number of the context version to create.
    pub major_version: c_uint,
    /// Minor number of the context version to create.
    pub minor_version: c_uint,
    /// The attribute flags to create the context with.
    pub attribute_flags: u32,
    /// Whether the context framebuffer is sRGB capable.
    pub srgb_capable: Bool,
}

Example 2

Here are the bindings for sf::Vector2:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct sf_Vector2<T> {
    pub x: T,
    pub y: T,
    pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>>,
}

Here, bindgen adds a _phantom_0 field, which I don't believe is necessary for my type to function correctly, but it does make the API less ergonomic, as the user now has to take that type into account when doing initialization, pattern mathing, etc.

I would like the ability to remove this field.

Summary

I would like the ability to

1: Add (possibly remove?) derives for generated types
2: (optional) Rename top-level generated items
3: Rename fields/variants/etc. of generated types
4: Add documentation to generated items
5: Remove (possibly unnecessary) fields of generated structs

I was considering using https://docs.rs/bindgen/0.30.0/bindgen/struct.Bindings.html?search=#method.into_ast , and somehow writing that out, but from what I was told on IRC, this functionality was removed on the master branch.

One alternative would be reparsing the written-out bindings using libsyntax or syntex-syntax or whatever, and manipulating that, and writing it out once again, but this seems kind of wasteful. If nothing else, it would be nice to get raw access to the AST so we can do the manipulation with whatever library bindgen uses, and write the result ourselves using whatever method that library supports.

[fitzgen]

Thanks for the bug report!

If possible, I would like to derive more traits for it, instead of having to manually add impl blocks.

Did you use --with-derive-blah or builder equivalents? We should support deriving all derive-able traits (on master, hoping to have a new release soon) but many are just off by default: #886

I want to rename this to just ContextSettings, the sf_ prefix is just needless noise, due to the module support in Rust. Re-exports make this possible, but it wouldn't hurt to have a direct ability to rename types.

Did you use --enable-cxx-namespaces or --disable-name-namespacing or their builder equivalents? They should achieve your goals.

Here, bindgen adds a _phantom_0 field, which I don't believe is necessary for my type to function correctly

It is there to force it so that the type is invariant with regards to lifetime subtyping if instantiated with references or anything else that has a lifetime. This is conservative, since we can't really know what guarantees the C++ thing gives.

I want the fields to follow the idiomatic Rust naming convention.

Filed #1098 to track this.

I want to add documentation for both the type, and its fields.

#1089 should allow arbitrary custom attributes, and doc comments are just sugar for #[doc = "..."].

I was considering using https://docs.rs/bindgen/0.30.0/bindgen/struct.Bindings.html?search=#method.into_ast , and somehow writing that out, but from what I was told on IRC, this functionality was removed on the master branch.

One alternative would be reparsing the written-out bindings using libsyntax or syntex-syntax or whatever, and manipulating that, and writing it out once again, but this seems kind of wasteful. If nothing else, it would be nice to get raw access to the AST so we can do the manipulation with whatever library bindgen uses, and write the result ourselves using whatever method that library supports.

We just use quote! { ... } now (and our build times cut in half!) so we don't have any libsyntax or syntex AST to give out. Our internal IR is pretty much permanently unstable...

I don't consider wrapping this in a wrapper type a solution,

In general, bindgen can't create Rust-y APIs for you, and if that's what you want, you're probably not gonna have a great time.

On the other hand, it is pretty good at creating -sys crates that you can then create wrappers around with a Rust-y API on top of.

I know it probably isn't what you wanted to hear, but that's just the reality of where things are :-/

[crumblingstatue]

Did you use --with-derive-blah or builder equivalents? We should support deriving all derive-able traits (on master, hoping to have a new release soon) but many are just off by default: #886

I'll look into it and see how it works out.

Did you use --enable-cxx-namespaces or --disable-name-namespacing or their builder equivalents? They should achieve your goals.

CXX namespaces would just add an extra level of nesting, wouldn't it? (You would have to re-export the items to remove the nesting, at which point you can also rename them) Why isn't disable-name-namespacing default? Probably because there could be name clashes. If so, then it's not a very robust solution.

But as I've stated, re-exports kind of solve this problem, so it's entirely optional, but would be nice to have if an ability to manipulate the generated items was added.

It is there to force it so that the type is invariant with regards to lifetime subtyping if instantiated with references or anything else that has a lifetime. This is conservative, since we can't really know what guarantees the C++ thing gives.

Right, which is why an ability to override this behavior for a specific item would be nice. The developer knows more than bindgen does, so it makes sense to allow the developer to have a say in this matter.

Filed #1098 to track this.

I was more thinking of an ability for the user to rename fields, not just an automatic conversion, which could possibly generate undesirable results.

#1089 should allow arbitrary custom attributes, and doc comments are just sugar for #[doc = "..."].

Would this also allow attributes on individual fields/variants as well? (for the purposes of documenting those)

We just use quote! { ... } now (and our build times cut in half!) so we don't have any libsyntax or syntex AST to give out. Our internal IR is pretty much permanently unstable...

Alright, so bindgen can't expose the AST. But there must be a time where bindgen takes a look at the C++ input and converts that to Rust code. It's there where it decides how to name the items/fields, add derive attributes, etc. Perhaps there could be an API that hooks into this, and allows the user to customize specific items?

In general, bindgen can't create Rust-y APIs for you, and if that's what you want, you're probably not gonna have a great time.

Right, this is why it would be nice to allow the user to customize the output, so they can make it Rusty.

On the other hand, it is pretty good at creating -sys crates that you can then create wrappers around with a Rust-y API on top of.

This is fine for "opaque" types, and types with private internals that the user shouldn't have access to.

It's not so nice for POD structs, enums, etc. that the user should be able to directly access without the inconvenience of a wrapper type with methods, etc. Putting such items in wrappers makes the API inferior to the original C++ API.

As I've mentioned in the original post, currently I am just hand-writing these types in Rust, and transmuting them to their C++ equivalents. This is error-prone compared to bindgen making sure I've covered all the fields/variants, and they have the correct type. But it is the best alternative, because it provides the best API for the end-user, and I'll continue doing this if there is no better alternative.

[emilio]

Alright, so bindgen can't expose the AST. But there must be a time where bindgen takes a look at the C++ input and converts that to Rust code. It's there where it decides how to name the items/fields, add derive attributes, etc. Perhaps there could be an API that hooks into this, and allows the user to customize specific items?

We have ParseCallbacks in order to customize enums. Extending that to renaming fields, doesn't sound terribly hard, I think.

[pvdrz]

@crumblingstatue is there anything I could help you with?

[crumblingstatue]

Thank you for the question.
Since the initial opening of this issue, rust-sfml has stopped using rust-bindgen, so I no longer feel invested in this issue.
I'm going to close it, and if someone has similar needs, maybe a new fresh issue might make more sense.