Skip to content

Commit a2aaf47

Browse files
committed
codegen: Deduplicate derive traits added by add_derives() parse callback
Fixes #3286
1 parent 50ec3ee commit a2aaf47

File tree

4 files changed

+142
-4
lines changed

4 files changed

+142
-4
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
struct SimpleStruct {
2+
int a;
3+
};
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
#[cfg(test)]
2+
mod tests {
3+
use bindgen::callbacks::{DeriveInfo, ParseCallbacks};
4+
use bindgen::{Bindings, Builder};
5+
use std::path::PathBuf;
6+
7+
#[derive(Debug)]
8+
struct AddDerivesCallback(Vec<String>);
9+
10+
impl ParseCallbacks for AddDerivesCallback {
11+
fn add_derives(&self, _info: &DeriveInfo<'_>) -> Vec<String> {
12+
self.0.clone()
13+
}
14+
}
15+
16+
struct WriteAdapter<'a>(&'a mut Vec<u8>);
17+
18+
impl std::io::Write for WriteAdapter<'_> {
19+
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
20+
self.0.extend_from_slice(buf);
21+
Ok(buf.len())
22+
}
23+
fn flush(&mut self) -> std::io::Result<()> {
24+
Ok(())
25+
}
26+
}
27+
28+
fn write_bindings_to_string(bindings: &Bindings) -> String {
29+
let mut output = Vec::<u8>::new();
30+
bindings
31+
.write(Box::new(WriteAdapter(&mut output)))
32+
.expect("Failed to write generated bindings");
33+
String::from_utf8(output)
34+
.expect("Failed to convert generated bindings to string")
35+
}
36+
37+
/// Tests that adding a derive trait that's already derived automatically
38+
/// does not result in a duplicate derive trait (which would not compile).
39+
#[test]
40+
fn test_add_derives_callback_dedupe() {
41+
let crate_dir =
42+
PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap());
43+
let header_path = crate_dir.join(
44+
"tests/parse_callbacks/add_derives_callback/header_add_derives.h",
45+
);
46+
47+
let builder = Builder::default()
48+
.header(header_path.display().to_string())
49+
.derive_debug(true)
50+
.derive_copy(false)
51+
.derive_default(false)
52+
.derive_partialeq(false)
53+
.derive_eq(false)
54+
.derive_partialord(false)
55+
.derive_ord(false)
56+
.derive_hash(false)
57+
.parse_callbacks(Box::new(AddDerivesCallback(vec![
58+
"Debug".to_string()
59+
])));
60+
let bindings = builder.generate().expect("Failed to generate bindings");
61+
let output = write_bindings_to_string(&bindings);
62+
assert!(
63+
output.contains("#[derive(Debug)]") &&
64+
!output.contains("#[derive(Debug, Debug)]"),
65+
"Unexpected bindgen output:\n{}",
66+
output.as_str()
67+
);
68+
}
69+
70+
/// Tests that adding a derive trait that's not already derived automatically
71+
/// adds it to the end of the derive list.
72+
#[test]
73+
fn test_add_derives_callback() {
74+
let crate_dir =
75+
PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap());
76+
let header_path = crate_dir.join(
77+
"tests/parse_callbacks/add_derives_callback/header_add_derives.h",
78+
);
79+
80+
let builder = Builder::default()
81+
.header(header_path.display().to_string())
82+
.derive_debug(true)
83+
.derive_copy(false)
84+
.derive_default(false)
85+
.derive_partialeq(false)
86+
.derive_eq(false)
87+
.derive_partialord(false)
88+
.derive_ord(false)
89+
.derive_hash(false)
90+
.parse_callbacks(Box::new(AddDerivesCallback(vec![
91+
"Default".to_string()
92+
])));
93+
let bindings = builder.generate().expect("Failed to generate bindings");
94+
let output = write_bindings_to_string(&bindings);
95+
assert!(
96+
output.contains("#[derive(Debug, Default)]"),
97+
"Unexpected bindgen output:\n{}",
98+
output.as_str()
99+
);
100+
}
101+
}

bindgen-tests/tests/parse_callbacks/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
mod add_derives_callback;
12
mod item_discovery_callback;
23

34
use bindgen::callbacks::*;

bindgen/codegen/mod.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,25 @@ fn derives_of_item(
199199
derivable_traits
200200
}
201201

202+
/// Appends the contents of the `custom_derives` iterator to the `derives` vector,
203+
/// ignoring duplicates and preserving order.
204+
fn append_custom_derives<'a, I>(derives: &mut Vec<&'a str>, custom_derives: I)
205+
where
206+
I: Iterator<Item = &'a str>,
207+
{
208+
// Use a HashSet to track already seen elements.
209+
let mut seen: HashSet<&'a str> = Default::default();
210+
seen.extend(derives.iter().copied());
211+
212+
// Add the custom derives to the derives vector, ignoring duplicates.
213+
for custom_derive in custom_derives {
214+
if !seen.contains(custom_derive) {
215+
derives.push(custom_derive);
216+
seen.insert(custom_derive);
217+
}
218+
}
219+
}
220+
202221
impl From<DerivableTraits> for Vec<&'static str> {
203222
fn from(derivable_traits: DerivableTraits) -> Vec<&'static str> {
204223
[
@@ -1043,8 +1062,12 @@ impl CodeGenerator for Type {
10431062
})
10441063
});
10451064
// In most cases this will be a no-op, since custom_derives will be empty.
1046-
derives
1047-
.extend(custom_derives.iter().map(|s| s.as_str()));
1065+
if !custom_derives.is_empty() {
1066+
append_custom_derives(
1067+
&mut derives,
1068+
custom_derives.iter().map(|s| s.as_str()),
1069+
);
1070+
}
10481071
attributes.push(attributes::derives(&derives));
10491072

10501073
let custom_attributes =
@@ -2475,7 +2498,12 @@ impl CodeGenerator for CompInfo {
24752498
})
24762499
});
24772500
// In most cases this will be a no-op, since custom_derives will be empty.
2478-
derives.extend(custom_derives.iter().map(|s| s.as_str()));
2501+
if !custom_derives.is_empty() {
2502+
append_custom_derives(
2503+
&mut derives,
2504+
custom_derives.iter().map(|s| s.as_str()),
2505+
);
2506+
}
24792507

24802508
if !derives.is_empty() {
24812509
attributes.push(attributes::derives(&derives));
@@ -3678,7 +3706,12 @@ impl CodeGenerator for Enum {
36783706
})
36793707
});
36803708
// In most cases this will be a no-op, since custom_derives will be empty.
3681-
derives.extend(custom_derives.iter().map(|s| s.as_str()));
3709+
if !custom_derives.is_empty() {
3710+
append_custom_derives(
3711+
&mut derives,
3712+
custom_derives.iter().map(|s| s.as_str()),
3713+
);
3714+
}
36823715

36833716
attrs.extend(
36843717
item.annotations()

0 commit comments

Comments
 (0)