Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting the output semantically #2254

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 18 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Expand Up @@ -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"
Expand Down
70 changes: 70 additions & 0 deletions src/lib.rs
Expand Up @@ -86,6 +86,7 @@ use std::{env, iter};
// Some convenient typedefs for a fast hash map and hash set.
type HashMap<K, V> = ::rustc_hash::FxHashMap<K, V>;
type HashSet<K> = ::rustc_hash::FxHashSet<K>;
use quote::ToTokens;
pub(crate) use std::collections::hash_map::Entry;

/// Default prefix for the anon fields.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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<Bindings, BindgenError> {
// Add any extra arguments from the environment to the clang command line.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2153,6 +2169,7 @@ impl Default for BindgenOptions {
c_naming: false,
force_explicit_padding: false,
vtable_generation: false,
sort_semantically: false,
}
}
}
Expand Down Expand Up @@ -2437,6 +2454,59 @@ impl Bindings {

let (items, options) = 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 scond one won't panic because we know original output has something in
amanjeev marked this conversation as resolved.
Show resolved Hide resolved
// it already.
let mut syn_parsed_items =
syn::parse2::<syn::ItemMod>(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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you need warnings here for this to compile. r=me with that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was in the middle of adding those. May have to do that on Monday when I am back :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved (I cannot find the solved button for some odd reason)

options,
module: quote! {
#( #synful_items )*
},
});
}

Ok(Bindings {
options,
module: quote! {
Expand Down
7 changes: 7 additions & 0 deletions src/options.rs
Expand Up @@ -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"),
amanjeev marked this conversation as resolved.
Show resolved Hide resolved
amanjeev marked this conversation as resolved.
Show resolved Hide resolved
Arg::new("V")
.long("version")
.help("Prints the version, and exits"),
Expand Down Expand Up @@ -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))
}
75 changes: 75 additions & 0 deletions tests/expectations/tests/sorted-items.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

82 changes: 82 additions & 0 deletions tests/expectations/tests/unsorted-items.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions tests/headers/sorted/sorted-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;
15 changes: 15 additions & 0 deletions tests/headers/unsorted-items.h
@@ -0,0 +1,15 @@
int foo();
typedef int number;
int bar(number x);
struct Point
{
number x;
number y;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test testing much? It seems it's not using the new feature, so maybe remove it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could argue that this is testing that not using --sort-semantically doesn't change the order. But in a sense this is tested with the rest of the test suite already as we didn't modify any expected rs file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilio should we remove it then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way is fine, sorry for the lag.

};
struct Angle
{
number a;
number b;
};
int baz(struct Point point);
const number NUMBER = 42;