Allow marking specific template instantiations as opaque

Cargo.toml

@@ -13,7 +13,7 @@ name = "bindgen"
 readme = "README.md"
 repository = "https://github.com/servo/rust-bindgen"
 documentation = "https://docs.rs/bindgen"
-version = "0.26.1"
+version = "0.26.2"
 build = "build.rs"
 
 exclude = [

src/codegen/mod.rs

@@ -17,8 +17,8 @@ use ir::dot;
 use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue};
 use ir::function::{Abi, Function, FunctionSig};
 use ir::int::IntKind;
-use ir::item::{Item, ItemAncestors, ItemCanonicalName, ItemCanonicalPath,
-               ItemSet};
+use ir::item::{IsOpaque, Item, ItemAncestors, ItemCanonicalName,
+               ItemCanonicalPath, ItemSet};
 use ir::item_kind::ItemKind;
 use ir::layout::Layout;
 use ir::module::Module;
@@ -580,7 +580,7 @@ impl CodeGenerator for Type {
                 }
 
                 let mut used_template_params = item.used_template_params(ctx);
-                let inner_rust_type = if item.is_opaque(ctx) {
+                let inner_rust_type = if item.is_opaque(ctx, &()) {
                     used_template_params = None;
                     self.to_opaque(ctx, item)
                 } else {
@@ -759,8 +759,11 @@ impl CodeGenerator for TemplateInstantiation {
                    item: &Item) {
         // Although uses of instantiations don't need code generation, and are
         // just converted to rust types in fields, vars, etc, we take this
-        // opportunity to generate tests for their layout here.
-        if !ctx.options().layout_tests {
+        // opportunity to generate tests for their layout here. If the
+        // instantiation is opaque, then its presumably because we don't
+        // properly understand it (maybe because of specializations), and so we
+        // shouldn't emit layout tests either.
+        if !ctx.options().layout_tests || self.is_opaque(ctx, item) {
             return
         }
 
@@ -1568,7 +1571,7 @@ impl CodeGenerator for CompInfo {
         }
 
         // Yeah, sorry about that.
-        if item.is_opaque(ctx) {
+        if item.is_opaque(ctx, &()) {
             fields.clear();
             methods.clear();
 
@@ -1704,7 +1707,7 @@ impl CodeGenerator for CompInfo {
                         })
                         .count() > 1;
 
-                    let should_skip_field_offset_checks = item.is_opaque(ctx) ||
+                    let should_skip_field_offset_checks = item.is_opaque(ctx, &()) ||
                                                           too_many_base_vtables;
 
                     let check_field_offset = if should_skip_field_offset_checks {
@@ -2816,7 +2819,7 @@ impl TryToRustTy for Type {
                     .build())
             }
             TypeKind::TemplateInstantiation(ref inst) => {
-                inst.try_to_rust_ty(ctx, self)
+                inst.try_to_rust_ty(ctx, item)
             }
             TypeKind::ResolvedTypeRef(inner) => inner.try_to_rust_ty(ctx, &()),
             TypeKind::TemplateAlias(inner, _) |
@@ -2828,7 +2831,7 @@ impl TryToRustTy for Type {
                     .collect::<Vec<_>>();
 
                 let spelling = self.name().expect("Unnamed alias?");
-                if item.is_opaque(ctx) && !template_params.is_empty() {
+                if item.is_opaque(ctx, &()) && !template_params.is_empty() {
                     self.try_to_opaque(ctx, item)
                 } else if let Some(ty) = utils::type_from_named(ctx,
                                                                 spelling,
@@ -2841,7 +2844,7 @@ impl TryToRustTy for Type {
             TypeKind::Comp(ref info) => {
                 let template_params = item.used_template_params(ctx);
                 if info.has_non_type_template_params() ||
-                   (item.is_opaque(ctx) && template_params.is_some()) {
+                   (item.is_opaque(ctx, &()) && template_params.is_some()) {
                     return self.try_to_opaque(ctx, item);
                 }
 
@@ -2895,23 +2898,27 @@ impl TryToRustTy for Type {
 }
 
 impl TryToOpaque for TemplateInstantiation {
-    type Extra = Type;
+    type Extra = Item;
 
     fn try_get_layout(&self,
                       ctx: &BindgenContext,
-                      self_ty: &Type)
+                      item: &Item)
                       -> error::Result<Layout> {
-        self_ty.layout(ctx).ok_or(error::Error::NoLayoutForOpaqueBlob)
+        item.expect_type().layout(ctx).ok_or(error::Error::NoLayoutForOpaqueBlob)
     }
 }
 
 impl TryToRustTy for TemplateInstantiation {
-    type Extra = Type;
+    type Extra = Item;
 
     fn try_to_rust_ty(&self,
                       ctx: &BindgenContext,
-                      _: &Type)
+                      item: &Item)
                       -> error::Result<P<ast::Ty>> {
+        if self.is_opaque(ctx, item) {
+            return Err(error::Error::InstantiationOfOpaqueType);
+        }
+
         let decl = self.template_definition();
         let mut ty = decl.try_to_rust_ty(ctx, &())?.unwrap();
 
@@ -2924,7 +2931,7 @@ impl TryToRustTy for TemplateInstantiation {
                 extra_assert!(decl.into_resolver()
                                   .through_type_refs()
                                   .resolve(ctx)
-                                  .is_opaque(ctx));
+                                  .is_opaque(ctx, &()));
                 return Err(error::Error::InstantiationOfOpaqueType);
             }
         };

src/ir/comp.rs

@@ -4,7 +4,7 @@ use super::annotations::Annotations;
 use super::context::{BindgenContext, ItemId};
 use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
 use super::dot::DotAttributes;
-use super::item::Item;
+use super::item::{IsOpaque, Item};
 use super::layout::Layout;
 use super::traversal::{EdgeKind, Trace, Tracer};
 use super::template::TemplateParameters;
@@ -1585,7 +1585,7 @@ impl Trace for CompInfo {
         // We unconditionally trace `CompInfo`'s template parameters and inner
         // types for the the usage analysis. However, we don't want to continue
         // tracing anything else, if this type is marked opaque.
-        if item.is_opaque(context) {
+        if item.is_opaque(context, &()) {
             return;
         }
 

src/ir/context.rs

@@ -2,7 +2,7 @@
 
 use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault};
 use super::int::IntKind;
-use super::item::{Item, ItemAncestors, ItemCanonicalPath, ItemSet};
+use super::item::{IsOpaque, Item, ItemAncestors, ItemCanonicalPath, ItemSet};
 use super::item_kind::ItemKind;
 use super::module::{Module, ModuleKind};
 use super::named::{UsedTemplateParameters, analyze};
@@ -339,7 +339,7 @@ impl<'ctx> BindgenContext<'ctx> {
                location);
         debug_assert!(declaration.is_some() || !item.kind().is_type() ||
                       item.kind().expect_type().is_builtin_or_named() ||
-                      item.kind().expect_type().is_opaque(),
+                      item.kind().expect_type().is_opaque(self, &item),
                       "Adding a type without declaration?");
 
         let id = item.id();

src/ir/item.rs

@@ -61,6 +61,17 @@ pub trait ItemCanonicalPath {
     fn canonical_path(&self, ctx: &BindgenContext) -> Vec<String>;
 }
 
+/// A trait for determining if some IR thing is opaque or not.
+pub trait IsOpaque {
+    /// Extra context the IR thing needs to determine if it is opaque or not.
+    type Extra;
+
+    /// Returns `true` if the thing is opaque, and `false` otherwise.
+    ///
+    /// May only be called when `ctx` is in the codegen phase.
+    fn is_opaque(&self, ctx: &BindgenContext, extra: &Self::Extra) -> bool;
+}
+
 /// A trait for iterating over an item and its parents and up its ancestor chain
 /// up to (but not including) the implicit root module.
 pub trait ItemAncestors {
@@ -231,7 +242,7 @@ impl Trace for Item {
                 // don't want to stop collecting types even though they may be
                 // opaque.
                 if ty.should_be_traced_unconditionally() ||
-                   !self.is_opaque(ctx) {
+                   !self.is_opaque(ctx, &()) {
                     ty.trace(ctx, tracer, self);
                 }
             }
@@ -269,7 +280,7 @@ impl CanDeriveDebug for Item {
         let result = ctx.options().derive_debug &&
                      match self.kind {
             ItemKind::Type(ref ty) => {
-                if self.is_opaque(ctx) {
+                if self.is_opaque(ctx, &()) {
                     ty.layout(ctx)
                         .map_or(true, |l| l.opaque().can_derive_debug(ctx, ()))
                 } else {
@@ -292,7 +303,7 @@ impl CanDeriveDefault for Item {
         ctx.options().derive_default &&
         match self.kind {
             ItemKind::Type(ref ty) => {
-                if self.is_opaque(ctx) {
+                if self.is_opaque(ctx, &()) {
                     ty.layout(ctx)
                         .map_or(false,
                                 |l| l.opaque().can_derive_default(ctx, ()))
@@ -317,7 +328,7 @@ impl<'a> CanDeriveCopy<'a> for Item {
 
         let result = match self.kind {
             ItemKind::Type(ref ty) => {
-                if self.is_opaque(ctx) {
+                if self.is_opaque(ctx, &()) {
                     ty.layout(ctx)
                         .map_or(true, |l| l.opaque().can_derive_copy(ctx, ()))
                 } else {
@@ -335,7 +346,7 @@ impl<'a> CanDeriveCopy<'a> for Item {
     fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool {
         match self.kind {
             ItemKind::Type(ref ty) => {
-                if self.is_opaque(ctx) {
+                if self.is_opaque(ctx, &()) {
                     ty.layout(ctx)
                         .map_or(true, |l| {
                             l.opaque().can_derive_copy_in_array(ctx, ())
@@ -593,15 +604,6 @@ impl Item {
         ctx.hidden_by_name(&self.canonical_path(ctx), self.id)
     }
 
-    /// Is this item opaque?
-    pub fn is_opaque(&self, ctx: &BindgenContext) -> bool {
-        debug_assert!(ctx.in_codegen_phase(),
-                      "You're not supposed to call this yet");
-        self.annotations.opaque() ||
-        self.as_type().map_or(false, |ty| ty.is_opaque()) ||
-        ctx.opaque_by_name(&self.canonical_path(ctx))
-    }
-
     /// Is this a reference to another type?
     pub fn is_type_ref(&self) -> bool {
         self.as_type().map_or(false, |ty| ty.is_type_ref())
@@ -858,6 +860,28 @@ impl Item {
     }
 }
 
+impl IsOpaque for ItemId {
+    type Extra = ();
+
+    fn is_opaque(&self, ctx: &BindgenContext, _: &()) -> bool {
+        debug_assert!(ctx.in_codegen_phase(),
+                      "You're not supposed to call this yet");
+        ctx.resolve_item(*self).is_opaque(ctx, &())
+    }
+}
+
+impl IsOpaque for Item {
+    type Extra = ();
+
+    fn is_opaque(&self, ctx: &BindgenContext, _: &()) -> bool {
+        debug_assert!(ctx.in_codegen_phase(),
+                      "You're not supposed to call this yet");
+        self.annotations.opaque() ||
+            self.as_type().map_or(false, |ty| ty.is_opaque(ctx, self)) ||
+            ctx.opaque_by_name(&self.canonical_path(ctx))
+    }
+}
+
 /// A set of items.
 pub type ItemSet = BTreeSet<ItemId>;
 
@@ -874,7 +898,7 @@ impl DotAttributes for Item {
                       self.id,
                       self.name(ctx).get()));
 
-        if self.is_opaque(ctx) {
+        if self.is_opaque(ctx, &()) {
             writeln!(out, "<tr><td>opaque</td><td>true</td></tr>")?;
         }
 

src/ir/template.rs

@@ -29,7 +29,7 @@
 
 use super::context::{BindgenContext, ItemId};
 use super::derive::{CanDeriveCopy, CanDeriveDebug};
-use super::item::{Item, ItemAncestors};
+use super::item::{IsOpaque, Item, ItemAncestors, ItemCanonicalPath};
 use super::layout::Layout;
 use super::traversal::{EdgeKind, Trace, Tracer};
 use clang;
@@ -305,6 +305,40 @@ impl TemplateInstantiation {
     }
 }
 
+impl IsOpaque for TemplateInstantiation {
+    type Extra = Item;
+
+    /// Is this an opaque template instantiation?
+    fn is_opaque(&self, ctx: &BindgenContext, item: &Item) -> bool {
+        if self.template_definition().is_opaque(ctx, &()) {
+            return true;
+        }
+
+        // TODO(#774): This doesn't properly handle opaque instantiations where
+        // an argument is itself an instantiation because `canonical_name` does
+        // not insert the template arguments into the name, ie it for nested
+        // template arguments it creates "Foo" instead of "Foo<int>". The fully
+        // correct fix is to make `canonical_{name,path}` include template
+        // arguments properly.
+
+        let mut path = item.canonical_path(ctx);
+        let args: Vec<_> = self.template_arguments()
+            .iter()
+            .map(|arg| {
+                let arg_path = arg.canonical_path(ctx);
+                arg_path[1..].join("::")
+            }).collect();
+        {
+            let mut last = path.last_mut().unwrap();
+            last.push('<');
+            last.push_str(&args.join(", "));
+            last.push('>');
+        }
+
+        ctx.opaque_by_name(&path)
+    }
+}
+
 impl<'a> CanDeriveCopy<'a> for TemplateInstantiation {
     type Extra = ();
 

src/ir/ty.rs

@@ -7,7 +7,7 @@ use super::dot::DotAttributes;
 use super::enum_ty::Enum;
 use super::function::FunctionSig;
 use super::int::IntKind;
-use super::item::Item;
+use super::item::{IsOpaque, Item};
 use super::layout::{Layout, Opaque};
 use super::objc::ObjCInterface;
 use super::template::{AsTemplateParam, TemplateInstantiation, TemplateParameters};
@@ -102,14 +102,6 @@ impl Type {
         }
     }
 
-    /// Is this type of kind `TypeKind::Opaque`?
-    pub fn is_opaque(&self) -> bool {
-        match self.kind {
-            TypeKind::Opaque => true,
-            _ => false,
-        }
-    }
-
     /// Is this type of kind `TypeKind::Named`?
     pub fn is_named(&self) -> bool {
         match self.kind {
@@ -374,6 +366,18 @@ impl Type {
     }
 }
 
+impl IsOpaque for Type {
+    type Extra = Item;
+
+    fn is_opaque(&self, ctx: &BindgenContext, item: &Item) -> bool {
+        match self.kind {
+            TypeKind::Opaque => true,
+            TypeKind::TemplateInstantiation(ref inst) => inst.is_opaque(ctx, item),
+            _ => false,
+        }
+    }
+}
+
 impl AsTemplateParam for Type {
     type Extra = Item;
 

tests/expectations/tests/issue-674-1.rs

@@ -43,13 +43,4 @@ pub mod root {
     impl Clone for CapturingContentInfo {
         fn clone(&self) -> Self { *self }
     }
-    #[test]
-    fn __bindgen_test_layout_Maybe_instantiation() {
-        assert_eq!(::std::mem::size_of::<u8>() , 1usize , concat ! (
-                   "Size of template specialization: " , stringify ! ( u8 )
-                   ));
-        assert_eq!(::std::mem::align_of::<u8>() , 1usize , concat ! (
-                   "Alignment of template specialization: " , stringify ! ( u8
-                   ) ));
-    }
 }