Make bindgen generate enums as constants by default

Also simplifies the logic that determines which enum variation gets chosen.

4 files changed, 130 insertions, 76 deletions

diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
index f13bd65a..b8416314 100644
--- a/src/codegen/mod.rs
+++ b/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,39 +2381,28 @@ 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"));
         }
 
@@ -2382,7 +2410,7 @@ impl CodeGenerator for Enum {
             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 {
diff --git a/src/ir/enum_ty.rs b/src/ir/enum_ty.rs
index bcd56974..dc56f64a 100644
--- a/src/ir/enum_ty.rs
+++ b/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.
diff --git a/src/lib.rs b/src/lib.rs
index b6310713..dc640911 100644
--- a/src/lib.rs
+++ b/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![],
diff --git a/src/options.rs b/src/options.rs
index 50238f1c..383a8899 100644
--- a/src/options.rs
+++ b/src/options.rs
@@ -34,10 +34,10 @@ 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)
@@ -45,8 +45,7 @@ where
             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);
         }
     }