Auto merge of #976 - Cldfire:default-to-constified-enums, r=fitzgen

Generate constants for enums by default

Closes #758.

The first commit does strictly what the issue description described, the second does a small amount of (what I believe is) logic simplification.

Hopefully I didn't miss any tests when adding the `--rustified-enum .*` flag to the ones that needed it; all of the tests are passing for me, though, so I don't think I did.

bindgen-integration/build.rs

@@ -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"])

src/codegen/mod.rs

@@ -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 {
@@ -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;
@@ -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)
         }
     }
 
@@ -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"],
             ));
@@ -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.
@@ -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() {
@@ -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 {

src/ir/enum_ty.rs

@@ -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,
@@ -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.

src/lib.rs

@@ -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("$")
@@ -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
@@ -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,
@@ -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
@@ -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![],

src/options.rs

@@ -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)
@@ -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);
         }
     }
 

tests/headers/anon_enum.hpp

@@ -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;

tests/headers/anon_enum_trait.hpp

@@ -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 {

tests/headers/anon_enum_whitelist.h

@@ -1,4 +1,4 @@
-// bindgen-flags: --whitelist-var NODE_.*
+// bindgen-flags: --whitelist-var NODE_.* --rustified-enum .*
 
 enum {
   NODE_FLAG_FOO,

tests/headers/anon_union.hpp

@@ -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 {