rust-lang / rust-lang/rust-bindgen
feature request: Enable deriving `Clone`, `Copy`, etc... for `__IncompleteArrayField` and containing structs.
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.
- A method on the
builderwhich 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. - Pass
__IncompleteAraryFieldto theadd_derivescallback, and pass the already present derives to theadd_derivescallback so#[derive(Clone, Clone)]can be avoided with a_info.derives.contains("Clone)check. This would also enablebindgendetecting that__IncompleteArrayFieldimplementsClonewhile building since it would be added during the build.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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