Skip to content

Commit

Permalink
Make bindgen generate enums as constants by default
Browse files Browse the repository at this point in the history
Also simplifies the logic that determines which enum variation gets chosen.
  • Loading branch information
Cldfire committed Sep 11, 2017
1 parent 4dd4ac7 commit 89915f9
Show file tree
Hide file tree
Showing 51 changed files with 182 additions and 110 deletions.
1 change: 1 addition & 0 deletions bindgen-integration/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ fn main() {

let bindings = Builder::default()
.enable_cxx_namespaces()
.rustified_enum(".*")
.raw_line("pub use self::root::*;")
.header("cpp/Test.h")
.clang_args(&["-x", "c++", "-std=c++11"])
Expand Down
134 changes: 80 additions & 54 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2072,7 +2072,41 @@ impl MethodCodegen for Method {
}
}

/// A helper type to construct enums, either bitfield ones or rust-style ones.
/// A helper type that represents different enum variations.
#[derive(Copy, Clone)]
enum EnumVariation {
Rust,
Bitfield,
Consts,
ModuleConsts
}

impl EnumVariation {
fn is_rust(&self) -> bool {
match *self {
EnumVariation::Rust => true,
_ => false
}
}

fn is_bitfield(&self) -> bool {
match *self {
EnumVariation::Bitfield => true,
_ => false
}
}

/// Both the `Const` and `ModuleConsts` variants will cause this to return
/// true.
fn is_const(&self) -> bool {
match *self {
EnumVariation::Consts | EnumVariation::ModuleConsts => true,
_ => false
}
}
}

