Auto merge of rust-lang#184 - fitzgen:whitelisted-items, r=emilio

Factor out the collection and traversal of the transitive closure of whitelisted items

I'm going to re-use this stuff, plus I think it is way more understandable when pulled out of `codegen`.

This did have the unfortunate consequence of changing iteration order of types, resulting in different anonymous bindgen names for things like unions and such. We *could* sort the whitelisted item set before codegen to alleviate this kind of thing in the future, but I'm not sure it is worth the extra overhead...

r? @emilio

Author: bors-servo

src/codegen/mod.rs

@@ -14,7 +14,7 @@ use ir::item_kind::ItemKind;
 use ir::layout::Layout;
 use ir::module::Module;
 use ir::ty::{Type, TypeKind};
-use ir::type_collector::{ItemSet, TypeCollector};
+use ir::type_collector::ItemSet;
 use ir::var::Var;
 use self::helpers::{BlobTyBuilder, attributes};
 
@@ -1692,108 +1692,48 @@ impl CodeGenerator for Function {
     }
 }
 
+// Return true if any of the ancestors of `id` are in the whitelisted items set,
+// false otherwise.
+fn ancestor_is_whitelisted(ctx: &BindgenContext,
+                           whitelisted_items: &ItemSet,
+                           id: ItemId)
+                           -> bool {
+    let item = ctx.resolve_item(id);
+    let mut last = id;
+    let mut current = item.parent_id();
+
+    while last != current {
+        if whitelisted_items.contains(&current) {
+            return true;
+        }
+        last = current;
+        current = ctx.resolve_item(current).parent_id();
+    }
+
+    false
+}
+
 pub fn codegen(context: &mut BindgenContext) -> Vec<P<ast::Item>> {
     context.gen(|context| {
         let mut result = CodegenResult::new();
 
         debug!("codegen: {:?}", context.options());
 
-        // If the whitelisted types and functions sets are empty, just generate
-        // everything.
-        if context.options().whitelisted_types.is_empty() &&
-           context.options().whitelisted_functions.is_empty() &&
-           context.options().whitelisted_vars.is_empty() {
-            for (_item_id, item) in context.items() {
-                // Non-toplevel item parents are the responsible one for
-                // generating them.
-                if item.is_toplevel(context) {
-                    item.codegen(context, &mut result, &());
-                }
-            }
-        } else {
-            // Recursively collect all the types dependent on the whitelisted
-            // types, then generate them.
-            //
-            // FIXME(emilio): This pass is probably slow, but it can't be faster
-            // than docopt anyway :)
-            let mut items = ItemSet::new();
-            for (_item_id, item) in context.items() {
-                // FIXME(emilio): This probably should look only at whether the
-                // parent is a module.
-                if !item.is_toplevel(context) {
-                    continue;
-                }
-
-                let name = item.canonical_name(context);
-                match *item.kind() {
-                    ItemKind::Type(ref ty) => {
-                        if context.options().whitelisted_types.matches(&name) {
-                            item.collect_types(context, &mut items, &());
-                        }
-                        // Unnamed top-level enums are special and we whitelist
-                        // them via the whitelisted_vars filter, since they're
-                        // effectively top-level constants, and there's no way
-                        // for them to be referenced consistently.
-                        if let TypeKind::Enum(ref enum_) = *ty.kind() {
-                            if ty.name().is_none() {
-                                if enum_.variants().iter().any(|variant| {
-                                    context.options()
-                                        .whitelisted_vars
-                                        .matches(&variant.name())
-                                }) {
-                                    item.collect_types(context,
-                                                       &mut items,
-                                                       &());
-                                }
-                            }
-                        }
-                    }
-                    ItemKind::Function(ref fun) => {
-                        if context.options()
-                            .whitelisted_functions
-                            .matches(&name) {
-                            items.insert(item.id());
-                            fun.signature()
-                                .collect_types(context, &mut items, &());
-                        }
-                    }
-                    ItemKind::Var(ref var) => {
-                        if context.options().whitelisted_vars.matches(&name) {
-                            items.insert(item.id());
-                            var.ty().collect_types(context, &mut items, &());
-                        }
-                    }
-                    ItemKind::Module(..) => {}
-                }
-            }
-
-            fn contains_parent(ctx: &BindgenContext,
-                               types: &ItemSet,
-                               id: ItemId)
-                               -> bool {
-                let item = ctx.resolve_item(id);
-                let mut last = id;
-                let mut current = item.parent_id();
-
-                while last != current {
-                    if types.contains(&current) {
-                        return true;
-                    }
-                    last = current;
-                    current = ctx.resolve_item(current).parent_id();
-                }
+        let whitelisted_items: ItemSet = context.whitelisted_items().collect();
 
-                false
-            }
+        for &id in whitelisted_items.iter() {
+            let item = context.resolve_item(id);
 
-            for item_id in items.iter() {
-                let item = context.resolve_item(*item_id);
-                if item.is_toplevel(context) ||
-                   !contains_parent(context, &items, *item_id) {
-                    item.codegen(context, &mut result, &());
-                }
+            // Non-toplevel items' parents are responsible one for generating
+            // their children. However, if we find an orphaned reference to a
+            // non-toplevel item whose parent is not in our whitelisted set, we
+            // need to take responsibility for generating it.
+            if item.is_toplevel(context) ||
+               !ancestor_is_whitelisted(context, &whitelisted_items, id) {
+                item.codegen(context, &mut result, &());
             }
         }
+
         let saw_union = result.saw_union;
         let mut result = result.items;
         if saw_union && !context.options().unstable_rust {

src/ir/context.rs

@@ -12,6 +12,7 @@ use super::item::{Item, ItemCanonicalName, ItemId};
 use super::item_kind::ItemKind;
 use super::module::Module;
 use super::ty::{FloatKind, Type, TypeKind};
+use super::type_collector::{ItemSet, TypeCollector};
 use syntax::ast::Ident;
 use syntax::codemap::{DUMMY_SP, Span};
 use syntax::ext::base::ExtCtxt;
@@ -814,4 +815,119 @@ impl<'ctx> BindgenContext<'ctx> {
 
         self.current_module = previous_id;
     }
+
+    /// Iterate over all (explicitly or transitively) whitelisted items.
+    ///
+    /// If no items are explicitly whitelisted, then all items are considered
+    /// whitelisted.
+    pub fn whitelisted_items<'me>(&'me self)
+                                  -> WhitelistedItemsIter<'me, 'ctx> {
+        assert!(self.in_codegen_phase());
+        assert!(self.current_module == self.root_module);
+
+        let roots = self.items()
+            .filter(|&(_, item)| {
+                // If nothing is explicitly whitelisted, then everything is fair
+                // game.
+                if self.options().whitelisted_types.is_empty() &&
+                   self.options().whitelisted_functions.is_empty() &&
+                   self.options().whitelisted_vars.is_empty() {
+                    return true;
+                }
+
+                let name = item.canonical_name(self);
+                match *item.kind() {
+                    ItemKind::Module(..) => false,
+                    ItemKind::Function(_) => {
+                        self.options().whitelisted_functions.matches(&name)
+                    }
+                    ItemKind::Var(_) => {
+                        self.options().whitelisted_vars.matches(&name)
+                    }
+                    ItemKind::Type(ref ty) => {
+                        if self.options().whitelisted_types.matches(&name) {
+                            return true;
+                        }
+
+                        // Unnamed top-level enums are special and we whitelist
+                        // them via the `whitelisted_vars` filter, since they're
+                        // effectively top-level constants, and there's no way
+                        // for them to be referenced consistently.
+                        if let TypeKind::Enum(ref enum_) = *ty.kind() {
+                            if ty.name().is_none() &&
+                               enum_.variants().iter().any(|variant| {
+                                self.options()
+                                    .whitelisted_vars
+                                    .matches(&variant.name())
+                            }) {
+                                return true;
+                            }
+                        }
+
+                        false
+                    }
+                }
+            })
+            .map(|(&id, _)| id);
+
+        let seen: ItemSet = roots.collect();
+
+        // The .rev() preserves the expected ordering traversal, resulting in
+        // more stable-ish bindgen-generated names for anonymous types (like
+        // unions).
+        let to_iterate = seen.iter().cloned().rev().collect();
+
+        WhitelistedItemsIter {
+            ctx: self,
+            seen: seen,
+            to_iterate: to_iterate,
+        }
+    }
+}
+
+/// An iterator over whitelisted items.
+///
+/// See `BindgenContext::whitelisted_items` for more information.
+pub struct WhitelistedItemsIter<'ctx, 'gen>
+    where 'gen: 'ctx,
+{
+    ctx: &'ctx BindgenContext<'gen>,
+
+    // The set of whitelisted items we have seen. If you think of traversing
+    // whitelisted items like GC tracing, this is the mark bits, and contains
+    // both black and gray items.
+    seen: ItemSet,
+
+    // The set of whitelisted items that we have seen but have yet to iterate
+    // over and collect transitive references from. To return to the GC analogy,
+    // this is the mark stack, containing the set of gray items which we have
+    // not finished tracing yet.
+    to_iterate: Vec<ItemId>,
+}
+
+impl<'ctx, 'gen> Iterator for WhitelistedItemsIter<'ctx, 'gen>
+    where 'gen: 'ctx,
+{
+    type Item = ItemId;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let id = match self.to_iterate.pop() {
+            None => return None,
+            Some(id) => id,
+        };
+
+        debug_assert!(self.seen.contains(&id));
+
+        let mut sub_types = ItemSet::new();
+        id.collect_types(self.ctx, &mut sub_types, &());
+
+        for id in sub_types {
+            if !self.seen.contains(&id) {
+                self.to_iterate.push(id);
+                self.seen.insert(id);
+            }
+        }
+
+        Some(id)
+    }
 }

tests/expectations/type_alias_empty.rs

@@ -4,4 +4,4 @@
 #![allow(non_snake_case)]
 
 
-
+pub type bool_constant = ();

tests/tools/run-bindgen.py

@@ -69,6 +69,7 @@ def parse_args():
 def make_bindgen_env():
     """Build the environment to run bindgen in."""
     env = os.environ.copy()
+    env["RUST_BACKTRACE"] = "1"
 
     # El Capitan likes to unset dyld variables
     # https://forums.developer.apple.com/thread/9233