Auto merge of #437 - emilio:constify-all-enums, r=fitzgen

codegen: Respect original repr for bitfield-like enums, add a constifying variant.

r? @fitzgen

Fixes #430

Author: bors-servo

src/codegen/mod.rs

@@ -1479,28 +1479,33 @@ enum EnumBuilder<'a> {
         canonical_name: &'a str,
         aster: P<ast::Item>,
     },
+    Consts { aster: P<ast::Item>, }
 }
 
 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.
     fn new(aster: aster::item::ItemBuilder<aster::invoke::Identity>,
            name: &'a str,
-           repr_name: &str,
-           is_rust: bool)
+           repr: P<ast::Ty>,
+           bitfield_like: bool,
+           constify: bool)
            -> Self {
-        if is_rust {
-            EnumBuilder::Rust(aster.enum_(name))
-        } else {
+        if bitfield_like {
             EnumBuilder::Bitfield {
                 canonical_name: name,
                 aster: aster.tuple_struct(name)
                     .field()
                     .pub_()
-                    .ty()
-                    .id(repr_name)
+                    .build_ty(repr)
                     .build(),
             }
+        } else if constify {
+            EnumBuilder::Consts {
+                aster: aster.type_(name).build_ty(repr),
+            }
+        } else {
+            EnumBuilder::Rust(aster.enum_(name))
         }
     }
 
@@ -1550,6 +1555,25 @@ impl<'a> EnumBuilder<'a> {
                 result.push(constant);
                 self
             }
+            EnumBuilder::Consts { .. } => {
+                let constant_name = match mangling_prefix {
+                    Some(prefix) => {
+                        Cow::Owned(format!("{}_{}", prefix, variant_name))
+                    }
+                    None => variant_name,
+                };
+
+                let constant = aster::AstBuilder::new()
+                    .item()
+                    .pub_()
+                    .const_(&*constant_name)
+                    .expr()
+                    .build(expr)
+                    .build(rust_ty);
+
+                result.push(constant);
+                self
+            }
         }
     }
 
@@ -1579,6 +1603,7 @@ impl<'a> EnumBuilder<'a> {
                 result.push(impl_);
                 aster
             }
+            EnumBuilder::Consts { aster, .. } => aster,
         }
     }
 }
@@ -1634,6 +1659,8 @@ impl CodeGenerator for Enum {
 
         let mut builder = aster::AstBuilder::new().item().pub_();
 
+        // 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() &&
@@ -1642,30 +1669,42 @@ impl CodeGenerator for Enum {
                 .any(|v| ctx.options().bitfield_enums.matches(&v.name())))
         };
 
-        let is_rust_enum = !is_bitfield;
+        let is_constified_enum = {
+            ctx.options().constified_enums.matches(&name) ||
+            (enum_ty.name().is_none() &&
+             self.variants()
+                .iter()
+                .any(|v| ctx.options().constified_enums.matches(&v.name())))
+        };
+
+        let is_rust_enum = !is_bitfield && !is_constified_enum;
 
         // 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 !self.variants().is_empty() {
                 builder = builder.with_attr(attributes::repr(repr_name));
             }
-        } else {
+        } else if is_bitfield {
             builder = builder.with_attr(attributes::repr("C"));
         }
 
         if let Some(comment) = item.comment() {
             builder = builder.with_attr(attributes::doc(comment));
         }
 
-        let derives = attributes::derives(&["Debug",
-                                            "Copy",
-                                            "Clone",
-                                            "PartialEq",
-                                            "Eq",
-                                            "Hash"]);
+        if !is_constified_enum {
+            let derives = attributes::derives(&["Debug",
+                                                "Copy",
+                                                "Clone",
+                                                "PartialEq",
+                                                "Eq",
+                                                "Hash"]);
 
-        builder = builder.with_attr(derives);
+            builder = builder.with_attr(derives);
+        }
 
         fn add_constant<'a>(enum_: &Type,
                             // Only to avoid recomputing every time.
