[wip] Rework how bitfields are handled.

Author: emilio

src/codegen/mod.rs

@@ -2,7 +2,7 @@ mod helpers;
 mod struct_layout;
 
 use self::helpers::{BlobTyBuilder, attributes};
-use self::struct_layout::StructLayoutTracker;
+use self::struct_layout::{align_to, bytes_from_bits, StructLayoutTracker};
 use aster;
 
 use ir::annotations::FieldAccessorKind;
@@ -717,12 +717,12 @@ impl<'a> ItemToRustTy for Vtable<'a> {
 }
 
 struct Bitfield<'a> {
-    index: usize,
+    index: &'a mut usize,
     fields: Vec<&'a Field>,
 }
 
 impl<'a> Bitfield<'a> {
-    fn new(index: usize, fields: Vec<&'a Field>) -> Self {
+    fn new(index: &'a mut usize, fields: Vec<&'a Field>) -> Self {
         Bitfield {
             index: index,
             fields: fields,
@@ -732,89 +732,96 @@ impl<'a> Bitfield<'a> {
     fn codegen_fields(self,
                       ctx: &BindgenContext,
                       fields: &mut Vec<ast::StructField>,
-                      methods: &mut Vec<ast::ImplItem>)
+                      _methods: &mut Vec<ast::ImplItem>)
                       -> Layout {
         use aster::struct_field::StructFieldBuilder;
-        let mut total_width = self.fields
-            .iter()
-            .fold(0u32, |acc, f| acc + f.bitfield().unwrap());
-
-        if !total_width.is_power_of_two() || total_width < 8 {
-            total_width = cmp::max(8, total_width.next_power_of_two());
-        }
-        debug_assert_eq!(total_width % 8, 0);
-        let total_width_in_bytes = total_width as usize / 8;
-
-        let bitfield_layout = Layout::new(total_width_in_bytes,
-                                          total_width_in_bytes);
-        let bitfield_type = BlobTyBuilder::new(bitfield_layout).build();
-        let field_name = format!("_bitfield_{}", self.index);
-        let field_ident = ctx.ext_cx().ident_of(&field_name);
-        let field = StructFieldBuilder::named(&field_name)
-            .pub_()
-            .build_ty(bitfield_type.clone());
-        fields.push(field);
 
+        // NOTE: What follows is reverse-engineered from LLVM's
+        // lib/AST/RecordLayoutBuilder.cpp
+        //
+        // FIXME(emilio): There are some differences between Microsoft and the
+        // Itanium ABI, but we'll ignore those and stick to Itanium for now.
+        //
+        // Also, we need to handle packed bitfields and stuff.
+        // TODO(emilio): Take into account C++'s wide bitfields, and
+        // packing, sigh.
+        let mut total_size_in_bits = 0;
+        let mut max_align = 0;
+        let mut unfilled_bits_in_last_unit = 0;
+        let mut field_size_in_bits = 0;
+        *self.index += 1;
+        let mut last_field_name = format!("_bitfield_{}", self.index);
+        let mut last_field_align = 0;
 
-        let mut offset = 0;
         for field in self.fields {
             let width = field.bitfield().unwrap();
-            let field_name = field.name()
-                .map(ToOwned::to_owned)
-                .unwrap_or_else(|| format!("at_offset_{}", offset));
-
             let field_item = ctx.resolve_item(field.ty());
             let field_ty_layout = field_item.kind()
                 .expect_type()
                 .layout(ctx)
                 .expect("Bitfield without layout? Gah!");
 
-            let field_type = field_item.to_rust_ty(ctx);
-            let int_type = BlobTyBuilder::new(field_ty_layout).build();
+            let field_align = field_ty_layout.align;
 
-            let getter_name = ctx.rust_ident(&field_name);
-            let setter_name = ctx.ext_cx()
-                .ident_of(&format!("set_{}", &field_name));
-            let mask = ((1usize << width) - 1) << offset;
-            let prefix = ctx.trait_prefix();
-            // The transmute is unfortunate, but it's needed for enums in
-            // bitfields.
-            let item = quote_item!(ctx.ext_cx(),
-                impl X {
-                    #[inline]
-                    pub fn $getter_name(&self) -> $field_type {
-                        unsafe {
-                            ::$prefix::mem::transmute(
-                                (
-                                    (self.$field_ident &
-                                        ($mask as $bitfield_type))
-                                     >> $offset
-                                ) as $int_type
-                            )
-                        }
-                    }
+            if field_size_in_bits != 0 &&
+                (width == 0 || width as usize > unfilled_bits_in_last_unit) {
+                field_size_in_bits = align_to(field_size_in_bits, field_align);
+                // Push the new field.
+                let ty =
+                    BlobTyBuilder::new(Layout::new(bytes_from_bits(field_size_in_bits),
+                                                   bytes_from_bits(last_field_align)))
+                        .build();
 
-                    #[inline]
-                    pub fn $setter_name(&mut self, val: $field_type) {
-                        self.$field_ident &= !($mask as $bitfield_type);
-                        self.$field_ident |=
-                            (val as $int_type as $bitfield_type << $offset) &
-                                ($mask as $bitfield_type);
-                    }
-                }
-            )
-                .unwrap();
+                let field = StructFieldBuilder::named(&last_field_name)
+                    .pub_()
+                    .build_ty(ty);
+                fields.push(field);
 
-            let items = match item.unwrap().node {
-                ast::ItemKind::Impl(_, _, _, _, _, items) => items,
-                _ => unreachable!(),
-            };
+                // TODO(emilio): dedup this.
+                *self.index += 1;
+                last_field_name = format!("_bitfield_{}", self.index);
 
-            methods.extend(items.into_iter());
-            offset += width;
+                // Now reset the size and the rest of stuff.
+                // unfilled_bits_in_last_unit = 0;
+                field_size_in_bits = 0;
+                last_field_align = 0;
+            }
+
+            // TODO(emilio): Create the accessors. Problem here is that we still
+            // don't know which one is going to be the final alignment of the
+            // bitfield, and whether we have to index in it. Thus, we don't know
+            // which integer type do we need.
+            //
+            // We could push them to a Vec or something, but given how buggy
+            // they where maybe it's not a great idea?
+            field_size_in_bits += width as usize;
+            total_size_in_bits += width as usize;
+
+
+            let data_size = align_to(field_size_in_bits, field_align * 8);
+
+            max_align = cmp::max(max_align, field_align);
+
+            // NB: The width here is completely, absolutely intentional.
+            last_field_align = cmp::max(last_field_align, width as usize);
+
+            unfilled_bits_in_last_unit = data_size - field_size_in_bits;
         }
 
-        bitfield_layout
+        if field_size_in_bits != 0 {
+            // Push the last field.
+            let ty =
+                BlobTyBuilder::new(Layout::new(bytes_from_bits(field_size_in_bits),
+                                               bytes_from_bits(last_field_align)))
+                    .build();
+
+            let field = StructFieldBuilder::named(&last_field_name)
+                .pub_()
+                .build_ty(ty);
+            fields.push(field);
+        }
+
+        Layout::new(bytes_from_bits(total_size_in_bits), max_align)
     }
 }
 
@@ -1062,12 +1069,10 @@ impl CodeGenerator for CompInfo {
                 debug_assert!(!current_bitfield_fields.is_empty());
                 let bitfield_fields =
                     mem::replace(&mut current_bitfield_fields, vec![]);
-                bitfield_count += 1;
-                let bitfield_layout = Bitfield::new(bitfield_count,
+                let bitfield_layout = Bitfield::new(&mut bitfield_count,
                                                     bitfield_fields)
                     .codegen_fields(ctx, &mut fields, &mut methods);
-
-                struct_layout.saw_bitfield(bitfield_layout);
+                struct_layout.saw_bitfield_batch(bitfield_layout);
 
                 current_bitfield_width = None;
                 current_bitfield_layout = None;
@@ -1099,8 +1104,7 @@ impl CodeGenerator for CompInfo {
                 } else {
                     quote_ty!(ctx.ext_cx(), __BindgenUnionField<$ty>)
                 }
-            } else if let Some(item) =
-                field_ty.is_incomplete_array(ctx) {
+            } else if let Some(item) = field_ty.is_incomplete_array(ctx) {
                 result.saw_incomplete_array();
 
                 let inner = item.to_rust_ty(ctx);
@@ -1224,12 +1228,10 @@ impl CodeGenerator for CompInfo {
             debug_assert!(!current_bitfield_fields.is_empty());
             let bitfield_fields = mem::replace(&mut current_bitfield_fields,
                                                vec![]);
-            bitfield_count += 1;
-            let bitfield_layout = Bitfield::new(bitfield_count,
+            let bitfield_layout = Bitfield::new(&mut bitfield_count,
                                                 bitfield_fields)
                 .codegen_fields(ctx, &mut fields, &mut methods);
-
-            struct_layout.saw_bitfield(bitfield_layout);
+            struct_layout.saw_bitfield_batch(bitfield_layout);
         }
         debug_assert!(current_bitfield_fields.is_empty());
 

src/codegen/struct_layout.rs

@@ -23,6 +23,60 @@ pub struct StructLayoutTracker<'a, 'ctx: 'a> {
     max_field_align: usize,
 }
 
+/// Returns a size aligned to a given value.
+pub fn align_to(size: usize, align: usize) -> usize {
+    if align == 0 {
+        return size;
+    }
+
+    let rem = size % align;
+    if rem == 0 {
+        return size;
+    }
+
+    size + align - rem
+}
+
+/// Returns the lower power of two byte count that can hold at most n bits.
+pub fn bytes_from_bits(mut n: usize) -> usize {
+    if n == 0 {
+        return 0;
+    }
+
+    if n <= 8 {
+        return 1;
+    }
+
+    if !n.is_power_of_two() {
+        n = n.next_power_of_two();
+    }
+
+    n / 8
+}
+
+#[test]
+fn test_align_to() {
+    assert_eq!(align_to(1, 1), 1);
+    assert_eq!(align_to(1, 2), 2);
+    assert_eq!(align_to(1, 4), 4);
+    assert_eq!(align_to(5, 1), 5);
+    assert_eq!(align_to(17, 4), 20);
+}
+
+#[test]
+fn test_bytes_from_bits() {
+    assert_eq!(bytes_from_bits(0), 0);
+    for i in 1..9 {
+        assert_eq!(bytes_from_bits(i), 1);
+    }
+    for i in 9..17 {
+        assert_eq!(bytes_from_bits(i), 2);
+    }
+    for i in 17..33 {
+        assert_eq!(bytes_from_bits(i), 4);
+    }
+}
+
 impl<'a, 'ctx> StructLayoutTracker<'a, 'ctx> {
     pub fn new(ctx: &'a BindgenContext<'ctx>, comp: &'a CompInfo) -> Self {
         StructLayoutTracker {
@@ -51,13 +105,14 @@ impl<'a, 'ctx> StructLayoutTracker<'a, 'ctx> {
             self.max_field_align = cmp::max(self.max_field_align, layout.align);
         }
     }
-
-    pub fn saw_bitfield(&mut self, layout: Layout) {
+    pub fn saw_bitfield_batch(&mut self, layout: Layout) {
         self.align_to_latest_field();
 
         self.latest_offset += self.padding_bytes(layout) + layout.size;
         self.latest_field_layout = Some(layout);
-        self.max_field_align = cmp::max(self.max_field_align, layout.align);
+        // NB: We intentionally don't update the max_field_align here, since our
+        // bitfields code doesn't necessarily guarantee it, so we need to
+        // actually generate the dummy alignment.
     }
 
     pub fn saw_union(&mut self, layout: Layout) {
@@ -166,15 +221,7 @@ impl<'a, 'ctx> StructLayoutTracker<'a, 'ctx> {
     }
 
     fn padding_bytes(&self, layout: Layout) -> usize {
-        if layout.align == 0 {
-            warn!("try to padding bytes without layout");
-
-            0
-        } else if self.latest_offset % layout.align == 0 {
-            0
-        } else {
-            layout.align - (self.latest_offset % layout.align)
-        }
+        align_to(self.latest_offset, layout.align) - self.latest_offset
     }
 
     fn padding_field(&mut self, layout: Layout) -> ast::StructField {

src/ir/ty.rs

@@ -383,6 +383,9 @@ impl Type {
 
     /// If this type has a known size, return it (in bytes).
     pub fn calc_size(&self, ctx: &BindgenContext) -> Option<usize> {
+        if let Some(layout) = self.layout {
+            return Some(layout.size);
+        }
         match self.kind {
             TypeKind::Comp(ref ci) => {
                 ci.calc_layout(ctx).map(|layout| layout.size)