Merge pull request #1305 from tamird/remove-option. r=emiliov0.36.0

TemplateParameters do not return Option

9 files changed, 131 insertions, 180 deletions

diff --git a/Cargo.toml b/Cargo.toml
index a9f2bdc6..c15674c9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -53,7 +53,9 @@ peeking_take_while = "0.1.2"
 quote = { version = "0.5", default-features = false }
 regex = "0.2"
 which = "1.0.2"
-proc-macro2 = { version = "0.3.2", default-features = false }
+# New validation in 0.3.6 breaks bindgen-integration:
+# https://github.com/alexcrichton/proc-macro2/commit/489c642.
+proc-macro2 = { version = "0.3.2, < 0.3.6", default-features = false }
 
 [dependencies.env_logger]
 optional = true
diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
index 6483412e..04e28db2 100644
--- a/src/codegen/mod.rs
+++ b/src/codegen/mod.rs
@@ -299,16 +299,11 @@ impl AppendImplicitTemplateParams for quote::Tokens {
             _ => {},
         }
 
-        if let Some(params) = item.used_template_params(ctx) {
-            if params.is_empty() {
-                return;
-            }
-
-            let params = params.into_iter().map(|p| {
-                p.try_to_rust_ty(ctx, &())
-                    .expect("template params cannot fail to be a rust type")
-            });
-
+        let params: Vec<_> = item.used_template_params(ctx).iter().map(|p| {
+            p.try_to_rust_ty(ctx, &())
+                .expect("template params cannot fail to be a rust type")
+        }).collect();
+        if !params.is_empty() {
             self.append_all(quote! {
                 < #( #params ),* >
             });
@@ -479,11 +474,8 @@ impl CodeGenerator for Var {
         // 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;
-            }
+        if !item.all_template_params(ctx).is_empty() {
+            return;
         }
 
         let ty = self.ty().to_rust_ty_or_opaque(ctx, &());
@@ -636,15 +628,10 @@ impl CodeGenerator for Type {
                     return;
                 }
 
-                let mut outer_params = item.used_template_params(ctx)
-                    .and_then(|ps| if ps.is_empty() {
-                        None
-                    } else {
-                        Some(ps)
-                    });
+                let mut outer_params = item.used_template_params(ctx);
 
                 let inner_rust_type = if item.is_opaque(ctx, &()) {
-                    outer_params = None;
+                    outer_params = vec![];
                     self.to_opaque(ctx, item)
                 } else {
                     // Its possible that we have better layout information than
@@ -699,7 +686,7 @@ impl CodeGenerator for Type {
                         'A'...'Z' | 'a'...'z' | '0'...'9' | ':' | '_' | ' ' => true,
                         _ => false,
                     }) &&
-                    outer_params.is_none() &&
+                    outer_params.is_empty() &&
                     inner_item.expect_type().canonical_type(ctx).is_enum()
                 {
                     tokens.append_all(quote! {
@@ -718,25 +705,23 @@ impl CodeGenerator for Type {
                     pub type #rust_name
                 });
 
-                if let Some(params) = outer_params {
-                    let params: Vec<_> = params.into_iter()
-                        .filter_map(|p| p.as_template_param(ctx, &()))
-                        .collect();
-                    if params.iter().any(|p| ctx.resolve_type(*p).is_invalid_type_param()) {
-                        warn!(
-                            "Item contained invalid template \
-                             parameter: {:?}",
-                            item
-                        );
-                        return;
-                    }
-
-                    let params = params.iter()
-                        .map(|p| {
-                            p.try_to_rust_ty(ctx, &())
-                                .expect("type parameters can always convert to rust ty OK")
-                        });
+                let params: Vec<_> = outer_params.into_iter()
+                    .filter_map(|p| p.as_template_param(ctx, &()))
+                    .collect();
+                if params.iter().any(|p| ctx.resolve_type(*p).is_invalid_type_param()) {
+                    warn!(
+                        "Item contained invalid template \
+                         parameter: {:?}",
+                        item
+                    );
+                    return;
+                }
+                let params: Vec<_> = params.iter().map(|p| {
+                    p.try_to_rust_ty(ctx, &())
+                        .expect("type parameters can always convert to rust ty OK")
+                }).collect();
 
+                if !params.is_empty() {
                     tokens.append_all(quote! {
                         < #( #params ),* >
                     });
@@ -1418,8 +1403,6 @@ impl CodeGenerator for CompInfo {
             return;
         }
 
-        let used_template_params = item.used_template_params(ctx);
-
         let ty = item.expect_type();
         let layout = ty.layout(ctx);
         let mut packed = self.is_packed(ctx, &layout);
@@ -1601,21 +1584,19 @@ impl CodeGenerator for CompInfo {
 
         let mut generic_param_names = vec![];
 
-        if let Some(ref params) = used_template_params {
-            for (idx, ty) in params.iter().enumerate() {
-                let param = ctx.resolve_type(*ty);
-                let name = param.name().unwrap();
-                let ident = ctx.rust_ident(name);
-                generic_param_names.push(ident.clone());
+        for (idx, ty) in item.used_template_params(ctx).iter().enumerate() {
+            let param = ctx.resolve_type(*ty);
+            let name = param.name().unwrap();
+            let ident = ctx.rust_ident(name);
+            generic_param_names.push(ident.clone());
 
-                let prefix = ctx.trait_prefix();
-                let field_name = ctx.rust_ident(format!("_phantom_{}", idx));
-                fields.push(quote! {
-                    pub #field_name : ::#prefix::marker::PhantomData<
-                        ::#prefix::cell::UnsafeCell<#ident>
-                    > ,
-                });
-            }
+            let prefix = ctx.trait_prefix();
+            let field_name = ctx.rust_ident(format!("_phantom_{}", idx));
+            fields.push(quote! {
+                pub #field_name : ::#prefix::marker::PhantomData<
+                    ::#prefix::cell::UnsafeCell<#ident>
+                > ,
+            });
         }
 
         let generics = if !generic_param_names.is_empty() {
@@ -1668,11 +1649,13 @@ impl CodeGenerator for CompInfo {
                 ctx.options().derive_default && !self.is_forward_declaration();
         }
 
+        let all_template_params = item.all_template_params(ctx);
+
         if item.can_derive_copy(ctx) && !item.annotations().disallow_copy() {
             derives.push("Copy");
 
             if ctx.options().rust_features().builtin_clone_impls ||
-                used_template_params.is_some()
+                !all_template_params.is_empty()
             {
                 // FIXME: This requires extra logic if you have a big array in a
                 // templated struct. The reason for this is that the magic:
@@ -1754,7 +1737,7 @@ impl CodeGenerator for CompInfo {
             );
         }
 
-        if used_template_params.is_none() {
+        if all_template_params.is_empty() {
             if !is_opaque {
                 for var in self.inner_vars() {
                     ctx.resolve_item(*var).codegen(ctx, result, &());
@@ -3023,7 +3006,6 @@ impl TryToRustTy for Type {
             TypeKind::TemplateAlias(..) |
             TypeKind::Alias(..) => {
                 let template_params = item.used_template_params(ctx)
-                    .unwrap_or(vec![])
                     .into_iter()
                     .filter(|param| param.is_template_param(ctx, &()))
                     .collect::<Vec<_>>();
@@ -3042,9 +3024,9 @@ impl TryToRustTy for Type {
                 }
             }
             TypeKind::Comp(ref info) => {
-                let template_params = item.used_template_params(ctx);
+                let template_params = item.all_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_empty())
                 {
                     return self.try_to_opaque(ctx, item);
                 }
@@ -3138,18 +3120,16 @@ impl TryToRustTy for TemplateInstantiation {
         let def_path = def.namespace_aware_canonical_path(ctx);
         ty.append_separated(def_path.into_iter().map(|p| ctx.rust_ident(p)), Term::new("::", Span::call_site()));
 
-        let def_params = match def.self_template_params(ctx) {
-            Some(params) => params,
-            None => {
-                // This can happen if we generated an opaque type for a partial
-                // template specialization, and we've hit an instantiation of
-                // that partial specialization.
-                extra_assert!(
-                    def.is_opaque(ctx, &())
-                );
-                return Err(error::Error::InstantiationOfOpaqueType);
-            }
-        };
+        let def_params = def.self_template_params(ctx);
+        if def_params.is_empty() {
+            // This can happen if we generated an opaque type for a partial
+            // template specialization, and we've hit an instantiation of
+            // that partial specialization.
+            extra_assert!(
+                def.is_opaque(ctx, &())
+            );
+            return Err(error::Error::InstantiationOfOpaqueType);
+        }
 
         // TODO: If the definition type is a template class/struct
         // definition's member template definition, it could rely on
@@ -3242,11 +3222,8 @@ impl CodeGenerator for Function {
         // 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;
-            }
+        if !item.all_template_params(ctx).is_empty() {
+            return;
         }
 
         let name = self.name();
diff --git a/src/ir/analysis/derive_copy.rs b/src/ir/analysis/derive_copy.rs
index 69725ead..1e8dc819 100644
--- a/src/ir/analysis/derive_copy.rs
+++ b/src/ir/analysis/derive_copy.rs
@@ -246,8 +246,8 @@ impl<'ctx> MonotoneFramework for CannotDeriveCopy<'ctx> {
                     }
 
                     // https://github.com/rust-lang/rust/issues/36640
-                    if info.self_template_params(self.ctx).is_some() ||
-                        item.used_template_params(self.ctx).is_some()
+                    if !info.self_template_params(self.ctx).is_empty() ||
+                        !item.all_template_params(self.ctx).is_empty()
                     {
                         trace!(
                             "    comp cannot derive copy because issue 36640"
diff --git a/src/ir/analysis/template_params.rs b/src/ir/analysis/template_params.rs
index 00504aa4..b326e6b5 100644
--- a/src/ir/analysis/template_params.rs
+++ b/src/ir/analysis/template_params.rs
@@ -275,7 +275,7 @@ impl<'ctx> UsedTemplateParameters<'ctx> {
         let decl = self.ctx.resolve_type(instantiation.template_definition());
         let args = instantiation.template_arguments();
 
-        let params = decl.self_template_params(self.ctx).unwrap_or(vec![]);
+        let params = decl.self_template_params(self.ctx);
 
         debug_assert!(this_id != instantiation.template_definition());
         let used_by_def = self.used
@@ -419,7 +419,7 @@ impl<'ctx> MonotoneFramework for UsedTemplateParameters<'ctx> {
                     // template parameters, there is a single exception:
                     // opaque templates. Hence the unwrap_or.
                     let params =
-                        decl.self_template_params(ctx).unwrap_or(vec![]);
+                        decl.self_template_params(ctx);
 
                     for (arg, param) in args.iter().zip(params.iter()) {
                         let arg = arg.into_resolver()
diff --git a/src/ir/comp.rs b/src/ir/comp.rs
index 8c2be498..8c529067 100644
--- a/src/ir/comp.rs
+++ b/src/ir/comp.rs
@@ -1669,12 +1669,8 @@ impl TemplateParameters for CompInfo {
     fn self_template_params(
         &self,
         _ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
-        if self.template_params.is_empty() {
-            None
-        } else {
-            Some(self.template_params.clone())
-        }
+    ) -> Vec<TypeId> {
+        self.template_params.clone()
     }
 }
 
@@ -1685,8 +1681,7 @@ impl Trace for CompInfo {
     where
         T: Tracer,
     {
-        let params = item.all_template_params(context).unwrap_or(vec![]);
-        for p in params {
+        for p in item.all_template_params(context) {
             tracer.visit_kind(p.into(), EdgeKind::TemplateParameterDefinition);
         }
 
diff --git a/src/ir/context.rs b/src/ir/context.rs
index 58a90ba6..bbcc5698 100644
--- a/src/ir/context.rs
+++ b/src/ir/context.rs
@@ -1366,10 +1366,7 @@ impl BindgenContext {
             let mut used_params = HashMap::new();
             for &id in self.whitelisted_items() {
                 used_params.entry(id).or_insert(
-                    id.self_template_params(self).map_or(
-                        Default::default(),
-                        |params| params.into_iter().map(|p| p.into()).collect(),
-                    ),
+                    id.self_template_params(self).into_iter().map(|p| p.into()).collect()
                 );
             }
             self.used_template_parameters = Some(used_params);
@@ -1552,15 +1549,16 @@ impl BindgenContext {
             .and_then(|canon_decl| {
                 self.get_resolved_type(&canon_decl).and_then(
                     |template_decl_id| {
-                        template_decl_id.num_self_template_params(self).map(
-                            |num_params| {
-                                (
-                                    *canon_decl.cursor(),
-                                    template_decl_id.into(),
-                                    num_params,
-                                )
-                            },
-                        )
+                        let num_params = template_decl_id.num_self_template_params(self);
+                        if num_params == 0 {
+                            None
+                        } else {
+                            Some((
+                                *canon_decl.cursor(),
+                                template_decl_id.into(),
+                                num_params,
+                            ))
+                        }
                     },
                 )
             })
@@ -1581,15 +1579,16 @@ impl BindgenContext {
                             .cloned()
                     })
                     .and_then(|template_decl| {
-                        template_decl.num_self_template_params(self).map(
-                            |num_template_params| {
-                                (
-                                    *template_decl.decl(),
-                                    template_decl.id(),
-                                    num_template_params,
-                                )
-                            },
-                        )
+                        let num_template_params = template_decl.num_self_template_params(self);
+                        if num_template_params == 0 {
+                            None
+                        } else {
+                            Some((
+                                *template_decl.decl(),
+                                template_decl.id(),
+                                num_template_params,
+                            ))
+                        }
                     })
             })
     }
@@ -1636,17 +1635,14 @@ impl BindgenContext {
     ) -> Option<TypeId> {
         use clang_sys;
 
-        let num_expected_args = match self.resolve_type(template)
-            .num_self_template_params(self) {
-            Some(n) => n,
-            None => {
-                warn!(
-                    "Tried to instantiate a template for which we could not \
-                       determine any template parameters"
-                );
-                return None;
-            }
-        };
+        let num_expected_args = self.resolve_type(template).num_self_template_params(self);
+        if num_expected_args == 0 {
+            warn!(
+                "Tried to instantiate a template for which we could not \
+                   determine any template parameters"
+            );
+            return None;
+        }
 
         let mut args = vec![];
         let mut found_const_arg = false;
@@ -2647,13 +2643,13 @@ impl TemplateParameters for PartialType {
     fn self_template_params(
         &self,
         _ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
+    ) -> Vec<TypeId> {
         // Maybe at some point we will eagerly parse named types, but for now we
         // don't and this information is unavailable.
-        None
+        vec![]
     }
 
-    fn num_self_template_params(&self, _ctx: &BindgenContext) -> Option<usize> {
+    fn num_self_template_params(&self, _ctx: &BindgenContext) -> usize {
         // Wouldn't it be nice if libclang would reliably give us this
         // information‽
         match self.decl().kind() {
@@ -2672,9 +2668,9 @@ impl TemplateParameters for PartialType {
                     };
                     clang_sys::CXChildVisit_Continue
                 });
-                Some(num_params)
+                num_params
             }
-            _ => None,
+            _ => 0,
         }
     }
 }
diff --git a/src/ir/item.rs b/src/ir/item.rs
index 2b3c1b8c..1cb1d5f9 100644
--- a/src/ir/item.rs
+++ b/src/ir/item.rs
@@ -1112,8 +1112,8 @@ where
     fn self_template_params(
         &self,
         ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
-        ctx.resolve_item_fallible(*self).and_then(|item| {
+    ) -> Vec<TypeId> {
+        ctx.resolve_item_fallible(*self).map_or(vec![], |item| {
             item.self_template_params(ctx)
         })
     }
@@ -1123,7 +1123,7 @@ impl TemplateParameters for Item {
     fn self_template_params(
         &self,
         ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
+    ) -> Vec<TypeId> {
         self.kind.self_template_params(ctx)
     }
 }
@@ -1132,7 +1132,7 @@ impl TemplateParameters for ItemKind {
     fn self_template_params(
         &self,
         ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
+    ) -> Vec<TypeId> {
         match *self {
             ItemKind::Type(ref ty) => ty.self_template_params(ctx),
             // If we start emitting bindings to explicitly instantiated
@@ -1140,7 +1140,7 @@ impl TemplateParameters for ItemKind {
             // template params.
             ItemKind::Function(_) |
             ItemKind::Module(_) |
-            ItemKind::Var(_) => None,
+            ItemKind::Var(_) => vec![],
         }
     }
 }
diff --git a/src/ir/template.rs b/src/ir/template.rs
index 11a799f4..61654fe8 100644
--- a/src/ir/template.rs
+++ b/src/ir/template.rs
@@ -81,23 +81,23 @@ use parse::ClangItemParser;
 /// +------+----------------------+--------------------------+------------------------+----
 /// |Decl. | self_template_params | num_self_template_params | all_template_parameters| ...
 /// +------+----------------------+--------------------------+------------------------+----
-/// |Foo   | Some([T, U])         | Some(2)                  | Some([T, U])           | ...
-/// |Bar   | Some([V])            | Some(1)                  | Some([T, U, V])        | ...
-/// |Inner | None                 | None                     | Some([T, U])           | ...
-/// |Lol   | Some([W])            | Some(1)                  | Some([T, U, W])        | ...
-/// |Wtf   | Some([X])            | Some(1)                  | Some([T, U, X])        | ...
-/// |Qux   | None                 | None                     | None                   | ...
+/// |Foo   | [T, U]               | 2                        | [T, U]                 | ...
+/// |Bar   | [V]                  | 1                        | [T, U, V]              | ...
+/// |Inner | []                   | 0                        | [T, U]                 | ...
+/// |Lol   | [W]                  | 1                        | [T, U, W]              | ...
+/// |Wtf   | [X]                  | 1                        | [T, U, X]              | ...
+/// |Qux   | []                   | 0                        | []                     | ...
 /// +------+----------------------+--------------------------+------------------------+----
 ///
 /// ----+------+-----+----------------------+
 /// ... |Decl. | ... | used_template_params |
 /// ----+------+-----+----------------------+
-/// ... |Foo   | ... | Some([T, U])         |
-/// ... |Bar   | ... | Some([V])            |
-/// ... |Inner | ... | None                 |
-/// ... |Lol   | ... | Some([T])            |
-/// ... |Wtf   | ... | Some([T])            |
-/// ... |Qux   | ... | None                 |
+/// ... |Foo   | ... | [T, U]               |
+/// ... |Bar   | ... | [V]                  |
+/// ... |Inner | ... | []                   |
+/// ... |Lol   | ... | [T]                  |
+/// ... |Wtf   | ... | [T]                  |
+/// ... |Qux   | ... | []                   |
 /// ----+------+-----+----------------------+
 pub trait TemplateParameters {
     /// Get the set of `ItemId`s that make up this template declaration's free
@@ -109,17 +109,12 @@ pub trait TemplateParameters {
     /// anything but types, so we must treat them as opaque, and avoid
     /// instantiating them.
     fn self_template_params(&self, ctx: &BindgenContext)
-        -> Option<Vec<TypeId>>;
+        -> Vec<TypeId>;
 
     /// Get the number of free template parameters this template declaration
     /// has.
-    ///
-    /// Implementations *may* return `Some` from this method when
-    /// `template_params` returns `None`. This is useful when we only have
-    /// partial information about the template declaration, such as when we are
-    /// in the middle of parsing it.
-    fn num_self_template_params(&self, ctx: &BindgenContext) -> Option<usize> {
-        self.self_template_params(ctx).map(|params| params.len())
+    fn num_self_template_params(&self, ctx: &BindgenContext) -> usize {
+        self.self_template_params(ctx).len()
     }
 
     /// Get the complete set of template parameters that can affect this
@@ -136,30 +131,20 @@ pub trait TemplateParameters {
     /// how we would fully reference such a member type in C++:
     /// `Foo<int,char>::Inner`. `Foo` *must* be instantiated with template
     /// arguments before we can gain access to the `Inner` member type.
-    fn all_template_params(&self, ctx: &BindgenContext) -> Option<Vec<TypeId>>
+    fn all_template_params(&self, ctx: &BindgenContext) -> Vec<TypeId>
     where
         Self: ItemAncestors,
     {
-        let each_self_params: Vec<Vec<_>> = self.ancestors(ctx)
-            .filter_map(|id| id.self_template_params(ctx))
-            .collect();
-        if each_self_params.is_empty() {
-            None
-        } else {
-            Some(
-                each_self_params
-                    .into_iter()
-                    .rev()
-                    .flat_map(|params| params)
-                    .collect(),
-            )
-        }
+        let ancestors: Vec<_> = self.ancestors(ctx).collect();
+        ancestors.into_iter().rev().flat_map(|id| {
+            id.self_template_params(ctx).into_iter()
+        }).collect()
     }
 
     /// Get only the set of template parameters that this item uses. This is a
     /// subset of `all_template_params` and does not necessarily contain any of
     /// `self_template_params`.
-    fn used_template_params(&self, ctx: &BindgenContext) -> Option<Vec<TypeId>>
+    fn used_template_params(&self, ctx: &BindgenContext) -> Vec<TypeId>
     where
         Self: AsRef<ItemId>,
     {
@@ -169,14 +154,10 @@ pub trait TemplateParameters {
         );
 
         let id = *self.as_ref();
-        ctx.resolve_item(id).all_template_params(ctx).map(
-            |all_params| {
-                all_params
+        ctx.resolve_item(id).all_template_params(ctx)
                     .into_iter()
                     .filter(|p| ctx.uses_template_parameter(id, *p))
                     .collect()
-            },
-        )
     }
 }
 
diff --git a/src/ir/ty.rs b/src/ir/ty.rs
index b42f4424..b5c78c16 100644
--- a/src/ir/ty.rs
+++ b/src/ir/ty.rs
@@ -540,7 +540,7 @@ impl TemplateParameters for Type {
     fn self_template_params(
         &self,
         ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
+    ) -> Vec<TypeId> {
         self.kind.self_template_params(ctx)
     }
 }
@@ -549,13 +549,13 @@ impl TemplateParameters for TypeKind {
     fn self_template_params(
         &self,
         ctx: &BindgenContext,
-    ) -> Option<Vec<TypeId>> {
+    ) -> Vec<TypeId> {
         match *self {
             TypeKind::ResolvedTypeRef(id) => {
                 ctx.resolve_type(id).self_template_params(ctx)
             }
             TypeKind::Comp(ref comp) => comp.self_template_params(ctx),
-            TypeKind::TemplateAlias(_, ref args) => Some(args.clone()),
+            TypeKind::TemplateAlias(_, ref args) => args.clone(),
 
             TypeKind::Opaque |
             TypeKind::TemplateInstantiation(..) |
@@ -575,7 +575,7 @@ impl TemplateParameters for TypeKind {
             TypeKind::Alias(_) |
             TypeKind::ObjCId |
             TypeKind::ObjCSel |
-            TypeKind::ObjCInterface(_) => None,
+            TypeKind::ObjCInterface(_) => vec![],
         }
     }
 }