Skip to content

Generate bitfield accessor names eagerly #1058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/codegen/impl_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(", ");
Expand Down Expand Up @@ -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 ()
});
Expand Down
5 changes: 3 additions & 2 deletions src/codegen/impl_partialeq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
});
Expand Down
60 changes: 10 additions & 50 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 }
}

Expand All @@ -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,
Expand All @@ -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());
Expand Down
120 changes: 103 additions & 17 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -302,6 +313,8 @@ impl Bitfield {
Bitfield {
offset_into_unit: offset_into_unit,
data: raw.0,
getter_name: None,
setter_name: None,
}
}

Expand Down Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these comments :)

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 {
Expand Down Expand Up @@ -661,30 +698,79 @@ impl CompFields {
);
}

fn deanonymize_fields(&mut self) {
fn deanonymize_fields(&mut self, ctx: &BindgenContext, methods: &[Method]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assign_field_names is a slightly better name for this method now? I'm not married to either name...

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);
}
},
}
}
}
}
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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!");
Expand Down
Loading