Skip to content

Commit

Permalink
Auto merge of #1059 - aeleos:master, r=fitzgen
Browse files Browse the repository at this point in the history
Remove early anonymous bitfield filtering and consolidate name method

This PR is some changes to early bitfield filtering to help fix (#1007) This does not close (#1007), but allows for checking if the bitfield is too large during a later stage.

@fitzgen r?
  • Loading branch information
bors-servo authored Oct 4, 2017
2 parents 4e74c39 + a1ee87f commit 2930a85
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 27 deletions.
13 changes: 8 additions & 5 deletions src/codegen/impl_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,14 @@ impl<'a> ImplDebug<'a> for BitfieldUnit {
if i > 0 {
format_string.push_str(", ");
}
format_string.push_str(&format!("{} : {{:?}}", bu.name()));
let name_ident = ctx.rust_ident_raw(bu.name());
tokens.push(quote! {
self.#name_ident ()
});

if let Some(name) = bu.name() {
format_string.push_str(&format!("{} : {{:?}}", name));
let name_ident = ctx.rust_ident_raw(name);
tokens.push(quote! {
self.#name_ident ()
});
}
}

Some((format_string, tokens))
Expand Down
10 changes: 6 additions & 4 deletions src/codegen/impl_partialeq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ pub fn gen_partialeq_impl(
tokens.push(gen_field(ctx, ty_item, name));
}
Field::Bitfields(ref bu) => for bitfield in bu.bitfields() {
let name_ident = ctx.rust_ident_raw(bitfield.name());
tokens.push(quote! {
self.#name_ident () == other.#name_ident ()
});
if let Some(name) = bitfield.name() {
let name_ident = ctx.rust_ident_raw(name);
tokens.push(quote! {
self.#name_ident () == other.#name_ident ()
});
}
},
}
}
Expand Down
12 changes: 9 additions & 3 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,10 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
let mut ctor_impl = quote! { 0 };

for bf in self.bitfields() {
// Codegen not allowed for anonymous bitfields
if bf.name().is_none() {
continue;
}
bf.codegen(
ctx,
fields_should_be_private,
Expand All @@ -1198,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());
let param_name = bitfield_getter_name(ctx, parent, bf.name().unwrap());
let bitfield_ty_item = ctx.resolve_item(bf.ty());
let bitfield_ty = bitfield_ty_item.expect_type();
let bitfield_ty =
Expand Down Expand Up @@ -1307,9 +1311,11 @@ 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());
let setter_name = bitfield_setter_name(ctx, parent, self.name());
let getter_name = bitfield_getter_name(ctx, parent, self.name().unwrap());
let setter_name = bitfield_setter_name(ctx, parent, self.name().unwrap());
let unit_field_ident = quote::Ident::new(unit_field_name);

let bitfield_ty_item = ctx.resolve_item(self.ty());
Expand Down
21 changes: 7 additions & 14 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl DotAttributes for Bitfield {
writeln!(
out,
"<tr><td>{} : {}</td><td>{:?}</td></tr>",
self.name(),
self.name().unwrap_or("(anonymous)"),
self.width(),
self.ty()
)
Expand All @@ -298,7 +298,6 @@ impl Bitfield {
/// Construct a new bitfield.
fn new(offset_into_unit: usize, raw: RawField) -> Bitfield {
assert!(raw.bitfield().is_some());
assert!(raw.name().is_some());

Bitfield {
offset_into_unit: offset_into_unit,
Expand Down Expand Up @@ -332,11 +331,6 @@ impl Bitfield {
pub fn width(&self) -> u32 {
self.data.bitfield().unwrap()
}

/// Get the name of this bitfield.
pub fn name(&self) -> &str {
self.data.name().unwrap()
}
}

impl FieldMethods for Bitfield {
Expand Down Expand Up @@ -581,13 +575,12 @@ fn bitfields_to_allocation_units<E, I>(
}
}

// Only keep named bitfields around. Unnamed bitfields (with > 0
// bitsize) are used for padding. Because the `Bitfield` struct stores
// the bit-offset into its allocation unit where its bits begin, we
// don't need any padding bits hereafter.
if bitfield.name().is_some() {
bitfields_in_unit.push(Bitfield::new(offset, bitfield));
}
// Always keep all bitfields around. While unnamed bitifields are used
// for padding (and usually not needed hereafter), large unnamed
// bitfields over their types size cause weird allocation size behavior from clang.
// Therefore, all bitfields needed to be kept around in order to check for this
// and make the struct opaque in this case
bitfields_in_unit.push(Bitfield::new(offset, bitfield));

max_align = cmp::max(max_align, bitfield_align);

Expand Down
1 change: 0 additions & 1 deletion tests/expectations/tests/bitfield_large_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,3 @@ extern "C" {
#[link_name = "a"]
pub static mut a: _bindgen_ty_1;
}

0 comments on commit 2930a85

Please sign in to comment.