Skip to content

ir: Track the codegen-reachable items, and use it instead of whitelisted_items() for code generation. #836

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 1 commit into from
Jul 21, 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
7 changes: 3 additions & 4 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3282,10 +3282,9 @@ pub fn codegen(context: &mut BindgenContext) -> Vec<P<ast::Item>> {

debug!("codegen: {:?}", context.options());

let whitelisted_items = context.whitelisted_items();

let codegen_items = context.codegen_items();
if context.options().emit_ir {
for &id in whitelisted_items {
for &id in codegen_items {
let item = context.resolve_item(id);
println!("ir: {:?} = {:#?}", id, item);
}
Expand All @@ -3299,7 +3298,7 @@ pub fn codegen(context: &mut BindgenContext) -> Vec<P<ast::Item>> {
}

context.resolve_item(context.root_module())
.codegen(context, &mut result, whitelisted_items, &());
.codegen(context, &mut result, codegen_items, &());
Copy link
Member

Choose a reason for hiding this comment

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

File a follow up E-Easy issue to remove this argument, and remove the whitelisted_items parameter from all the codegen functions using ctx.codegen_items() instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.


result.items
})
Expand Down
89 changes: 63 additions & 26 deletions src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,16 @@ pub struct BindgenContext<'ctx> {
/// Whether a bindgen complex was generated
generated_bindegen_complex: Cell<bool>,

/// The set of `ItemId`s that are whitelisted for code generation. This the
/// very first thing computed after parsing our IR, and before running any
/// of our analyses.
/// The set of `ItemId`s that are whitelisted. This the very first thing
/// computed after parsing our IR, and before running any of our analyses.
whitelisted: Option<ItemSet>,

/// The set of `ItemId`s that are whitelisted for code generation _and_ that
/// we should generate accounting for the codegen options.
///
/// It's computed right after computing the whitelisted items.
codegen_items: Option<ItemSet>,

/// Map from an item's id to the set of template parameter items that it
/// uses. See `ir::named` for more details. Always `Some` during the codegen
/// phase.
Expand All @@ -182,7 +187,7 @@ pub struct BindgenContext<'ctx> {
}

/// A traversal of whitelisted items.
pub struct WhitelistedItems<'ctx, 'gen>
struct WhitelistedItemsTraversal<'ctx, 'gen>
where 'gen: 'ctx
{
ctx: &'ctx BindgenContext<'gen>,
Expand All @@ -193,7 +198,7 @@ pub struct WhitelistedItems<'ctx, 'gen>
for<'a> fn(&'a BindgenContext, Edge) -> bool>,
}

impl<'ctx, 'gen> Iterator for WhitelistedItems<'ctx, 'gen>
impl<'ctx, 'gen> Iterator for WhitelistedItemsTraversal<'ctx, 'gen>
where 'gen: 'ctx
{
type Item = ItemId;
Expand All @@ -209,26 +214,23 @@ impl<'ctx, 'gen> Iterator for WhitelistedItems<'ctx, 'gen>
}
}

impl<'ctx, 'gen> WhitelistedItems<'ctx, 'gen>
impl<'ctx, 'gen> WhitelistedItemsTraversal<'ctx, 'gen>
where 'gen: 'ctx
{
/// Construct a new whitelisted items traversal.
pub fn new<R>(ctx: &'ctx BindgenContext<'gen>,
roots: R)
-> WhitelistedItems<'ctx, 'gen>
roots: R,
predicate: for<'a> fn(&'a BindgenContext, Edge) -> bool)
-> Self
where R: IntoIterator<Item = ItemId>,
{
let predicate = if ctx.options().whitelist_recursively {
traversal::all_edges
} else {
traversal::no_edges
};
WhitelistedItems {
WhitelistedItemsTraversal {
ctx: ctx,
traversal: ItemTraversal::new(ctx, roots, predicate)
}
}
}

