rust-lang / rust-lang/rust-bindgen

feature request: Enable deriving `Clone`, `Copy`, etc... for `__IncompleteArrayField` and containing structs.

Open
#2,832 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
5.3k
Forks
829
Avg merge
1d 1h
Merged PRs (30d)
15

Description

Preface:
Bindgen is a very useful tool. Thank you for releasing it!

Background

As indicated in #1431 and #2791 __IncompleteArrayField purposefully doesn't implement Clone or Copy and structs containing it don't either. #1093 describes Eq and Hash.

The reasoning seems to be that a struct H which contains the __IncompleteArrayField implicitly refers to a larger collection and copying the struct would break this. For the other traits the argument is that the default doesn't make sense for the struct.

As indicated in #1892 the stable usage in the book violates stacked borrows.

The book also mentions that on nightly the fam containing structs can be emulated with a DST which duplicates the size (once in the original c definition and once in the fat pointer itself) which is an unnecessary inefficiency.

The fambox crate I wrote allows using these c structs on stable without nightly DST features in a way that doesn't violate stacked borrows (and therefore passes miri). It is implemented as a thin pointer without duplicating the struct's contained length.
The main idea is that a type containing a flexible array member implements https://docs.rs/fambox/0.1.1/fambox/trait.FamHeader.html and is treated as a sized type with the necessary metadata to describe the buffer stored immediately after. This seems better than both advocated solutions in the book.

Under this conceptualization the struct H doesn't have an unknown size that must be carefully managed, and it is makes perfect sense to Clone, Copy, cmp, hash, etc... the Sized header. In fact, FamBox<H> can only implement Clone if H implements Clone (and same for the other traits).

It's not a big deal that __IncompleteArrayField doesn't implement these because one can write

    include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
    impl<T: std::marker::Copy> Copy for __IncompleteArrayField<T> {}
//    etc..

though it is annoying.

The larger problem is that H won't derive Copy even if __IncompleteArrayField does (since that won't be known at bindgen's build time) and so you wind up with something like this

struct DeriveImpls;
impl DeriveImpls {
    const COPY_CLONE_BLACKLIST: &'static [&'static str] = &["struct_that_already_implements_Clone", "other_struct_that_already_implements_Copy", ...];
    const PARTIAL_EQ_BLACKLIST: &'static [&'static str] = &["struct_already_implements_PartialEq", ...]
    // etc...
}
impl ParseCallbacks for DeriveImpls {
    fn add_derives(&self, _info: &DeriveInfo<'_>) -> Vec<String> {
        let mut out = vec![];
        out.extend_from_slice(&["bytemuck::Pod".to_owned(), "bytemuck::Zeroable".to_owned()]);
        if !Self::COPY_CLONE_BLACKLIST.contains(&_info.name) {
            out.extend_from_slice(&["Copy".to_owned()]);
            out.extend_from_slice(&["Clone".to_owned()]);
        }
        if !Self::PARTIAL_EQ_BLACKLIST.contains(&_info.name) {
            out.extend_from_slice(&["PartialEq".to_owned()]);
        }
        // etc...
        out
    }
}

which is very annoying and tedious.

Proposal

There seems like two reasonable options to enable this feature. Both seem reasonable but the second would also help with Serialize and other custom derives so would probably be more important.

  1. A method on the builder which opts into derives on __IncompleteArrayField. By enabling this the user acknowledges that they won't be using the translated structs as DST but rather as headers.
  2. Pass __IncompleteAraryField to the add_derives callback, and pass the already present derives to the add_derives callback so #[derive(Clone, Clone)] can be avoided with a _info.derives.contains("Clone) check. This would also enable bindgen detecting that __IncompleteArrayField implements Clone while building since it would be added during the build.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the builder and the ParseCallbacks::add_derives entry point described in the proposal, along with how __IncompleteArrayField derives are generated. Compare the two proposed approaches and determine which callback or builder behavior should be specified. Done means a selected design supports the requested derives without duplicate derive names and has corresponding coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, rust
Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.