Auto merge of #1058 - pepyakin:bitfield-accessors, r=fitzgen

Generate bitfield accessor names eagerly

@fitzgen r?

I'm not sure about `deanonymize_fields`, as we now assign names to data members and also generate names for the bitfield accessors, but can't come up with a better name.

Author: bors-servo

src/codegen/impl_debug.rs

@@ -28,7 +28,6 @@ pub fn gen_debug_impl(
                     &Field::Bitfields(ref bu) => bu.impl_debug(ctx, ()),
                 });
 
-
                 for (i, (fstring, toks)) in processed_fields.enumerate() {
                     if i > 0 {
                         format_string.push_str(", ");
@@ -91,14 +90,15 @@ impl<'a> ImplDebug<'a> for BitfieldUnit {
     ) -> Option<(String, Vec<quote::Tokens>)> {
         let mut format_string = String::new();
         let mut tokens = vec![];
-        for (i, bu) in self.bitfields().iter().enumerate() {
+        for (i, bitfield) in self.bitfields().iter().enumerate() {
             if i > 0 {
                 format_string.push_str(", ");
             }
 
-            if let Some(name) = bu.name() {
-                format_string.push_str(&format!("{} : {{:?}}", name));
-                let name_ident = ctx.rust_ident_raw(name);
+            if let Some(bitfield_name) = bitfield.name() {
+                format_string.push_str(&format!("{} : {{:?}}", bitfield_name));
+                let getter_name = bitfield.getter_name();
+                let name_ident = ctx.rust_ident_raw(getter_name);
                 tokens.push(quote! {
                     self.#name_ident ()
                 });

src/codegen/impl_partialeq.rs

@@ -51,8 +51,9 @@ pub fn gen_partialeq_impl(
                     tokens.push(gen_field(ctx, ty_item, name));
                 }
                 Field::Bitfields(ref bu) => for bitfield in bu.bitfields() {
-                    if let Some(name) = bitfield.name() {
-                        let name_ident = ctx.rust_ident_raw(name);
+                    if let Some(_) = bitfield.name() {
+                        let getter_name = bitfield.getter_name();
+                        let name_ident = ctx.rust_ident_raw(getter_name);
                         tokens.push(quote! {
                             self.#name_ident () == other.#name_ident ()
                         });

src/codegen/mod.rs

@@ -1202,7 +1202,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
                 (&unit_field_name, unit_field_int_ty.clone()),
             );
 
-            let param_name = bitfield_getter_name(ctx, parent, bf.name().unwrap());
+            let param_name = bitfield_getter_name(ctx, bf);
             let bitfield_ty_item = ctx.resolve_item(bf.ty());
             let bitfield_ty = bitfield_ty_item.expect_type();
             let bitfield_ty =
@@ -1236,59 +1236,21 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
     }
 }
 
-fn parent_has_method(
-    ctx: &BindgenContext,
-    parent: &CompInfo,
-    name: &str,
-) -> bool {
-    parent.methods().iter().any(|method| {
-        let method_name = match *ctx.resolve_item(method.signature()).kind() {
-            ItemKind::Function(ref func) => func.name(),
-            ref otherwise => {
-                panic!(
-                    "a method's signature should always be a \
-                 item of kind ItemKind::Function, found: \
-                 {:?}",
-                    otherwise
-                )
-            }
-        };
-
-        method_name == name || ctx.rust_mangle(&method_name) == name
-    })
-}
-
 fn bitfield_getter_name(
     ctx: &BindgenContext,
-    parent: &CompInfo,
-    bitfield_name: &str,
+    bitfield: &Bitfield,
 ) -> quote::Tokens {
-    let name = ctx.rust_mangle(bitfield_name);
-
-    if parent_has_method(ctx, parent, &name) {
-        let mut name = name.to_string();
-        name.push_str("_bindgen_bitfield");
-        let name = ctx.rust_ident(name);
-        return quote! { #name };
-    }
-
-    let name = ctx.rust_ident(name);
+    let name = bitfield.getter_name();
+    let name = ctx.rust_ident_raw(name);
     quote! { #name }
 }
 
 fn bitfield_setter_name(
     ctx: &BindgenContext,
-    parent: &CompInfo,
-    bitfield_name: &str,
+    bitfield: &Bitfield,
 ) -> quote::Tokens {
-    let setter = format!("set_{}", bitfield_name);
-    let mut setter = ctx.rust_mangle(&setter).to_string();
-
-    if parent_has_method(ctx, parent, &setter) {
-        setter.push_str("_bindgen_bitfield");
-    }
-
-    let setter = ctx.rust_ident(setter);
+    let setter = bitfield.setter_name();
+    let setter = ctx.rust_ident_raw(setter);
     quote! { #setter }
 }
 
@@ -1301,7 +1263,7 @@ impl<'a> FieldCodegen<'a> for Bitfield {
         _fields_should_be_private: bool,
         _codegen_depth: usize,
         _accessor_kind: FieldAccessorKind,
-        parent: &CompInfo,
+        _parent: &CompInfo,
         _result: &mut CodegenResult,
         _struct_layout: &mut StructLayoutTracker,
         _fields: &mut F,
@@ -1311,11 +1273,9 @@ impl<'a> FieldCodegen<'a> for Bitfield {
         F: Extend<quote::Tokens>,
         M: Extend<quote::Tokens>,
     {
-        // Should never be called with name() as None, as codegen can't be done
-        // on an anonymous bitfield
         let prefix = ctx.trait_prefix();
-        let getter_name = bitfield_getter_name(ctx, parent, self.name().unwrap());
-        let setter_name = bitfield_setter_name(ctx, parent, self.name().unwrap());
+        let getter_name = bitfield_getter_name(ctx, self);
+        let setter_name = bitfield_setter_name(ctx, self);
         let unit_field_ident = quote::Ident::new(unit_field_name);
 
         let bitfield_ty_item = ctx.resolve_item(self.ty());

src/ir/comp.rs

@@ -17,6 +17,7 @@ use peeking_take_while::PeekableExt;
 use std::cmp;
 use std::io;
 use std::mem;
+use std::collections::HashMap;
 
 /// The kind of compound type.
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -292,6 +293,16 @@ pub struct Bitfield {
 
     /// The field data for this bitfield.
     data: FieldData,
+
+    /// Name of the generated Rust getter for this bitfield.
+    /// 
+    /// Should be assigned before codegen.
+    getter_name: Option<String>,
+
+    /// Name of the generated Rust setter for this bitfield.
+    /// 
+    /// Should be assigned before codegen.
+    setter_name: Option<String>,
 }
 
 impl Bitfield {
@@ -302,6 +313,8 @@ impl Bitfield {
         Bitfield {
             offset_into_unit: offset_into_unit,
             data: raw.0,
+            getter_name: None,
+            setter_name: None,
         }
     }
 
@@ -331,6 +344,30 @@ impl Bitfield {
     pub fn width(&self) -> u32 {
         self.data.bitfield().unwrap()
     }
+
+    /// Name of the generated Rust getter for this bitfield.
+    /// 
+    /// Panics if called before assigning bitfield accessor names or if 
+    /// this bitfield have no name.
+    pub fn getter_name(&self) -> &str {
+        assert!(self.name().is_some(), "`Bitfield::getter_name` called on anonymous field");
+        self.getter_name.as_ref().expect(
+            "`Bitfield::getter_name` should only be called after\
+             assigning bitfield accessor names",
+        )
+    }
+
+    /// Name of the generated Rust setter for this bitfield.
+    /// 
+    /// Panics if called before assigning bitfield accessor names or if 
+    /// this bitfield have no name.
+    pub fn setter_name(&self) -> &str {
+        assert!(self.name().is_some(), "`Bitfield::setter_name` called on anonymous field");
+        self.setter_name.as_ref().expect(
+            "`Bitfield::setter_name` should only be called\
+             after assigning bitfield accessor names",
+        )
+    }
 }
 
 impl FieldMethods for Bitfield {
@@ -661,30 +698,79 @@ impl CompFields {
         );
     }
 
-    fn deanonymize_fields(&mut self) {
+    fn deanonymize_fields(&mut self, ctx: &BindgenContext, methods: &[Method]) {
         let fields = match *self {
-            CompFields::AfterComputingBitfieldUnits(ref mut fields) => {
-                fields
-            }
+            CompFields::AfterComputingBitfieldUnits(ref mut fields) => fields,
             CompFields::BeforeComputingBitfieldUnits(_) => {
                 panic!("Not yet computed bitfield units.");
             }
         };
 
+        fn has_method(methods: &[Method], ctx: &BindgenContext, name: &str) -> bool {
+            methods.iter().any(|method| {
+                let method_name = ctx.resolve_func(method.signature()).name();
+                method_name == name || ctx.rust_mangle(&method_name) == name
+            })
+        }
+
+        struct AccessorNamesPair {
+            getter: String,
+            setter: String,
+        }
+
+        let mut accessor_names: HashMap<String, AccessorNamesPair> = fields
+            .iter()
+            .flat_map(|field| match *field {
+                Field::Bitfields(ref bu) => &*bu.bitfields,
+                Field::DataMember(_) => &[],
+            })
+            .filter_map(|bitfield| bitfield.name())
+            .map(|bitfield_name| {
+                let bitfield_name = bitfield_name.to_string();
+                let getter = {
+                    let mut getter = ctx.rust_mangle(&bitfield_name).to_string();
+                    if has_method(methods, ctx, &getter) {
+                        getter.push_str("_bindgen_bitfield");
+                    }
+                    getter
+                };
+                let setter = {
+                    let setter = format!("set_{}", bitfield_name);
+                    let mut setter = ctx.rust_mangle(&setter).to_string();
+                    if has_method(methods, ctx, &setter) {
+                        setter.push_str("_bindgen_bitfield");
+                    }
+                    setter
+                };
+                (bitfield_name, AccessorNamesPair { getter, setter })
+            })
+            .collect();
+
         let mut anon_field_counter = 0;
         for field in fields.iter_mut() {
-            let field_data = match *field {
-                Field::DataMember(ref mut fd) => fd,
-                Field::Bitfields(_) => continue,
-            };
+            match *field {
+                Field::DataMember(FieldData { ref mut name, .. }) => {
+                    if let Some(_) = *name {
+                        continue;
+                    }
 
-            if let Some(_) = field_data.name  {
-                continue;
-            }
+                    anon_field_counter += 1;
+                    let generated_name = format!("__bindgen_anon_{}", anon_field_counter);
+                    *name = Some(generated_name);
+                }
+                Field::Bitfields(ref mut bu) => for bitfield in &mut bu.bitfields {
+                    if bitfield.name().is_none() {
+                        continue;
+                    }
 
-            anon_field_counter += 1;
-            let name = format!("__bindgen_anon_{}", anon_field_counter);
-            field_data.name = Some(name);
+                    if let Some(AccessorNamesPair { getter, setter }) =
+                        accessor_names.remove(bitfield.name().unwrap())
+                    {
+                        bitfield.getter_name = Some(getter);
+                        bitfield.setter_name = Some(setter);
+                    }
+                },
+            }
         }
     }
 }
@@ -1397,8 +1483,8 @@ impl CompInfo {
     }
 
     /// Assign for each anonymous field a generated name.
-    pub fn deanonymize_fields(&mut self) {
-        self.fields.deanonymize_fields();
+    pub fn deanonymize_fields(&mut self, ctx: &BindgenContext) {
+        self.fields.deanonymize_fields(ctx, &self.methods);
     }
 
     /// Returns whether the current union can be represented as a Rust `union`
@@ -1484,7 +1570,7 @@ impl IsOpaque for CompInfo {
                 false
             },
             Field::Bitfields(ref unit) => {
-                    unit.bitfields().iter().any(|bf| {
+                unit.bitfields().iter().any(|bf| {
                     let bitfield_layout = ctx.resolve_type(bf.ty())
                         .layout(ctx)
                         .expect("Bitfield without layout? Gah!");