Ensure that every item is in some module's children list

Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769

Author: fitzgen

src/codegen/mod.rs

@@ -435,6 +435,16 @@ impl CodeGenerator for Var {
         }
         result.saw_var(&canonical_name);
 
+        // We can't generate bindings to static variables of templates. The
+        // number of actual variables for a single declaration are open ended
+        // and we don't know what instantiations do or don't exist.
+        let type_params = item.all_template_params(ctx);
+        if let Some(params) = type_params {
+            if !params.is_empty() {
+                return;
+            }
+        }
+
         let ty = self.ty().to_rust_ty_or_opaque(ctx, &());
 
         if let Some(val) = self.val() {
@@ -752,15 +762,25 @@ impl CodeGenerator for TemplateInstantiation {
             return
         }
 
+        // If there are any unbound type parameters, then we can't generate a
+        // layout test because we aren't dealing with a concrete type with a
+        // concrete size and alignment.
+        if ctx.uses_any_template_parameters(item.id()) {
+            return;
+        }
+
         let layout = item.kind().expect_type().layout(ctx);
 
         if let Some(layout) = layout {
             let size = layout.size;
             let align = layout.align;
 
             let name = item.canonical_name(ctx);
-            let fn_name = format!("__bindgen_test_layout_{}_instantiation_{}",
-                                  name, item.exposed_id(ctx));
+            let mut fn_name = format!("__bindgen_test_layout_{}_instantiation", name);
+            let times_seen = result.overload_number(&fn_name);
+            if times_seen > 0 {
+                write!(&mut fn_name, "_{}", times_seen).unwrap();
+            }
 
             let fn_name = ctx.rust_ident_raw(&fn_name);
 
@@ -2920,6 +2940,17 @@ impl CodeGenerator for Function {
                    item: &Item) {
         debug!("<Function as CodeGenerator>::codegen: item = {:?}", item);
 
+        // Similar to static member variables in a class template, we can't
+        // generate bindings to template functions, because the set of
+        // instantiations is open ended and we have no way of knowing which
+        // monomorphizations actually exist.
+        let type_params = item.all_template_params(ctx);
+        if let Some(params) = type_params {
+            if !params.is_empty() {
+                return;
+            }
+        }
+
         let name = self.name();
         let mut canonical_name = item.canonical_name(ctx);
         let mangled_name = self.mangled_name();

src/ir/context.rs

@@ -2,7 +2,7 @@
 
 use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
 use super::int::IntKind;
-use super::item::{Item, ItemCanonicalPath, ItemSet};
+use super::item::{Item, ItemAncestors, ItemCanonicalPath, ItemSet};
 use super::item_kind::ItemKind;
 use super::module::{Module, ModuleKind};
 use super::named::{UsedTemplateParameters, analyze};
@@ -348,14 +348,8 @@ impl<'ctx> BindgenContext<'ctx> {
         let is_template_instantiation =
             is_type && item.expect_type().is_template_instantiation();
 
-        // Be sure to track all the generated children under namespace, even
-        // those generated after resolving typerefs, etc.
-        if item.id() != item.parent_id() {
-            if let Some(mut parent) = self.items.get_mut(&item.parent_id()) {
-                if let Some(mut module) = parent.as_module_mut() {
-                    module.children_mut().push(item.id());
-                }
-            }
+        if item.id() != self.root_module {
+            self.add_item_to_module(&item);
         }
 
         if is_type && item.expect_type().is_comp() {
@@ -407,6 +401,38 @@ impl<'ctx> BindgenContext<'ctx> {
         }
     }
 
+    /// Ensure that every item (other than the root module) is in a module's
+    /// children list. This is to make sure that every whitelisted item get's
+    /// codegen'd, even if its parent is not whitelisted. See issue #769 for
+    /// details.
+    fn add_item_to_module(&mut self, item: &Item) {
+        assert!(item.id() != self.root_module);
+        assert!(!self.items.contains_key(&item.id()));
+
+        if let Some(mut parent) = self.items.get_mut(&item.parent_id()) {
+            if let Some(mut module) = parent.as_module_mut() {
+                debug!("add_item_to_module: adding {:?} as child of parent module {:?}",
+                       item.id(),
+                       item.parent_id());
+
+                module.children_mut().insert(item.id());
+                return;
+            }
+        }
+
+        debug!("add_item_to_module: adding {:?} as child of current module {:?}",
+               item.id(),
+               self.current_module);
+
+        self.items
+            .get_mut(&self.current_module)
+            .expect("Should always have an item for self.current_module")
+            .as_module_mut()
+            .expect("self.current_module should always be a module")
+            .children_mut()
+            .insert(item.id());
+    }
+
     /// Add a new named template type parameter to this context's item set.
     pub fn add_named_type(&mut self, item: Item, definition: clang::Cursor) {
         debug!("BindgenContext::add_named_type: item = {:?}; definition = {:?}",
@@ -418,6 +444,8 @@ impl<'ctx> BindgenContext<'ctx> {
         assert_eq!(definition.kind(),
                    clang_sys::CXCursor_TemplateTypeParameter);
 
+        self.add_item_to_module(&item);
+
         let id = item.id();
         let old_item = self.items.insert(id, item);
         assert!(old_item.is_none(),
@@ -620,41 +648,65 @@ impl<'ctx> BindgenContext<'ctx> {
                 item.parent_id()
             };
 
+            // Relocate the replacement item from where it was declared, to
+            // where the thing it is replacing was declared.
+            //
+            // First, we'll make sure that its parent id is correct.
 
-            // Reparent the item.
             let old_parent = self.resolve_item(replacement).parent_id();
-
             if new_parent == old_parent {
+                // Same parent and therefore also same containing
+                // module. Nothing to do here.
                 continue;
             }
 
-            if let Some(mut module) = self.items
-                .get_mut(&old_parent)
+            self.items
+                .get_mut(&replacement)
                 .unwrap()
-                .as_module_mut() {
-                // Deparent the replacement.
-                let position = module.children()
-                    .iter()
-                    .position(|id| *id == replacement)
-                    .unwrap();
-                module.children_mut().remove(position);
-            }
+                .set_parent_for_replacement(new_parent);
 
-            if let Some(mut module) = self.items
-                .get_mut(&new_parent)
-                .unwrap()
-                .as_module_mut() {
-                module.children_mut().push(replacement);
+            // Second, make sure that it is in the correct module's children
+            // set.
+
+            let old_module = {
+                let immut_self = &*self;
+                old_parent.ancestors(immut_self)
+                    .chain(Some(immut_self.root_module))
+                    .find(|id| {
+                        let item = immut_self.resolve_item(*id);
+                        item.as_module().map_or(false, |m| m.children().contains(&replacement))
+                    })
+            };
+            let old_module = old_module.expect("Every replacement item should be in a module");
+
+            let new_module = {
+                let immut_self = &*self;
+                new_parent.ancestors(immut_self).find(|id| {
+                    immut_self.resolve_item(*id).is_module()
+                })
+            };
+            let new_module = new_module.unwrap_or(self.root_module);
+
+            if new_module == old_module {
+                // Already in the correct module.
+                continue;
             }
 
             self.items
-                .get_mut(&replacement)
+                .get_mut(&old_module)
                 .unwrap()
-                .set_parent_for_replacement(new_parent);
+                .as_module_mut()
+                .unwrap()
+                .children_mut()
+                .remove(&replacement);
+
             self.items
-                .get_mut(&id)
+                .get_mut(&new_module)
+                .unwrap()
+                .as_module_mut()
                 .unwrap()
-                .set_parent_for_replacement(old_parent);
+                .children_mut()
+                .insert(replacement);
         }
     }
 
@@ -783,6 +835,21 @@ impl<'ctx> BindgenContext<'ctx> {
             .map_or(false, |items_used_params| items_used_params.contains(&template_param))
     }
 
+    /// Return `true` if `item` uses any unbound, generic template parameters,
+    /// `false` otherwise.
+    ///
+    /// Has the same restrictions that `uses_template_parameter` has.
+    pub fn uses_any_template_parameters(&self, item: ItemId) -> bool {
+        assert!(self.in_codegen_phase(),
+                "We only compute template parameter usage as we enter codegen");
+
+        self.used_template_parameters
+            .as_ref()
+            .expect("should have template parameter usage info in codegen phase")
+            .get(&item)
+            .map_or(false, |used| !used.is_empty())
+    }
+
     // This deserves a comment. Builtin types don't get a valid declaration, so
     // we can't add it to the cursor->type map.
     //
@@ -794,6 +861,7 @@ impl<'ctx> BindgenContext<'ctx> {
     fn add_builtin_item(&mut self, item: Item) {
         debug!("add_builtin_item: item = {:?}", item);
         debug_assert!(item.kind().is_type());
+        self.add_item_to_module(&item);
         let id = item.id();
         let old_item = self.items.insert(id, item);
         assert!(old_item.is_none(), "Inserted type twice?");
@@ -932,7 +1000,6 @@ impl<'ctx> BindgenContext<'ctx> {
     fn instantiate_template(&mut self,
                             with_id: ItemId,
                             template: ItemId,
-                            parent_id: ItemId,
                             ty: &clang::Type,
                             location: clang::Cursor)
                             -> Option<ItemId> {
@@ -1038,13 +1105,14 @@ impl<'ctx> BindgenContext<'ctx> {
                         let sub_item = Item::new(sub_id,
                                                  None,
                                                  None,
-                                                 template_decl_id,
+                                                 self.current_module,
                                                  ItemKind::Type(sub_ty));
 
                         // Bypass all the validations in add_item explicitly.
                         debug!("instantiate_template: inserting nested \
                                 instantiation item: {:?}",
                                sub_item);
+                        self.add_item_to_module(&sub_item);
                         debug_assert!(sub_id == sub_item.id());
                         self.items.insert(sub_id, sub_item);
                         args.push(sub_id);
@@ -1086,10 +1154,11 @@ impl<'ctx> BindgenContext<'ctx> {
                            type_kind,
                            ty.is_const());
         let item =
-            Item::new(with_id, None, None, parent_id, ItemKind::Type(ty));
+            Item::new(with_id, None, None, self.current_module, ItemKind::Type(ty));
 
         // Bypass all the validations in add_item explicitly.
         debug!("instantiate_template: inserting item: {:?}", item);
+        self.add_item_to_module(&item);
         debug_assert!(with_id == item.id());
         self.items.insert(with_id, item);
         Some(with_id)
@@ -1143,11 +1212,6 @@ impl<'ctx> BindgenContext<'ctx> {
                    location.is_some() {
                     let location = location.unwrap();
 
-                    // It is always safe to hang instantiations off of the root
-                    // module. They use their template definition for naming,
-                    // and don't need the parent for anything else.
-                    let parent_id = self.root_module();
-
                     // For specialized type aliases, there's no way to get the
                     // template parameters as of this writing (for a struct
                     // specialization we wouldn't be in this branch anyway).
@@ -1166,7 +1230,6 @@ impl<'ctx> BindgenContext<'ctx> {
 
                     return self.instantiate_template(with_id,
                                                      id,
-                                                     parent_id,
                                                      ty,
                                                      location)
                         .or_else(|| Some(id));

src/ir/module.rs

@@ -1,7 +1,8 @@
 //! Intermediate representation for modules (AKA C++ namespaces).
 
-use super::context::{BindgenContext, ItemId};
+use super::context::BindgenContext;
 use super::dot::DotAttributes;
+use super::item::ItemSet;
 use clang;
 use parse::{ClangSubItemParser, ParseError, ParseResult};
 use parse_one;
@@ -24,7 +25,7 @@ pub struct Module {
     /// The kind of module this is.
     kind: ModuleKind,
     /// The children of this module, just here for convenience.
-    children_ids: Vec<ItemId>,
+    children: ItemSet,
 }
 
 impl Module {
@@ -33,7 +34,7 @@ impl Module {
         Module {
             name: name,
             kind: kind,
-            children_ids: vec![],
+            children: ItemSet::new(),
         }
     }
 
@@ -43,13 +44,13 @@ impl Module {
     }
 
     /// Get a mutable reference to this module's children.
-    pub fn children_mut(&mut self) -> &mut Vec<ItemId> {
-        &mut self.children_ids
+    pub fn children_mut(&mut self) -> &mut ItemSet {
+        &mut self.children
     }
 
     /// Get this module's children.
-    pub fn children(&self) -> &[ItemId] {
-        &self.children_ids
+    pub fn children(&self) -> &ItemSet {
+        &self.children
     }
 
     /// Whether this namespace is inline.

tests/expectations/tests/anon_union.rs

@@ -80,7 +80,7 @@ impl Default for ErrorResult {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[test]
-fn __bindgen_test_layout_TErrorResult_instantiation_1() {
+fn __bindgen_test_layout_TErrorResult_instantiation() {
     assert_eq!(::std::mem::size_of::<TErrorResult>() , 24usize , concat ! (
                "Size of template specialization: " , stringify ! (
                TErrorResult ) ));

tests/expectations/tests/class_nested.rs

@@ -78,7 +78,7 @@ extern "C" {
     pub static mut var: A_B;
 }
 #[test]
-fn __bindgen_test_layout_A_D_instantiation_1() {
+fn __bindgen_test_layout_A_D_instantiation() {
     assert_eq!(::std::mem::size_of::<A_D<::std::os::raw::c_int>>() , 4usize ,
                concat ! (
                "Size of template specialization: " , stringify ! (

tests/expectations/tests/class_with_dtor.rs

@@ -35,7 +35,7 @@ impl Default for WithoutDtor {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[test]
-fn __bindgen_test_layout_HandleWithDtor_instantiation_1() {
+fn __bindgen_test_layout_HandleWithDtor_instantiation() {
     assert_eq!(::std::mem::size_of::<HandleWithDtor<::std::os::raw::c_int>>()
                , 8usize , concat ! (
                "Size of template specialization: " , stringify ! (