impl<'ctx> BindgenContext<'ctx> {
/// Construct the context for the given `options`.
pub fn new(options: BindgenOptions) -> Self {
Expand Down Expand Up @@ -303,6 +305,7 @@ impl<'ctx> BindgenContext<'ctx> {
options: options,
generated_bindegen_complex: Cell::new(false),
whitelisted: None,
codegen_items: None,
used_template_parameters: None,
need_bitfield_allocation: Default::default(),
needs_mangling_hack: needs_mangling_hack,
Expand Down Expand Up @@ -770,7 +773,7 @@ impl<'ctx> BindgenContext<'ctx> {
// Compute the whitelisted set after processing replacements and
// resolving type refs, as those are the final mutations of the IR
// graph, and their completion means that the IR graph is now frozen.
self.compute_whitelisted_items();
self.compute_whitelisted_and_codegen_items();

// Make sure to do this after processing replacements, since that messes
// with the parentage and module children, and we want to assert that it
Expand Down Expand Up @@ -1589,15 +1592,22 @@ impl<'ctx> BindgenContext<'ctx> {
self.whitelisted.as_ref().unwrap()
}

/// Get a reference to the set of items we should generate.
pub fn codegen_items(&self) -> &ItemSet {
assert!(self.in_codegen_phase());
assert!(self.current_module == self.root_module);
self.codegen_items.as_ref().unwrap()
}

/// Compute the whitelisted items set and populate `self.whitelisted`.
fn compute_whitelisted_items(&mut self) {
fn compute_whitelisted_and_codegen_items(&mut self) {
assert!(self.in_codegen_phase());
assert!(self.current_module == self.root_module);
assert!(self.whitelisted.is_none());

self.whitelisted = Some({
let roots = self.items()
// Only consider items that are enabled for codegen.
let roots = {
let mut roots = self.items()
// Only consider roots that are enabled for codegen.
.filter(|&(_, item)| item.is_enabled_for_codegen(self))
.filter(|&(_, item)| {
// If nothing is explicitly whitelisted, then everything is fair
Expand Down Expand Up @@ -1657,16 +1667,43 @@ impl<'ctx> BindgenContext<'ctx> {
}
}
})
.map(|(&id, _)| id);
.map(|(&id, _)| id)
.collect::<Vec<_>>();

// The reversal preserves the expected ordering of traversal, resulting
// in more stable-ish bindgen-generated names for anonymous types (like
// unions).
let mut roots: Vec<_> = roots.collect();
// The reversal preserves the expected ordering of traversal,
// resulting in more stable-ish bindgen-generated names for
// anonymous types (like unions).
roots.reverse();
roots
};

WhitelistedItems::new(self, roots).collect()
});
let whitelisted_items_predicate =
if self.options().whitelist_recursively {
traversal::all_edges
} else {
traversal::no_edges
};

let whitelisted =
WhitelistedItemsTraversal::new(
self,
roots.clone(),
whitelisted_items_predicate,
).collect::<ItemSet>();

let codegen_items =
if self.options().whitelist_recursively {
WhitelistedItemsTraversal::new(
self,
roots.clone(),
traversal::codegen_edges,
).collect::<ItemSet>()
Copy link
Member

Choose a reason for hiding this comment

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

Mildly surprised the type inference couldn't figure this out.

} else {
whitelisted.clone()
};

self.whitelisted = Some(whitelisted);
self.codegen_items = Some(codegen_items);
}

/// Convenient method for getting the prefix to use for most traits in
Expand Down
28 changes: 28 additions & 0 deletions src/ir/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,34 @@ pub fn no_edges(_: &BindgenContext, _: Edge) -> bool {
false
}

/// A `TraversalPredicate` implementation that only follows edges to items that
/// are enabled for code generation. This lets us skip considering items for
/// which are not reachable from code generation.
pub fn codegen_edges(ctx: &BindgenContext, edge: Edge) -> bool {
let cc = &ctx.options().codegen_config;
match edge.kind {
EdgeKind::Generic => ctx.resolve_item(edge.to).is_enabled_for_codegen(ctx),

// We statically know the kind of item that non-generic edges can point
// to, so we don't need to actually resolve the item and check
// `Item::is_enabled_for_codegen`.
EdgeKind::TemplateParameterDefinition |
EdgeKind::TemplateArgument |
EdgeKind::TemplateDeclaration |
EdgeKind::BaseMember |
EdgeKind::Field |
EdgeKind::InnerType |
EdgeKind::FunctionReturn |
EdgeKind::FunctionParameter |
EdgeKind::VarType |
EdgeKind::TypeReference => cc.types,
EdgeKind::InnerVar => cc.vars,
EdgeKind::Method => cc.methods,
EdgeKind::Constructor => cc.constructors,
EdgeKind::Destructor => cc.destructors,
}
}

/// The storage for the set of items that have been seen (although their
/// outgoing edges might not have been fully traversed yet) in an active
/// traversal.
Expand Down
10 changes: 10 additions & 0 deletions tests/expectations/tests/issue-833-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

#[repr(C)] pub struct nsTArray { pub hdr: *const () }

extern "C" {
pub fn func() -> *mut nsTArray;
}
12 changes: 12 additions & 0 deletions tests/expectations/tests/issue-833-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

// If the output of this changes, please ensure issue-833-1.hpp changes too

#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct nsTArray {
pub _address: u8,
}
21 changes: 21 additions & 0 deletions tests/expectations/tests/issue-834.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]


#[repr(C)]
#[derive(Debug, Default, Copy)]
pub struct U {
pub _address: u8,
}
#[test]
fn bindgen_test_layout_U() {
assert_eq!(::std::mem::size_of::<U>() , 1usize , concat ! (
"Size of: " , stringify ! ( U ) ));
assert_eq! (::std::mem::align_of::<U>() , 1usize , concat ! (
"Alignment of " , stringify ! ( U ) ));
}
impl Clone for U {
fn clone(&self) -> Self { *self }
}
8 changes: 8 additions & 0 deletions tests/headers/issue-833-1.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// bindgen-flags: --generate functions --whitelist-function func --raw-line "#[repr(C)] pub struct nsTArray { pub hdr: *const () }"

template<typename T>
class nsTArray {
static T* sFoo;
};

extern "C" nsTArray<int>* func();
6 changes: 6 additions & 0 deletions tests/headers/issue-833-2.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// bindgen-flags: --raw-line "// If the output of this changes, please ensure issue-833-1.hpp changes too"

template<typename T>
class nsTArray {
static T* sFoo;
};
6 changes: 6 additions & 0 deletions tests/headers/issue-834.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// bindgen-flags: --whitelist-type U --generate types

struct T {};
struct U {
void test(T a);
};