Skip to content

Commit 67ff630

Browse files
fitzgenfitzgen
committed
Refactor IR graph traversal infrastructure
This makes the IR traversal infrastructure generic. It makes it so we can use the same traversal code for whitelisting traversals and asserting no dangling item references. Therefore the queue of items to visit is generic (whitelisting uses DFS, while asserting against dangling uses BFS), the storage for the seen set (whitelisting uses a simple set, asserting against dangling uses a map from item to the item from which it was discovered). It also introduces the concept of different kinds of edges in the IR graph, and the ability to choose which edges to follow. This means we can simplify non-transitive whitelisting to a simple function that always returns "no do not follow this edge". It plays an important part for future analysis of which template declaration type parameters are used or not.
1 parent a0bb2ce commit 67ff630

File tree

6 files changed

+390
-190
lines changed

6 files changed

+390
-190
lines changed

‎src/ir/comp.rs‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
use super::annotations::Annotations;
44
use super::context::{BindgenContext, ItemId};
55
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
6-
use super::item::{Item, ItemSet};
6+
use super::item::Item;
77
use super::layout::Layout;
8+
use super::traversal::{EdgeKind, Trace, Tracer};
89
use super::ty::{TemplateDeclaration, Type};
910
use super::traversal::Trace;
1011
use clang;
@@ -1078,41 +1079,55 @@ impl<'a> CanDeriveCopy<'a> for CompInfo {
10781079
impl Trace for CompInfo {
10791080
type Extra = Item;
10801081

1081-
fn trace(&self,
1082-
context: &BindgenContext,
1083-
types: &mut ItemSet,
1084-
item: &Item) {
1082+
fn trace<T>(&self,
1083+
context: &BindgenContext,
1084+
tracer: &mut T,
1085+
item: &Item)
1086+
where T: Tracer
1087+
{
1088+
// TODO: We should properly distinguish template instantiations from
1089+
// template declarations at the type level. Why are some template
1090+
// instantiations represented here instead of as
1091+
// TypeKind::TemplateInstantiation?
10851092
if let Some(template) = self.specialized_template() {
1086-
types.insert(template);
1087-
}
1088-
1089-
let applicable_template_args = item.applicable_template_args(context);
1090-
for arg in applicable_template_args {
1091-
types.insert(arg);
1093+
// This is an instantiation of a template declaration with concrete
1094+
// template type arguments.
1095+
tracer.visit(template);
1096+
let args = item.applicable_template_args(context);
1097+
for a in args {
1098+
tracer.visit(a);
1099+
}
1100+
} else {
1101+
let params = item.applicable_template_args(context);
1102+
// This is a template declaration with abstract template type
1103+
// parameters.
1104+
for p in params {
1105+
tracer.visit_kind(p, EdgeKind::TemplateParameterDefinition);
1106+
}
10921107
}
10931108

10941109
for base in self.base_members() {
1095-
types.insert(base.ty);
1110+
tracer.visit(base.ty);
10961111
}
10971112

10981113
for field in self.fields() {
1099-
types.insert(field.ty());
1114+
tracer.visit(field.ty());
11001115
}
11011116

11021117
for &ty in self.inner_types() {
1103-
types.insert(ty);
1118+
tracer.visit(ty);
11041119
}
11051120

11061121
for &var in self.inner_vars() {
1107-
types.insert(var);
1122+
tracer.visit(var);
11081123
}
11091124

11101125
for method in self.methods() {
1111-
types.insert(method.signature);
1126+
tracer.visit(method.signature);
11121127
}
11131128

11141129
for &ctor in self.constructors() {
1115-
types.insert(ctor);
1130+
tracer.visit(ctor);
11161131
}
11171132
}
11181133
}

‎src/ir/context.rs‎

Lines changed: 23 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use super::int::IntKind;
55
use super::item::{Item, ItemSet, ItemCanonicalPath};
66
use super::item_kind::ItemKind;
77
use super::module::{Module, ModuleKind};
8-
use super::traversal::{ItemTraversal, Trace};
8+
use super::traversal::{self, Edge, ItemTraversal};
99
use super::ty::{FloatKind, TemplateDeclaration, Type, TypeKind};
1010
use BindgenOptions;
1111
use cexpr;
@@ -15,7 +15,7 @@ use clang_sys;
1515
use parse::ClangItemParser;
1616
use std::borrow::Cow;
1717
use std::cell::Cell;
18-
use std::collections::{HashMap, VecDeque, hash_map};
18+
use std::collections::{HashMap, hash_map};
1919
use std::collections::btree_map::{self, BTreeMap};
2020
use std::fmt;
2121
use std::iter::IntoIterator;
@@ -151,6 +151,10 @@ pub struct BindgenContext<'ctx> {
151151
generated_bindegen_complex: Cell<bool>,
152152
}
153153

154+
/// A traversal of whitelisted items.
155+
pub type WhitelistedItems<'ctx, 'gen> =
156+
ItemTraversal<'ctx, 'gen, ItemSet, Vec<ItemId>, fn(Edge) -> bool>;
157+
154158
impl<'ctx> BindgenContext<'ctx> {
155159
/// Construct the context for the given `options`.
156160
pub fn new(options: BindgenOptions) -> Self {
@@ -538,23 +542,12 @@ impl<'ctx> BindgenContext<'ctx> {
538542

539543
fn assert_no_dangling_item_traversal<'me>
540544
(&'me self)
541-
-> AssertNoDanglingItemIter<'me, 'ctx> {
545+
-> traversal::AssertNoDanglingItemsTraversal<'me, 'ctx> {
542546
assert!(self.in_codegen_phase());
543547
assert!(self.current_module == self.root_module);
544548

545-
let mut roots = self.items().map(|(&id, _)| id);
546-
547-
let mut seen = BTreeMap::<ItemId, ItemId>::new();
548-
let next_child = roots.next().map(|id| id).unwrap();
549-
seen.insert(next_child, next_child);
550-
551-
let to_iterate = seen.iter().map(|(&id, _)| id).rev().collect();
552-
553-
AssertNoDanglingItemIter {
554-
ctx: self,
555-
seen: seen,
556-
to_iterate: to_iterate,
557-
}
549+
let roots = self.items().map(|(&id, _)| id);
550+
traversal::AssertNoDanglingItemsTraversal::new(self, roots, traversal::all_edges)
558551
}
559552

560553
// This deserves a comment. Builtin types don't get a valid declaration, so
@@ -1202,8 +1195,7 @@ impl<'ctx> BindgenContext<'ctx> {
12021195
///
12031196
/// If no items are explicitly whitelisted, then all items are considered
12041197
/// whitelisted.
1205-
pub fn whitelisted_items<'me>(&'me self)
1206-
-> ItemTraversal<'me, 'ctx> {
1198+
pub fn whitelisted_items<'me>(&'me self) -> WhitelistedItems<'me, 'ctx> {
12071199
assert!(self.in_codegen_phase());
12081200
assert!(self.current_module == self.root_module);
12091201

@@ -1268,7 +1260,19 @@ impl<'ctx> BindgenContext<'ctx> {
12681260
})
12691261
.map(|(&id, _)| id);
12701262

1271-
ItemTraversal::new(self, roots)
1263+
// The reversal preserves the expected ordering of traversal, resulting
1264+
// in more stable-ish bindgen-generated names for anonymous types (like
1265+
// unions).
1266+
let mut roots: Vec<_> = roots.collect();
1267+
roots.reverse();
1268+
1269+
let predicate = if self.options().whitelist_recursively {
1270+
traversal::all_edges
1271+
} else {
1272+
traversal::no_edges
1273+
};
1274+
1275+
WhitelistedItems::new(self, roots, predicate)
12721276
}
12731277

12741278
/// Convenient method for getting the prefix to use for most traits in
@@ -1353,75 +1357,3 @@ impl TemplateDeclaration for PartialType {
13531357
}
13541358
}
13551359
}
1356-
1357-
/// An iterator to find any dangling items.
1358-
///
1359-
/// See `BindgenContext::assert_no_dangling_item_traversal` for more
1360-
/// information.
1361-
pub struct AssertNoDanglingItemIter<'ctx, 'gen>
1362-
where 'gen: 'ctx,
1363-
{
1364-
ctx: &'ctx BindgenContext<'gen>,
1365-
seen: BTreeMap<ItemId, ItemId>,
1366-
to_iterate: VecDeque<ItemId>,
1367-
}
1368-
1369-
impl<'ctx, 'gen> Iterator for AssertNoDanglingItemIter<'ctx, 'gen>
1370-
where 'gen: 'ctx,
1371-
{
1372-
type Item = ItemId;
1373-
1374-
fn next(&mut self) -> Option<Self::Item> {
1375-
let id = match self.to_iterate.pop_front() {
1376-
None => {
1377-
// We've traversed everything reachable from the previous
1378-
// root(s), see if we have any more roots.
1379-
match self.ctx
1380-
.items()
1381-
.filter(|&(id, _)| !self.seen.contains_key(id))
1382-
.next()
1383-
.map(|(id, _)| *id) {
1384-
None => return None,
1385-
Some(id) => {
1386-
// This is a new root.
1387-
self.seen.insert(id, id);
1388-
id
1389-
}
1390-
}
1391-
}
1392-
Some(id) => id,
1393-
};
1394-
1395-
let mut sub_types = ItemSet::new();
1396-
id.trace(self.ctx, &mut sub_types, &());
1397-
1398-
if self.ctx.resolve_item_fallible(id).is_none() {
1399-
let mut path = vec![];
1400-
let mut current = id;
1401-
loop {
1402-
let predecessor = *self.seen
1403-
.get(&current)
1404-
.expect("We know we found this item id, so it must have a \
1405-
predecessor");
1406-
if predecessor == current {
1407-
break;
1408-
}
1409-
path.push(predecessor);
1410-
current = predecessor;
1411-
}
1412-
path.reverse();
1413-
panic!("Found reference to dangling id = {:?}\nvia path = {:?}",
1414-
id,
1415-
path);
1416-
}
1417-
1418-
for sub_id in sub_types {
1419-
if self.seen.insert(sub_id, id).is_none() {
1420-
// We've never visited this sub item before.
1421-
self.to_iterate.push_back(sub_id);
1422-
}
1423-
}
1424-
1425-
Some(id)
1426-
}
1427-
}

‎src/ir/function.rs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
//! Intermediate representation for C/C++ functions and methods.
22
33
use super::context::{BindgenContext, ItemId};
4-
use super::item::{Item, ItemSet};
4+
use super::item::Item;
5+
use super::traversal::{Trace, Tracer};
56
use super::ty::TypeKind;
67
use super::traversal::Trace;
78
use clang;
@@ -317,16 +318,18 @@ impl ClangSubItemParser for Function {
317318
}
318319

319320
impl Trace for FunctionSig {
320-
type Extra = Item;
321+
type Extra = ();
321322

322-
fn trace(&self,
323-
_context: &BindgenContext,
324-
types: &mut ItemSet,
325-
_item: &Item) {
326-
types.insert(self.return_type());
323+
fn trace<T>(&self,
324+
_: &BindgenContext,
325+
tracer: &mut T,
326+
_: &())
327+
where T: Tracer
328+
{
329+
tracer.visit(self.return_type());
327330

328331
for &(_, ty) in self.argument_types() {
329-
types.insert(ty);
332+
tracer.visit(ty);
330333
}
331334
}
332335
}

‎src/ir/item.rs‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
66
use super::function::Function;
77
use super::item_kind::ItemKind;
88
use super::module::Module;
9+
use super::traversal::{Trace, Tracer};
910
use super::ty::{TemplateDeclaration, Type, TypeKind};
1011
use super::traversal::Trace;
1112
use clang;
@@ -170,22 +171,20 @@ impl ItemAncestors for Item {
170171
impl Trace for ItemId {
171172
type Extra = ();
172173

173-
fn trace(&self,
174-
ctx: &BindgenContext,
175-
types: &mut ItemSet,
176-
extra: &()) {
177-
ctx.resolve_item(*self).trace(ctx, types, extra);
174+
fn trace<T>(&self, ctx: &BindgenContext, tracer: &mut T, extra: &())
175+
where T: Tracer
176+
{
177+
ctx.resolve_item(*self).trace(ctx, tracer, extra);
178178
}
179179
}
180180

181181
impl Trace for Item {
182182
type Extra = ();
183183

184-
fn trace(&self,
185-
ctx: &BindgenContext,
186-
types: &mut ItemSet,
187-
_extra: &()) {
188-
if self.is_hidden(ctx) || types.contains(&self.id()) {
184+
fn trace<T>(&self, ctx: &BindgenContext, tracer: &mut T, _extra: &())
185+
where T: Tracer
186+
{
187+
if self.is_hidden(ctx) {
189188
return;
190189
}
191190

@@ -196,22 +195,25 @@ impl Trace for Item {
196195
// opaque.
197196
if ty.should_be_traced_unconditionally() ||
198197
!self.is_opaque(ctx) {
199-
ty.trace(ctx, types, self);
198+
ty.trace(ctx, tracer, self);
200199
}
201200
}
202201
ItemKind::Function(ref fun) => {
203202
// Just the same way, it has not real meaning for a function to
204203
// be opaque, so we trace across it.
205-
types.insert(fun.signature());
204+
tracer.visit(fun.signature());
206205
}
207206
ItemKind::Var(ref var) => {
208-
types.insert(var.ty());
207+
tracer.visit(var.ty());
209208
}
210209
ItemKind::Module(_) => {
211210
// Module -> children edges are "weak", and we do not want to
212211
// trace them. If we did, then whitelisting wouldn't work as
213212
// expected: everything in every module would end up
214213
// whitelisted.
214+
//
215+
// TODO: make a new edge kind for module -> children edges and
216+
// filter them during whitelisting traversals.
215217
}
216218
}
217219
}
@@ -923,10 +925,11 @@ impl TemplateDeclaration for ItemKind {
923925
fn template_params(&self, ctx: &BindgenContext) -> Option<Vec<ItemId>> {
924926
match *self {
925927
ItemKind::Type(ref ty) => ty.template_params(ctx),
926-
// TODO FITZGEN: shouldn't functions be able to have free template
927-
// params?
928-
ItemKind::Module(_) |
928+
// If we start emitting bindings to explicitly instantiated
929+
// functions, then we'll need to check ItemKind::Function for
930+
// template params.
929931
ItemKind::Function(_) |
932+
ItemKind::Module(_) |
930933
ItemKind::Var(_) => None,
931934
}
932935
}

0 commit comments

Comments
 (0)