From 4f7ee815ca513da3c5ef73f7c3af0759e1ccf8d0 Mon Sep 17 00:00:00 2001 From: Amanjeev Sethi Date: Wed, 24 Aug 2022 18:45:03 -0400 Subject: [PATCH] Sorting the output semantically (#2254) Generated code needs some sorting in a way that is semantically appealing. The request[1] asks for basic sorting like "types are declared first, then all structs, then all consts, then all function signatures, etc. [1] https://github.com/rust-lang/rust-bindgen/issues/1743 Signed-off-by: Amanjeev Sethi Co-authored-by: Christian Poveda Co-authored-by: Darren Kulp --- CHANGELOG.md | 4 ++ Cargo.lock | 24 +++++-- Cargo.toml | 1 + src/lib.rs | 71 +++++++++++++++++++ src/options.rs | 7 ++ tests/expectations/tests/sorted-items.rs | 82 ++++++++++++++++++++++ tests/expectations/tests/unsorted-items.rs | 82 ++++++++++++++++++++++ tests/headers/sorted-items.h | 17 +++++ tests/headers/unsorted-items.h | 15 ++++ 9 files changed, 297 insertions(+), 6 deletions(-) create mode 100644 tests/expectations/tests/sorted-items.rs create mode 100644 tests/expectations/tests/unsorted-items.rs create mode 100644 tests/headers/sorted-items.h create mode 100644 tests/headers/unsorted-items.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b9b164ae9..6c2e64e670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -143,6 +143,8 @@ ## Added + * new feature: `--sort-semantically` flag to sort the output in a predefined manner [(#1743)] + ## Changed * clap has been updated, new msrv is 1.57. @@ -153,6 +155,8 @@ ## Security +[(#1743)]: https://github.com/rust-lang/rust-bindgen/issues/1743 + # 0.60.1 Released 2022/06/06 diff --git a/Cargo.lock b/Cargo.lock index 0c7c2f7851..fa908ea9d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -47,6 +47,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", + "syn", "tempfile", "which", ] @@ -260,11 +261,11 @@ checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857" [[package]] name = "proc-macro2" -version = "1.0.28" +version = "1.0.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c7ed8b8c7b886ea3ed7dde405212185f423ab44682667c8c6dd14aa1d9f6612" +checksum = "0a2ca2c61bc9f3d74d2886294ab7b9853abd9c1ad903a3ac7815c58989bb7bab" dependencies = [ - "unicode-xid", + "unicode-ident", ] [[package]] @@ -369,6 +370,17 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "syn" +version = "1.0.99" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58dbef6ec655055e20b86b15a8cc6d439cca19b667537ac6a1369572d151ab13" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "tempfile" version = "3.2.0" @@ -399,10 +411,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b1141d4d61095b28419e22cb0bbf02755f5e54e0526f97f1e3d1d160e60885fb" [[package]] -name = "unicode-xid" -version = "0.2.2" +name = "unicode-ident" +version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" +checksum = "c4f5b37a154999a8f3f98cc23a628d850e154479cd94decf3414696e12e31aaf" [[package]] name = "version_check" diff --git a/Cargo.toml b/Cargo.toml index ed7571ae99..311110b3ec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,7 @@ lazycell = "1" lazy_static = "1" peeking_take_while = "0.1.2" quote = { version = "1", default-features = false } +syn = { version = "1.0.99", features = ["full", "extra-traits"]} regex = { version = "1.5", default-features = false , features = ["std", "unicode"] } which = { version = "4.2.1", optional = true, default-features = false } shlex = "1" diff --git a/src/lib.rs b/src/lib.rs index c102c47d69..b90faba613 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -86,6 +86,7 @@ use std::{env, iter}; // Some convenient typedefs for a fast hash map and hash set. type HashMap = ::rustc_hash::FxHashMap; type HashSet = ::rustc_hash::FxHashSet; +use quote::ToTokens; pub(crate) use std::collections::hash_map::Entry; /// Default prefix for the anon fields. @@ -587,6 +588,10 @@ impl Builder { output_vector.push("--vtable-generation".into()); } + if self.options.sort_semantically { + output_vector.push("--sort-semantically".into()); + } + // Add clang arguments output_vector.push("--".into()); @@ -1476,6 +1481,14 @@ impl Builder { self } + /// 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 { + self.options.sort_semantically = 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. @@ -2005,6 +2018,9 @@ struct BindgenOptions { /// Emit vtable functions. vtable_generation: bool, + + /// Sort the code generation + sort_semantically: bool, } /// TODO(emilio): This is sort of a lie (see the error message that results from @@ -2153,6 +2169,7 @@ impl Default for BindgenOptions { c_naming: false, force_explicit_padding: false, vtable_generation: false, + sort_semantically: false, } } } @@ -2438,6 +2455,60 @@ impl Bindings { let (items, options, warnings) = codegen::codegen(context); + if options.sort_semantically { + let module_wrapped_tokens = + quote!(mod wrapper_for_sorting_hack { #( #items )* }); + + // This semantically sorting business is a hack, for now. This means that we are + // re-parsing already generated code using `syn` (as opposed to `quote`) because + // `syn` provides us more control over the elements. + // One caveat is that some of the items coming from `quote`d output might have + // multiple items within them. Hence, we have to wrap the incoming in a `mod`. + // The two `unwrap`s here are deliberate because + // The first one won't panic because we build the `mod` and know it is there + // The second one won't panic because we know original output has something in + // it already. + let mut syn_parsed_items = + syn::parse2::(module_wrapped_tokens) + .unwrap() + .content + .unwrap() + .1; + + syn_parsed_items.sort_by_key(|item| match item { + syn::Item::Type(_) => 0, + syn::Item::Struct(_) => 1, + syn::Item::Const(_) => 2, + syn::Item::Fn(_) => 3, + syn::Item::Enum(_) => 4, + syn::Item::Union(_) => 5, + syn::Item::Static(_) => 6, + syn::Item::Trait(_) => 7, + syn::Item::TraitAlias(_) => 8, + syn::Item::Impl(_) => 9, + syn::Item::Mod(_) => 10, + syn::Item::Use(_) => 11, + syn::Item::Verbatim(_) => 12, + syn::Item::ExternCrate(_) => 13, + syn::Item::ForeignMod(_) => 14, + syn::Item::Macro(_) => 15, + syn::Item::Macro2(_) => 16, + _ => 18, + }); + + let synful_items = syn_parsed_items + .into_iter() + .map(|item| item.into_token_stream()); + + return Ok(Bindings { + options, + warnings, + module: quote! { + #( #synful_items )* + }, + }); + } + Ok(Bindings { options, warnings, diff --git a/src/options.rs b/src/options.rs index a11f3ddd7c..83da21f42f 100644 --- a/src/options.rs +++ b/src/options.rs @@ -515,6 +515,9 @@ where Arg::new("vtable-generation") .long("vtable-generation") .help("Enables generation of vtable functions."), + Arg::new("sort-semantically") + .long("sort-semantically") + .help("Enables sorting of code generation in a predefined manner."), Arg::new("V") .long("version") .help("Prints the version, and exits"), @@ -1000,5 +1003,9 @@ where builder = builder.vtable_generation(true); } + if matches.is_present("sort-semantically") { + builder = builder.sort_semantically(true); + } + Ok((builder, output, verbose)) } diff --git a/tests/expectations/tests/sorted-items.rs b/tests/expectations/tests/sorted-items.rs new file mode 100644 index 0000000000..7df7c3d71e --- /dev/null +++ b/tests/expectations/tests/sorted-items.rs @@ -0,0 +1,82 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +pub type number = ::std::os::raw::c_int; +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Point { + pub x: number, + pub y: number, +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Angle { + pub a: number, + pub b: number, +} +pub const NUMBER: number = 42; +#[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::(), + 8usize, + 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)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).y) as usize - ptr as usize }, + 4usize, + concat!("Offset of field: ", stringify!(Point), "::", stringify!(y)) + ); +} +#[test] +fn bindgen_test_layout_Angle() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(Angle)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(Angle)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).a) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Angle), "::", stringify!(a)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).b) as usize - ptr as usize }, + 4usize, + concat!("Offset of field: ", stringify!(Angle), "::", stringify!(b)) + ); +} +extern "C" { + pub fn foo() -> ::std::os::raw::c_int; +} +extern "C" { + pub fn bar(x: number) -> ::std::os::raw::c_int; +} +extern "C" { + pub fn baz(point: Point) -> ::std::os::raw::c_int; +} diff --git a/tests/expectations/tests/unsorted-items.rs b/tests/expectations/tests/unsorted-items.rs new file mode 100644 index 0000000000..ce0c5f3f66 --- /dev/null +++ b/tests/expectations/tests/unsorted-items.rs @@ -0,0 +1,82 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +extern "C" { + pub fn foo() -> ::std::os::raw::c_int; +} +pub type number = ::std::os::raw::c_int; +extern "C" { + pub fn bar(x: number) -> ::std::os::raw::c_int; +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Point { + pub x: number, + pub y: number, +} +#[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::(), + 8usize, + 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)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).y) as usize - ptr as usize }, + 4usize, + concat!("Offset of field: ", stringify!(Point), "::", stringify!(y)) + ); +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Angle { + pub a: number, + pub b: number, +} +#[test] +fn bindgen_test_layout_Angle() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(Angle)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(Angle)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).a) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Angle), "::", stringify!(a)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).b) as usize - ptr as usize }, + 4usize, + concat!("Offset of field: ", stringify!(Angle), "::", stringify!(b)) + ); +} +extern "C" { + pub fn baz(point: Point) -> ::std::os::raw::c_int; +} +pub const NUMBER: number = 42; diff --git a/tests/headers/sorted-items.h b/tests/headers/sorted-items.h new file mode 100644 index 0000000000..11fc2ef40b --- /dev/null +++ b/tests/headers/sorted-items.h @@ -0,0 +1,17 @@ +// bindgen-flags: --sort-semantically -- --target=x86_64-unknown-linux + +int foo(); +typedef int number; +int bar(number x); +struct Point +{ + number x; + number y; +}; +struct Angle +{ + number a; + number b; +}; +int baz(struct Point point); +const number NUMBER = 42; diff --git a/tests/headers/unsorted-items.h b/tests/headers/unsorted-items.h new file mode 100644 index 0000000000..23962d18d8 --- /dev/null +++ b/tests/headers/unsorted-items.h @@ -0,0 +1,15 @@ +int foo(); +typedef int number; +int bar(number x); +struct Point +{ + number x; + number y; +}; +struct Angle +{ + number a; + number b; +}; +int baz(struct Point point); +const number NUMBER = 42;