@@ -1695,8 +1734,16 @@ impl CodeGenerator for Enum {
             result.push(constant);
         }
 
+        let repr = self.repr()
+            .map(|repr| repr.to_rust_ty(ctx))
+            .unwrap_or_else(|| helpers::ast_ty::raw_type(ctx, repr_name));
+
         let mut builder =
-            EnumBuilder::new(builder, &name, repr_name, is_rust_enum);
+            EnumBuilder::new(builder,
+                             &name,
+                             repr,
+                             is_bitfield,
+                             is_constified_enum);
 
         // A map where we keep a value -> variant relation.
         let mut seen_values = HashMap::<_, String>::new();

src/lib.rs

@@ -182,34 +182,39 @@ impl Builder {
         self
     }
 
-    /// Hide the given type from the generated bindings. Regular expressions are supported.
+    /// Hide the given type from the generated bindings. Regular expressions are
+    /// supported.
     pub fn hide_type<T: AsRef<str>>(mut self, arg: T) -> Builder {
         self.options.hidden_types.insert(arg);
         self
     }
 
-    /// Treat the given type as opaque in the generated bindings. Regular expressions are supported.
+    /// Treat the given type as opaque in the generated bindings. Regular
+    /// expressions are supported.
     pub fn opaque_type<T: AsRef<str>>(mut self, arg: T) -> Builder {
         self.options.opaque_types.insert(arg);
         self
     }
 
     /// Whitelist the given type so that it (and all types that it transitively
-    /// refers to) appears in the generated bindings. Regular expressions are supported.
+    /// refers to) appears in the generated bindings. Regular expressions are
+    /// supported.
     pub fn whitelisted_type<T: AsRef<str>>(mut self, arg: T) -> Builder {
         self.options.whitelisted_types.insert(arg);
         self
     }
 
     /// Whitelist the given function so that it (and all types that it
-    /// transitively refers to) appears in the generated bindings. Regular expressions are supported.
+    /// transitively refers to) appears in the generated bindings. Regular
+    /// expressions are supported.
     pub fn whitelisted_function<T: AsRef<str>>(mut self, arg: T) -> Builder {
         self.options.whitelisted_functions.insert(arg);
         self
     }
 
     /// Whitelist the given variable so that it (and all types that it