/// A helper type to construct different enum variations.
enum EnumBuilder<'a> {
Rust(quote::Tokens),
Bitfield {
Expand All @@ -2088,26 +2122,44 @@ enum EnumBuilder<'a> {

impl<'a> EnumBuilder<'a> {
/// Create a new enum given an item builder, a canonical name, a name for
/// the representation, and whether it should be represented as a rust enum.
/// the representation, and which variation it should be generated as.
fn new(
name: &'a str,
attrs: Vec<quote::Tokens>,
repr: quote::Tokens,
bitfield_like: bool,
constify: bool,
constify_module: bool,
enum_variation: EnumVariation
) -> Self {
let ident = quote::Ident::new(name);
if bitfield_like {
EnumBuilder::Bitfield {
canonical_name: name,
tokens: quote! {

match enum_variation {
EnumVariation::Bitfield => {
EnumBuilder::Bitfield {
canonical_name: name,
tokens: quote! {
#( #attrs )*
pub struct #ident (pub #repr);
},
}
}

EnumVariation::Rust => {
let mut tokens = quote! {
#( #attrs )*
pub struct #ident (pub #repr);
},
pub enum #ident
};
tokens.append("{");
EnumBuilder::Rust(tokens)
}
} else if constify {
if constify_module {

EnumVariation::Consts => {
EnumBuilder::Consts(vec![
quote! {
pub type #ident = #repr;
}
])
}

EnumVariation::ModuleConsts => {
let ident = quote::Ident::new(CONSTIFIED_ENUM_MODULE_REPR_NAME);
let type_definition = quote! {
pub type #ident = #repr;
Expand All @@ -2117,20 +2169,7 @@ impl<'a> EnumBuilder<'a> {
module_name: name,
module_items: vec![type_definition],
}
} else {
EnumBuilder::Consts(vec![
quote! {
pub type #ident = #repr;
}
])
}
} else {
let mut tokens = quote! {
#( #attrs )*
pub enum #ident
};
tokens.append("{");
EnumBuilder::Rust(tokens)
}
}

Expand Down Expand Up @@ -2342,47 +2381,36 @@ impl CodeGenerator for Enum {

// FIXME(emilio): These should probably use the path so it can
// disambiguate between namespaces, just like is_opaque etc.
let is_bitfield = {
ctx.options().bitfield_enums.matches(&name) ||
(enum_ty.name().is_none() &&
self.variants().iter().any(|v| {
ctx.options().bitfield_enums.matches(&v.name())
}))
};

let is_constified_enum_module =
self.is_constified_enum_module(ctx, item);

let is_constified_enum = {
is_constified_enum_module ||
ctx.options().constified_enums.matches(&name) ||
(enum_ty.name().is_none() &&
self.variants().iter().any(|v| {
ctx.options().constified_enums.matches(&v.name())
}))
let variation = if self.is_bitfield(ctx, item) {
EnumVariation::Bitfield
} else if self.is_rustified_enum(ctx, item) {
EnumVariation::Rust
} else if self.is_constified_enum_module(ctx, item) {
EnumVariation::ModuleConsts
} else {
// We generate consts by default
EnumVariation::Consts
};

let is_rust_enum = !is_bitfield && !is_constified_enum;

let mut attrs = vec![];

// FIXME: Rust forbids repr with empty enums. Remove this condition when
// this is allowed.
//
// TODO(emilio): Delegate this to the builders?
if is_rust_enum {
if variation.is_rust() {
if !self.variants().is_empty() {
attrs.push(attributes::repr(repr_name));
}
} else if is_bitfield {
} else if variation.is_bitfield() {
attrs.push(attributes::repr("C"));
}

if let Some(comment) = item.comment(ctx) {
attrs.push(attributes::doc(comment));
}

if !is_constified_enum {
if !variation.is_const() {
attrs.push(attributes::derives(
&["Debug", "Copy", "Clone", "PartialEq", "Eq", "Hash"],
));
Expand Down Expand Up @@ -2427,9 +2455,7 @@ impl CodeGenerator for Enum {
&name,
attrs,
repr,
is_bitfield,
is_constified_enum,
is_constified_enum_module,
variation
);

// A map where we keep a value -> variant relation.
Expand Down Expand Up @@ -2475,7 +2501,7 @@ impl CodeGenerator for Enum {

match seen_values.entry(variant.val()) {
Entry::Occupied(ref entry) => {
if is_rust_enum {
if variation.is_rust() {
let variant_name = ctx.rust_mangle(variant.name());
let mangled_name =
if is_toplevel || enum_ty.name().is_some() {
Expand Down Expand Up @@ -2523,7 +2549,7 @@ impl CodeGenerator for Enum {
// If it's an unnamed enum, or constification is enforced,
// we also generate a constant so it can be properly
// accessed.
if (is_rust_enum && enum_ty.name().is_none()) ||
if (variation.is_rust() && enum_ty.name().is_none()) ||
variant.force_constification()
{
let mangled_name = if is_toplevel {
Expand Down
24 changes: 24 additions & 0 deletions src/ir/enum_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,18 @@ impl Enum {
Ok(Enum::new(repr, variants))
}

/// Whether the enum should be a bitfield
pub fn is_bitfield(&self, ctx: &BindgenContext, item: &Item) -> bool {
let name = item.canonical_name(ctx);
let enum_ty = item.expect_type();

ctx.options().bitfield_enums.matches(&name) ||
(enum_ty.name().is_none() &&
self.variants().iter().any(|v| {
ctx.options().bitfield_enums.matches(&v.name())
}))
}

/// Whether the enum should be an constified enum module
pub fn is_constified_enum_module(
&self,
Expand All @@ -143,6 +155,18 @@ impl Enum {
ctx.options().constified_enum_modules.matches(&v.name())
}))
}

/// Whether the enum should be a Rust enum
pub fn is_rustified_enum(&self, ctx: &BindgenContext, item: &Item) -> bool {
let name = item.canonical_name(ctx);
let enum_ty = item.expect_type();

ctx.options().rustified_enums.matches(&name) ||
(enum_ty.name().is_none() &&
self.variants().iter().any(|v| {
ctx.options().rustified_enums.matches(&v.name())
}))
}
}

/// A single enum variant, to be contained only in an enum.
Expand Down
31 changes: 18 additions & 13 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@ impl Builder {
.count();

self.options
.constified_enums
.rustified_enums
.get_items()
.iter()
.map(|item| {
output_vector.push("--constified-enum".into());
output_vector.push("--rustified-enum".into());
output_vector.push(
item.trim_left_matches("^")
.trim_right_matches("$")
Expand Down Expand Up @@ -666,21 +666,26 @@ impl Builder {
self
}

/// Mark the given enum (or set of enums, if using a pattern) as a set of
/// constants.
/// Mark the given enum (or set of enums, if using a pattern) as a Rust
/// enum.
///
/// This makes bindgen generate constants instead of enums. Regular
/// This makes bindgen generate enums instead of constants. Regular
/// expressions are supported.
pub fn constified_enum<T: AsRef<str>>(mut self, arg: T) -> Builder {
self.options.constified_enums.insert(arg);
///
/// **Use this with caution.** You should not be using Rust enums unless
/// you have complete control of the C/C++ code that you're binding to.
/// Take a look at https://github.com/rust-lang/rust/issues/36927 for
/// more information.
pub fn rustified_enum<T: AsRef<str>>(mut self, arg: T) -> Builder {
self.options.rustified_enums.insert(arg);
self
}

/// Mark the given enum (or set of enums, if using a pattern) as a set of
/// constants that should be put into a module.
///
/// This makes bindgen generate a modules containing constants instead of
/// enums. Regular expressions are supported.
/// This makes bindgen generate modules containing constants instead of
/// just constants. Regular expressions are supported.
pub fn constified_enum_module<T: AsRef<str>>(mut self, arg: T) -> Builder {
self.options.constified_enum_modules.insert(arg);
self
Expand Down Expand Up @@ -1094,8 +1099,8 @@ pub struct BindgenOptions {
/// The enum patterns to mark an enum as bitfield.
pub bitfield_enums: RegexSet,

/// The enum patterns to mark an enum as constant.
pub constified_enums: RegexSet,
/// The enum patterns to mark an enum as a Rust enum.
pub rustified_enums: RegexSet,

/// The enum patterns to mark an enum as a module of constants.
pub constified_enum_modules: RegexSet,
Expand Down Expand Up @@ -1251,7 +1256,7 @@ impl BindgenOptions {
self.opaque_types.build();
self.bitfield_enums.build();
self.constified_enum_modules.build();
self.constified_enums.build();
self.rustified_enums.build();
}

/// Update rust target version
Expand Down Expand Up @@ -1286,7 +1291,7 @@ impl Default for BindgenOptions {
whitelisted_functions: Default::default(),
whitelisted_vars: Default::default(),
bitfield_enums: Default::default(),
constified_enums: Default::default(),
rustified_enums: Default::default(),
constified_enum_modules: Default::default(),
builtins: false,
links: vec![],
Expand Down
17 changes: 8 additions & 9 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,18 @@ where
.takes_value(true)
.multiple(true)
.number_of_values(1),
Arg::with_name("constified-enum")
.long("constified-enum")
.help("Mark any enum whose name matches <regex> as a set of \
constants instead of an enumeration.")
Arg::with_name("rustified-enum")
.long("rustified-enum")
.help("Mark any enum whose name matches <regex> as a Rust enum \
instead of a set of constants.")
.value_name("regex")
.takes_value(true)
.multiple(true)
.number_of_values(1),
Arg::with_name("constified-enum-module")
.long("constified-enum-module")
.help("Mark any enum whose name matches <regex> as a module of \
constants instead of an enumeration. This option \
implies \"--constified-enum.\"")
constants instead of just constants.")
.value_name("regex")
.takes_value(true)
.multiple(true)
Expand Down Expand Up @@ -292,9 +291,9 @@ where
}
}

if let Some(constifieds) = matches.values_of("constified-enum") {
for regex in constifieds {
builder = builder.constified_enum(regex);
if let Some(rustifieds) = matches.values_of("rustified-enum") {
for regex in rustifieds {
builder = builder.rustified_enum(regex);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/headers/anon_enum.hpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --with-derive-hash --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialeq --with-derive-eq --rustified-enum .*
struct Test {
int foo;
float bar;
Expand Down
2 changes: 1 addition & 1 deletion tests/headers/anon_enum_trait.hpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --with-derive-hash --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialeq --with-derive-eq --rustified-enum .*

template<typename _Tp>
class DataType {
Expand Down
2 changes: 1 addition & 1 deletion tests/headers/anon_enum_whitelist.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --whitelist-var NODE_.*
// bindgen-flags: --whitelist-var NODE_.* --rustified-enum .*

enum {
NODE_FLAG_FOO,
Expand Down
2 changes: 1 addition & 1 deletion tests/headers/anon_union.hpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --with-derive-hash --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialeq --with-derive-eq --rustified-enum .*
template<typename T>
struct TErrorResult {
enum UnionState {
Expand Down
Loading

0 comments on commit 89915f9

Please sign in to comment.