Auto merge of #773 - fitzgen:allow-making-particular-instantiations-opaque, r=emilio

Allow marking specific template instantiations as opaque

If a template has a specialization that bindgen doesn't understand, it can be
helpful to mark it as opaque and continue making forward progress in the
meantime. This is something we need in the SpiderMonkey bindings.

r? @emilio

6 files changed, 114 insertions, 45 deletions

diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
index d926f02b..50ebebf2 100644
--- a/src/codegen/mod.rs
+++ b/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);
             }
         };
diff --git a/src/ir/comp.rs b/src/ir/comp.rs
index 315d342a..6dfc9980 100644
--- a/src/ir/comp.rs
+++ b/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;
         }
 
diff --git a/src/ir/context.rs b/src/ir/context.rs
index eb0fd98a..7b4e9b6a 100644
--- a/src/ir/context.rs
+++ b/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();
diff --git a/src/ir/item.rs b/src/ir/item.rs
index b80ddbd9..3564c6e8 100644
--- a/src/ir/item.rs
+++ b/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>")?;
         }
 
diff --git a/src/ir/template.rs b/src/ir/template.rs
index 722c1b81..fcba40e1 100644
--- a/src/ir/template.rs
+++ b/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 = ();
 
diff --git a/src/ir/ty.rs b/src/ir/ty.rs
index 3e5f53b0..d19a4889 100644
--- a/src/ir/ty.rs
+++ b/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;