From 3378edf272c3728eba86fc92233539f58c4a0a1e Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 25 Aug 2022 13:19:29 -0500 Subject: [PATCH 1/2] add option to deduplicate extern blocks --- src/lib.rs | 73 ++++++++++++++++++- src/options.rs | 7 ++ .../tests/deduplicate-extern-blocks.rs | 37 ++++++++++ tests/headers/deduplicate-extern-blocks.h | 6 ++ 4 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 tests/expectations/tests/deduplicate-extern-blocks.rs create mode 100644 tests/headers/deduplicate-extern-blocks.h diff --git a/src/lib.rs b/src/lib.rs index fb02fe16f5..03cbfffc53 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -592,6 +592,10 @@ impl Builder { output_vector.push("--sort-semantically".into()); } + if self.options.deduplicate_extern_blocks { + output_vector.push("--deduplicate-extern-blocks".into()); + } + // Add clang arguments output_vector.push("--".into()); @@ -1481,7 +1485,7 @@ impl Builder { self } - /// If true, enables the sorting of the output in a predefined manner + /// If true, enables the sorting of the output in a predefined manner. /// /// TODO: Perhaps move the sorting order out into a config pub fn sort_semantically(mut self, doit: bool) -> Self { @@ -1489,6 +1493,12 @@ impl Builder { self } + /// If true, deduplicates extern blocks. + pub fn deduplicate_extern_blocks(mut self, doit: bool) -> Self { + self.options.deduplicate_extern_blocks = doit; + self + } + /// Generate the Rust bindings using the options built up thus far. pub fn generate(mut self) -> Result { // Add any extra arguments from the environment to the clang command line. @@ -2019,8 +2029,11 @@ struct BindgenOptions { /// Emit vtable functions. vtable_generation: bool, - /// Sort the code generation + /// Sort the code generation. sort_semantically: bool, + + /// Deduplicate `extern` blocks. + deduplicate_extern_blocks: bool, } /// TODO(emilio): This is sort of a lie (see the error message that results from @@ -2031,7 +2044,7 @@ impl ::std::panic::UnwindSafe for BindgenOptions {} impl BindgenOptions { /// Whether any of the enabled options requires `syn`. fn require_syn(&self) -> bool { - self.sort_semantically + self.sort_semantically || self.deduplicate_extern_blocks } fn build(&mut self) { @@ -2175,6 +2188,7 @@ impl Default for BindgenOptions { force_explicit_padding: false, vtable_generation: false, sort_semantically: false, + deduplicate_extern_blocks: false, } } } @@ -2480,6 +2494,59 @@ impl Bindings { .unwrap() .1; + if options.deduplicate_extern_blocks { + // Here we will store all the items after deduplication. + let mut items = Vec::new(); + + // Keep all the extern blocks in a different `Vec` for faster search. + let mut foreign_mods = Vec::::new(); + for item in syn_parsed_items { + match item { + syn::Item::ForeignMod(syn::ItemForeignMod { + attrs, + abi, + brace_token, + items: foreign_items, + }) => { + let mut exists = false; + for foreign_mod in &mut foreign_mods { + // Check if there is a extern block with the same ABI and + // attributes. + if foreign_mod.attrs == attrs && + foreign_mod.abi == abi + { + // Merge the items of the two blocks. + foreign_mod + .items + .extend_from_slice(&foreign_items); + exists = true; + break; + } + } + // If no existing extern block had the same ABI and attributes, store + // it. + if !exists { + foreign_mods.push(syn::ItemForeignMod { + attrs, + abi, + brace_token, + items: foreign_items, + }); + } + } + // If the item is not an extern block, we don't have to do anything. + _ => items.push(item), + } + } + + // Move all the extern blocks alongiside the rest of the items. + for foreign_mod in foreign_mods { + items.push(syn::Item::ForeignMod(foreign_mod)); + } + + syn_parsed_items = items; + } + if options.sort_semantically { syn_parsed_items.sort_by_key(|item| match item { syn::Item::Type(_) => 0, diff --git a/src/options.rs b/src/options.rs index 83da21f42f..7164773630 100644 --- a/src/options.rs +++ b/src/options.rs @@ -518,6 +518,9 @@ where Arg::new("sort-semantically") .long("sort-semantically") .help("Enables sorting of code generation in a predefined manner."), + Arg::new("deduplicate-extern-blocks") + .long("deduplicate-extern-blocks") + .help("Deduplicates extern blocks."), Arg::new("V") .long("version") .help("Prints the version, and exits"), @@ -1007,5 +1010,9 @@ where builder = builder.sort_semantically(true); } + if matches.is_present("deduplicate-extern-blocks") { + builder = builder.deduplicate_extern_blocks(true); + } + Ok((builder, output, verbose)) } diff --git a/tests/expectations/tests/deduplicate-extern-blocks.rs b/tests/expectations/tests/deduplicate-extern-blocks.rs new file mode 100644 index 0000000000..66ceeff07f --- /dev/null +++ b/tests/expectations/tests/deduplicate-extern-blocks.rs @@ -0,0 +1,37 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Point { + pub x: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_Point() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(Point)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(Point)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).x) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Point), "::", stringify!(x)) + ); +} +extern "C" { + pub fn foo() -> ::std::os::raw::c_int; + pub fn bar() -> ::std::os::raw::c_int; +} diff --git a/tests/headers/deduplicate-extern-blocks.h b/tests/headers/deduplicate-extern-blocks.h new file mode 100644 index 0000000000..7795ed2162 --- /dev/null +++ b/tests/headers/deduplicate-extern-blocks.h @@ -0,0 +1,6 @@ +// bindgen-flags: --deduplicate-extern-blocks -- --target=x86_64-unknown-linux +int foo(); +typedef struct Point { + int x; +} Point; +int bar(); From 70735e2fed0fd640b0250e9cdab20fa7d79beea4 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Fri, 9 Sep 2022 16:09:04 -0500 Subject: [PATCH 2/2] `s/deduplicate/merge/g` --- src/lib.rs | 18 +++++++++--------- src/options.rs | 8 ++++---- ...extern-blocks.rs => merge-extern-blocks.rs} | 0 tests/headers/deduplicate-extern-blocks.h | 6 ------ tests/headers/merge-extern-blocks.h | 6 ++++++ 5 files changed, 19 insertions(+), 19 deletions(-) rename tests/expectations/tests/{deduplicate-extern-blocks.rs => merge-extern-blocks.rs} (100%) delete mode 100644 tests/headers/deduplicate-extern-blocks.h create mode 100644 tests/headers/merge-extern-blocks.h diff --git a/src/lib.rs b/src/lib.rs index 03cbfffc53..a4dfd7b1ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -592,8 +592,8 @@ impl Builder { output_vector.push("--sort-semantically".into()); } - if self.options.deduplicate_extern_blocks { - output_vector.push("--deduplicate-extern-blocks".into()); + if self.options.merge_extern_blocks { + output_vector.push("--merge-extern-blocks".into()); } // Add clang arguments @@ -1493,9 +1493,9 @@ impl Builder { self } - /// If true, deduplicates extern blocks. - pub fn deduplicate_extern_blocks(mut self, doit: bool) -> Self { - self.options.deduplicate_extern_blocks = doit; + /// If true, merges extern blocks. + pub fn merge_extern_blocks(mut self, doit: bool) -> Self { + self.options.merge_extern_blocks = doit; self } @@ -2033,7 +2033,7 @@ struct BindgenOptions { sort_semantically: bool, /// Deduplicate `extern` blocks. - deduplicate_extern_blocks: bool, + merge_extern_blocks: bool, } /// TODO(emilio): This is sort of a lie (see the error message that results from @@ -2044,7 +2044,7 @@ impl ::std::panic::UnwindSafe for BindgenOptions {} impl BindgenOptions { /// Whether any of the enabled options requires `syn`. fn require_syn(&self) -> bool { - self.sort_semantically || self.deduplicate_extern_blocks + self.sort_semantically || self.merge_extern_blocks } fn build(&mut self) { @@ -2188,7 +2188,7 @@ impl Default for BindgenOptions { force_explicit_padding: false, vtable_generation: false, sort_semantically: false, - deduplicate_extern_blocks: false, + merge_extern_blocks: false, } } } @@ -2494,7 +2494,7 @@ impl Bindings { .unwrap() .1; - if options.deduplicate_extern_blocks { + if options.merge_extern_blocks { // Here we will store all the items after deduplication. let mut items = Vec::new(); diff --git a/src/options.rs b/src/options.rs index 7164773630..8ae7cbe864 100644 --- a/src/options.rs +++ b/src/options.rs @@ -518,8 +518,8 @@ where Arg::new("sort-semantically") .long("sort-semantically") .help("Enables sorting of code generation in a predefined manner."), - Arg::new("deduplicate-extern-blocks") - .long("deduplicate-extern-blocks") + Arg::new("merge-extern-blocks") + .long("merge-extern-blocks") .help("Deduplicates extern blocks."), Arg::new("V") .long("version") @@ -1010,8 +1010,8 @@ where builder = builder.sort_semantically(true); } - if matches.is_present("deduplicate-extern-blocks") { - builder = builder.deduplicate_extern_blocks(true); + if matches.is_present("merge-extern-blocks") { + builder = builder.merge_extern_blocks(true); } Ok((builder, output, verbose)) diff --git a/tests/expectations/tests/deduplicate-extern-blocks.rs b/tests/expectations/tests/merge-extern-blocks.rs similarity index 100% rename from tests/expectations/tests/deduplicate-extern-blocks.rs rename to tests/expectations/tests/merge-extern-blocks.rs diff --git a/tests/headers/deduplicate-extern-blocks.h b/tests/headers/deduplicate-extern-blocks.h deleted file mode 100644 index 7795ed2162..0000000000 --- a/tests/headers/deduplicate-extern-blocks.h +++ /dev/null @@ -1,6 +0,0 @@ -// bindgen-flags: --deduplicate-extern-blocks -- --target=x86_64-unknown-linux -int foo(); -typedef struct Point { - int x; -} Point; -int bar(); diff --git a/tests/headers/merge-extern-blocks.h b/tests/headers/merge-extern-blocks.h new file mode 100644 index 0000000000..1d46b7d4a3 --- /dev/null +++ b/tests/headers/merge-extern-blocks.h @@ -0,0 +1,6 @@ +// bindgen-flags: --merge-extern-blocks -- --target=x86_64-unknown-linux +int foo(); +typedef struct Point { + int x; +} Point; +int bar();