-    /// transitively refers to) appears in the generated bindings. Regular expressions are supported.
+    /// transitively refers to) appears in the generated bindings. Regular
+    /// expressions are supported.
     pub fn whitelisted_var<T: AsRef<str>>(mut self, arg: T) -> Builder {
         self.options.whitelisted_vars.insert(arg);
         self
@@ -218,12 +223,23 @@ impl Builder {
     /// Mark the given enum (or set of enums, if using a pattern) as being
     /// bitfield-like. Regular expressions are supported.
     ///
-    /// This makes bindgen generate a type that isn't a rust `enum`.
+    /// This makes bindgen generate a type that isn't a rust `enum`. Regular
+    /// expressions are supported.
     pub fn bitfield_enum<T: AsRef<str>>(mut self, arg: T) -> Builder {
         self.options.bitfield_enums.insert(arg);
         self
     }
 
+    /// Mark the given enum (or set of enums, if using a pattern) as being
+    /// constant.
+    ///
+    /// This makes bindgen generate constants instead of enums. Regular
+    /// expressions are supported.
+    pub fn constified_enum<T: AsRef<str>>(mut self, arg: T) -> Builder {
+        self.options.constified_enums.insert(arg);
+        self
+    }
+
     /// Add a string to prepend to the generated bindings. The string is passed
     /// through without any modification.
     pub fn raw_line<T: Into<String>>(mut self, arg: T) -> Builder {
@@ -424,6 +440,9 @@ 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,
+
     /// Whether we should generate builtins or not.
     pub builtins: bool,
 
@@ -502,6 +521,7 @@ impl BindgenOptions {
         self.hidden_types.build();
         self.opaque_types.build();
         self.bitfield_enums.build();
+        self.constified_enums.build();
     }
 }
 
@@ -514,6 +534,7 @@ impl Default for BindgenOptions {
             whitelisted_functions: Default::default(),
             whitelisted_vars: Default::default(),
             bitfield_enums: Default::default(),
+            constified_enums: Default::default(),
             builtins: false,
             links: vec![],
             emit_ast: false,

src/options.rs

@@ -24,6 +24,14 @@ pub fn builder_from_flags<I>(args: I)
                 .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.")
+                .value_name("regex")
+                .takes_value(true)
+                .multiple(true)
+                .number_of_values(1),
             Arg::with_name("blacklist-type")
                 .long("blacklist-type")
                 .help("Mark a type as hidden.")
@@ -174,6 +182,12 @@ pub fn builder_from_flags<I>(args: I)
         }
     }
 
+    if let Some(bitfields) = matches.values_of("constified-enum") {
+        for regex in bitfields {
+            builder = builder.constified_enum(regex);
+        }
+    }
+
     if let Some(hidden_types) = matches.values_of("blacklist-type") {
         for ty in hidden_types {
             builder = builder.hide_type(ty);

tests/expectations/tests/bitfield-enum-basic.rs

@@ -18,7 +18,7 @@ impl ::std::ops::BitOr<Foo> for Foo {
 }
 #[repr(C)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub struct Foo(pub i32);
+pub struct Foo(pub ::std::os::raw::c_int);
 pub const Buz_Bar: Buz = Buz(2);
 pub const Buz_Baz: Buz = Buz(4);
 pub const Buz_Duplicated: Buz = Buz(4);
@@ -33,7 +33,7 @@ impl ::std::ops::BitOr<Buz> for Buz {
 }
 #[repr(C)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub struct Buz(pub i8);
+pub struct Buz(pub ::std::os::raw::c_char);
 pub const NS_FOO: _bindgen_ty_1 = _bindgen_ty_1(1);
 pub const NS_BAR: _bindgen_ty_1 = _bindgen_ty_1(2);
 impl ::std::ops::BitOr<_bindgen_ty_1> for _bindgen_ty_1 {
@@ -46,7 +46,7 @@ impl ::std::ops::BitOr<_bindgen_ty_1> for _bindgen_ty_1 {
 }
 #[repr(C)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub struct _bindgen_ty_1(pub u32);
+pub struct _bindgen_ty_1(pub ::std::os::raw::c_uint);
 #[repr(C)]
 #[derive(Debug, Copy)]
 pub struct Dummy {
@@ -66,7 +66,7 @@ impl ::std::ops::BitOr<Dummy__bindgen_ty_1> for Dummy__bindgen_ty_1 {
 }
 #[repr(C)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub struct Dummy__bindgen_ty_1(pub u32);
+pub struct Dummy__bindgen_ty_1(pub ::std::os::raw::c_uint);
 #[test]
 fn bindgen_test_layout_Dummy() {
     assert_eq!(::std::mem::size_of::<Dummy>() , 1usize);

tests/expectations/tests/constify-all-enums.rs

@@ -0,0 +1,23 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+pub const foo_THIS: foo = 0;
+pub const foo_SHOULD_BE: foo = 1;
+pub const foo_A_CONSTANT: foo = 2;
+pub type foo = ::std::os::raw::c_uint;
+#[repr(C)]
+#[derive(Debug, Copy)]
+pub struct bar {
+    pub this_should_work: foo,
+}
+#[test]
+fn bindgen_test_layout_bar() {
+    assert_eq!(::std::mem::size_of::<bar>() , 4usize);
+    assert_eq!(::std::mem::align_of::<bar>() , 4usize);
+}
+impl Clone for bar {
+    fn clone(&self) -> Self { *self }
+}

tests/headers/constify-all-enums.h

@@ -0,0 +1,11 @@
+// bindgen-flags: --constified-enum foo
+
+enum foo {
+  THIS,
+  SHOULD_BE,
+  A_CONSTANT,
+};
+
+struct bar {
+  enum foo this_should_work;
+};