From 290707f9d3d4d507eef6bc3cdd75f9555fa372d2 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 3 Feb 2017 15:07:13 -0800 Subject: [PATCH 01/12] Derive Copy for clang::Type --- src/clang.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clang.rs b/src/clang.rs index ffe9d5d04d..72cd567cef 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -573,7 +573,7 @@ impl Hash for Cursor { } /// The type of a node in clang's AST. -#[derive(Clone)] +#[derive(Clone, Copy)] pub struct Type { x: CXType, } From 910b0d6458f51e553b0e229af41fa313b3d3f10c Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:26:05 -0800 Subject: [PATCH 02/12] Add children-related helper methods to Cursor This commit adds collect_children, has_children, and has_at_least_num_children methods to Cursor. --- src/clang.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/clang.rs b/src/clang.rs index 72cd567cef..b35d218787 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -361,6 +361,41 @@ impl Cursor { } } + /// Collect all of this cursor's children into a vec and return them. + pub fn collect_children(&self) -> Vec { + let mut children = vec![]; + self.visit(|c| { + children.push(c); + CXChildVisit_Continue + }); + children + } + + /// Does this cursor have any children? + pub fn has_children(&self) -> bool { + let mut has_children = false; + self.visit(|_| { + has_children = true; + CXChildVisit_Break + }); + has_children + } + + /// Does this cursor have at least `n` children? + pub fn has_at_least_num_children(&self, n: usize) -> bool { + assert!(n > 0); + let mut num_left = n; + self.visit(|_| { + num_left -= 1; + if num_left == 0 { + CXChildVisit_Break + } else { + CXChildVisit_Continue + } + }); + num_left == 0 + } + /// Returns whether the given location contains a cursor with the given /// kind in the first level of nesting underneath (doesn't look /// recursively). From 775c15c3dcfa488c0ca4236805606f345fbbc2e2 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:33:57 -0800 Subject: [PATCH 03/12] Introduce a CanonicalDeclaration type The `CanonicalTypeDeclaration` type exists as proof-by-construction that its cursor is the canonical declaration for its type. If you have a `CanonicalTypeDeclaration` instance, you know for sure that the type and cursor match up in a canonical declaration relationship, and it simply cannot be otherwise. --- src/clang.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/clang.rs b/src/clang.rs index b35d218787..6474db2cee 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -684,6 +684,31 @@ impl Type { } } + /// Get the canonical declaration of this type, if it is available. + pub fn canonical_declaration(&self, + location: Option<&Cursor>) + -> Option { + let mut declaration = self.declaration(); + if !declaration.is_valid() { + if let Some(location) = location { + let mut location = *location; + if let Some(referenced) = location.referenced() { + location = referenced; + } + if location.is_template_like() { + declaration = location; + } + } + } + + let canonical = declaration.canonical(); + if canonical.is_valid() && canonical.kind() != CXCursor_NoDeclFound { + Some(CanonicalTypeDeclaration(*self, canonical)) + } else { + None + } + } + /// Get a raw display name for this type. pub fn spelling(&self) -> String { unsafe { cxstring_into_string(clang_getTypeSpelling(self.x)) } @@ -874,6 +899,26 @@ impl Type { } } +/// The `CanonicalTypeDeclaration` type exists as proof-by-construction that its +/// cursor is the canonical declaration for its type. If you have a +/// `CanonicalTypeDeclaration` instance, you know for sure that the type and +/// cursor match up in a canonical declaration relationship, and it simply +/// cannot be otherwise. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CanonicalTypeDeclaration(Type, Cursor); + +impl CanonicalTypeDeclaration { + /// Get the type. + pub fn ty(&self) -> &Type { + &self.0 + } + + /// Get the type's canonical declaration cursor. + pub fn cursor(&self) -> &Cursor { + &self.1 + } +} + /// An iterator for a type's template arguments. pub struct TypeTemplateArgIterator { x: CXType, From abb10933a43599db506138c01285ada9f5fce114 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:36:13 -0800 Subject: [PATCH 04/12] Rustfmt on src/clang.rs --- src/clang.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/clang.rs b/src/clang.rs index 6474db2cee..86105fc1fc 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -132,11 +132,11 @@ impl Cursor { // `clang_Cursor_getNumTemplateArguments` is totally unreliable. // Therefore, try former first, and only fallback to the latter if we // have to. - self.cur_type().num_template_args() + self.cur_type() + .num_template_args() .or_else(|| { - let n: c_int = unsafe { - clang_Cursor_getNumTemplateArguments(self.x) - }; + let n: c_int = + unsafe { clang_Cursor_getNumTemplateArguments(self.x) }; if n >= 0 { Some(n as u32) @@ -783,10 +783,12 @@ impl Type { /// If this type is a class template specialization, return its /// template arguments. Otherwise, return None. pub fn template_args(&self) -> Option { - self.num_template_args().map(|n| TypeTemplateArgIterator { - x: self.x, - length: n, - index: 0, + self.num_template_args().map(|n| { + TypeTemplateArgIterator { + x: self.x, + length: n, + index: 0, + } }) } @@ -888,9 +890,8 @@ impl Type { // Yep, the spelling of this containing type-parameter is extremely // nasty... But can happen in . Unfortunately I couldn't // reduce it enough :( - self.template_args().map_or(false, |args| { - args.len() > 0 - }) && match self.declaration().kind() { + self.template_args().map_or(false, |args| args.len() > 0) && + match self.declaration().kind() { CXCursor_ClassTemplatePartialSpecialization | CXCursor_TypeAliasTemplateDecl | CXCursor_TemplateTemplateParameter => false, From a69b50b01641f797d25a8caa60b036eb360550e3 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:40:59 -0800 Subject: [PATCH 05/12] Rename TypeKind::TemplateRef to TypeKind::TemplateInstantiation --- src/codegen/mod.rs | 4 ++-- src/ir/item.rs | 4 ++-- src/ir/ty.rs | 27 +++++++++++++-------------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index d3e204ce91..0d605c1482 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -521,7 +521,7 @@ impl CodeGenerator for Type { TypeKind::Pointer(..) | TypeKind::BlockPointer | TypeKind::Reference(..) | - TypeKind::TemplateRef(..) | + TypeKind::TemplateInstantiation(..) | TypeKind::Function(..) | TypeKind::ResolvedTypeRef(..) | TypeKind::Named => { @@ -2180,7 +2180,7 @@ impl ToRustTy for Type { let path = item.namespace_aware_canonical_path(ctx); aster::AstBuilder::new().ty().path().ids(path).build() } - TypeKind::TemplateRef(inner, ref template_args) => { + TypeKind::TemplateInstantiation(inner, ref template_args) => { // PS: Sorry for the duplication here. let mut inner_ty = inner.to_rust_ty(ctx).unwrap(); diff --git a/src/ir/item.rs b/src/ir/item.rs index c8de95c072..f3d7a64478 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -619,7 +619,7 @@ impl Item { // XXX Is this completely correct? Partial template specialization // is hard anyways, sigh... TypeKind::TemplateAlias(_, ref args) | - TypeKind::TemplateRef(_, ref args) => args.clone(), + TypeKind::TemplateInstantiation(_, ref args) => args.clone(), // In a template specialization we've got all we want. TypeKind::Comp(ref ci) if ci.is_template_specialization() => { ci.template_args().iter().cloned().collect() @@ -718,7 +718,7 @@ impl Item { } // Same as above. TypeKind::ResolvedTypeRef(inner) | - TypeKind::TemplateRef(inner, _) => { + TypeKind::TemplateInstantiation(inner, _) => { item = ctx.resolve_item(inner); } _ => return item.id(), diff --git a/src/ir/ty.rs b/src/ir/ty.rs index be108cc0b8..a2a6562342 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -217,7 +217,7 @@ impl Type { pub fn has_vtable(&self, ctx: &BindgenContext) -> bool { // FIXME: Can we do something about template parameters? Huh... match self.kind { - TypeKind::TemplateRef(t, _) | + TypeKind::TemplateInstantiation(t, _) | TypeKind::TemplateAlias(t, _) | TypeKind::Alias(t) | TypeKind::ResolvedTypeRef(t) => ctx.resolve_type(t).has_vtable(ctx), @@ -230,7 +230,7 @@ impl Type { /// Returns whether this type has a destructor. pub fn has_destructor(&self, ctx: &BindgenContext) -> bool { match self.kind { - TypeKind::TemplateRef(t, _) | + TypeKind::TemplateInstantiation(t, _) | TypeKind::TemplateAlias(t, _) | TypeKind::Alias(t) | TypeKind::ResolvedTypeRef(t) => { @@ -269,7 +269,7 @@ impl Type { .signature_contains_named_type(ctx, ty) } TypeKind::TemplateAlias(_, ref template_args) | - TypeKind::TemplateRef(_, ref template_args) => { + TypeKind::TemplateInstantiation(_, ref template_args) => { template_args.iter().any(|arg| { ctx.resolve_type(*arg) .signature_contains_named_type(ctx, ty) @@ -336,7 +336,7 @@ impl Type { TypeKind::ResolvedTypeRef(inner) | TypeKind::Alias(inner) | TypeKind::TemplateAlias(inner, _) | - TypeKind::TemplateRef(inner, _) => { + TypeKind::TemplateInstantiation(inner, _) => { ctx.resolve_type(inner).safe_canonical_type(ctx) } @@ -352,7 +352,7 @@ impl Type { TypeKind::Pointer(..) | TypeKind::Array(..) | TypeKind::Reference(..) | - TypeKind::TemplateRef(..) | + TypeKind::TemplateInstantiation(..) | TypeKind::ResolvedTypeRef(..) => true, _ => false, } @@ -379,7 +379,7 @@ impl Type { TypeKind::ResolvedTypeRef(inner) | TypeKind::Alias(inner) | TypeKind::TemplateAlias(inner, _) | - TypeKind::TemplateRef(inner, _) => { + TypeKind::TemplateInstantiation(inner, _) => { ctx.resolve_type(inner).calc_size(ctx) } TypeKind::Array(inner, len) => { @@ -474,7 +474,7 @@ impl CanDeriveDefault for Type { } TypeKind::Void | TypeKind::Named | - TypeKind::TemplateRef(..) | + TypeKind::TemplateInstantiation(..) | TypeKind::Reference(..) | TypeKind::NullPtr | TypeKind::Pointer(..) | @@ -501,7 +501,7 @@ impl<'a> CanDeriveCopy<'a> for Type { } TypeKind::ResolvedTypeRef(t) | TypeKind::TemplateAlias(t, _) | - TypeKind::TemplateRef(t, _) | + TypeKind::TemplateInstantiation(t, _) | TypeKind::Alias(t) => t.can_derive_copy(ctx, ()), TypeKind::Comp(ref info) => { info.can_derive_copy(ctx, (item, self.layout(ctx))) @@ -597,10 +597,9 @@ pub enum TypeKind { /// A reference to a type, as in: int& foo(). Reference(ItemId), - /// A reference to a template, with different template parameter names. To - /// see why this is needed, check out the creation of this variant in - /// `Type::from_clang_ty`. - TemplateRef(ItemId, Vec), + /// An instantiation of an abstract template declaration (first tuple + /// member) with a set of concrete template arguments (second tuple member). + TemplateInstantiation(ItemId, Vec), /// A reference to a yet-to-resolve type. This stores the clang cursor /// itself, and postpones its resolution. @@ -644,7 +643,7 @@ impl Type { TypeKind::ResolvedTypeRef(inner) | TypeKind::Alias(inner) | TypeKind::TemplateAlias(inner, _) | - TypeKind::TemplateRef(inner, _) => { + TypeKind::TemplateInstantiation(inner, _) => { ctx.resolve_type(inner).is_unsized(ctx) } TypeKind::Named | @@ -1087,7 +1086,7 @@ impl TypeCollector for Type { } TypeKind::TemplateAlias(inner, ref template_args) | - TypeKind::TemplateRef(inner, ref template_args) => { + TypeKind::TemplateInstantiation(inner, ref template_args) => { types.insert(inner); for &item in template_args { types.insert(item); From 19fe07d8cbef3124cad9394f005c96878422f085 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:44:18 -0800 Subject: [PATCH 06/12] Introduce the TemplateDeclaration trait The TemplateDeclaration trait aggregates information about template declarations (separate from instantiations and specializations) and their template parameters into a single source of truth. --- src/ir/comp.rs | 18 +++++++++++---- src/ir/context.rs | 2 +- src/ir/item.rs | 28 ++++++++++++++++++++++- src/ir/ty.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 1ca3955993..80372cabcc 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -5,7 +5,7 @@ use super::context::{BindgenContext, ItemId}; use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault}; use super::item::Item; use super::layout::Layout; -use super::ty::Type; +use super::ty::{TemplateDeclaration, Type}; use super::type_collector::{ItemSet, TypeCollector}; use clang; use parse::{ClangItemParser, ParseError}; @@ -564,10 +564,8 @@ impl CompInfo { }); ci.is_anonymous = cursor.is_anonymous(); ci.template_args = match ty.template_args() { - // In forward declarations and not specializations, - // etc, they are in - // the ast, we'll meet them in - // CXCursor_TemplateTypeParameter + // In forward declarations and not specializations, etc, they are in + // the ast, we'll meet them in CXCursor_TemplateTypeParameter None => vec![], Some(arg_types) => { let num_arg_types = arg_types.len(); @@ -916,6 +914,16 @@ impl CompInfo { } } +impl TemplateDeclaration for CompInfo { + fn template_params(&self, _ctx: &BindgenContext) -> Option> { + if self.template_args.is_empty() { + None + } else { + Some(self.template_args.clone()) + } + } +} + impl CanDeriveDebug for CompInfo { type Extra = Option; diff --git a/src/ir/context.rs b/src/ir/context.rs index a748239496..edf42ec8ea 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -5,7 +5,7 @@ use super::int::IntKind; use super::item::{Item, ItemCanonicalPath}; use super::item_kind::ItemKind; use super::module::{Module, ModuleKind}; -use super::ty::{FloatKind, Type, TypeKind}; +use super::ty::{FloatKind, TemplateDeclaration, Type, TypeKind}; use super::type_collector::{ItemSet, TypeCollector}; use BindgenOptions; use cexpr; diff --git a/src/ir/item.rs b/src/ir/item.rs index f3d7a64478..3ce228b65e 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -6,7 +6,7 @@ use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault}; use super::function::Function; use super::item_kind::ItemKind; use super::module::Module; -use super::ty::{Type, TypeKind}; +use super::ty::{TemplateDeclaration, Type, TypeKind}; use super::type_collector::{ItemSet, TypeCollector}; use clang; use clang_sys; @@ -902,6 +902,32 @@ impl Item { } } +impl TemplateDeclaration for ItemId { + fn template_params(&self, ctx: &BindgenContext) -> Option> { + ctx.resolve_item_fallible(*self) + .and_then(|item| item.template_params(ctx)) + } +} + +impl TemplateDeclaration for Item { + fn template_params(&self, ctx: &BindgenContext) -> Option> { + self.kind.template_params(ctx) + } +} + +impl TemplateDeclaration for ItemKind { + fn template_params(&self, ctx: &BindgenContext) -> Option> { + match *self { + ItemKind::Type(ref ty) => ty.template_params(ctx), + // TODO FITZGEN: shouldn't functions be able to have free template + // params? + ItemKind::Module(_) | + ItemKind::Function(_) | + ItemKind::Var(_) => None, + } + } +} + // An utility function to handle recursing inside nested types. fn visit_child(cur: clang::Cursor, id: ItemId, diff --git a/src/ir/ty.rs b/src/ir/ty.rs index a2a6562342..0c90547e09 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -14,6 +14,29 @@ use clang::{self, Cursor}; use parse::{ClangItemParser, ParseError, ParseResult}; use std::mem; +/// Template declaration related methods. +pub trait TemplateDeclaration { + /// Get the set of `ItemId`s that make up this template declaration's free + /// template parameters. + /// + /// Note that these might *not* all be named types: C++ allows + /// constant-value template parameters. Of course, Rust does not allow + /// generic parameters to be anything but types, so we must treat them as + /// opaque, and avoid instantiating them. + fn template_params(&self, ctx: &BindgenContext) -> Option>; + + /// 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_template_params(&self, ctx: &BindgenContext) -> Option { + self.template_params(ctx).map(|params| params.len()) + } +} + /// The base representation of a type in bindgen. /// /// A type has an optional name, which if present cannot be empty, a `layout` @@ -438,6 +461,41 @@ fn is_invalid_named_type_empty_name() { } +impl TemplateDeclaration for Type { + fn template_params(&self, ctx: &BindgenContext) -> Option> { + self.kind.template_params(ctx) + } +} + +impl TemplateDeclaration for TypeKind { + fn template_params(&self, ctx: &BindgenContext) -> Option> { + match *self { + TypeKind::ResolvedTypeRef(id) => { + ctx.resolve_type(id).template_params(ctx) + } + TypeKind::Comp(ref comp) => comp.template_params(ctx), + TypeKind::TemplateAlias(_, ref args) => Some(args.clone()), + + TypeKind::TemplateInstantiation(..) | + TypeKind::Void | + TypeKind::NullPtr | + TypeKind::Int(_) | + TypeKind::Float(_) | + TypeKind::Complex(_) | + TypeKind::Array(..) | + TypeKind::Function(_) | + TypeKind::Enum(_) | + TypeKind::Pointer(_) | + TypeKind::BlockPointer | + TypeKind::Reference(_) | + TypeKind::UnresolvedTypeRef(..) | + TypeKind::Named | + TypeKind::Alias(_) | + TypeKind::ObjCInterface(_) => None, + } + } +} + impl CanDeriveDebug for Type { type Extra = (); From c8a223e898e883c70f1d4d6b758ffc0cf3dc28d2 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:48:48 -0800 Subject: [PATCH 07/12] Create PartialType for types we are in the middle of parsing This commit create the PartialType type to represent types that we are in the middle of parsing and their cursor where we found them. Additionally, it fixes a long standing FIXME to make `currently_parsed_types` private. Finally, it implements `TemplateDeclaration` for `PartialType` so that we can get information about a partially parsed template declaration type's template parameters. --- src/ir/context.rs | 86 +++++++++++++++++++++++++++++++++++++++++++++-- src/ir/item.rs | 28 +++++++-------- src/ir/ty.rs | 2 +- 3 files changed, 98 insertions(+), 18 deletions(-) diff --git a/src/ir/context.rs b/src/ir/context.rs index edf42ec8ea..a2277b398f 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -120,9 +120,7 @@ pub struct BindgenContext<'ctx> { /// We could also use the `types` HashMap, but my intention with it is that /// only valid types and declarations end up there, and this could /// potentially break that assumption. - /// - /// FIXME: Should not be public, though... meh. - pub currently_parsed_types: Vec<(Cursor, ItemId)>, + currently_parsed_types: Vec, /// A HashSet with all the already parsed macro names. This is done to avoid /// hard errors while parsing duplicated macros, as well to allow macro @@ -193,6 +191,26 @@ impl<'ctx> BindgenContext<'ctx> { me } + /// Get the stack of partially parsed types that we are in the middle of + /// parsing. + pub fn currently_parsed_types(&self) -> &[PartialType] { + &self.currently_parsed_types[..] + } + + /// Begin parsing the given partial type, and push it onto the + /// `currently_parsed_types` stack so that we won't infinite recurse if we + /// run into a reference to it while parsing it. + pub fn begin_parsing(&mut self, partial_ty: PartialType) { + self.currently_parsed_types.push(partial_ty); + } + + /// Finish parsing the current partial type, pop it off the + /// `currently_parsed_types` stack, and return it. + pub fn finish_parsing(&mut self) -> PartialType { + self.currently_parsed_types.pop() + .expect("should have been parsing a type, if we finished parsing a type") + } + /// Get the user-provided type chooser by reference, if any. pub fn type_chooser(&self) -> Option<&TypeChooser> { self.options().type_chooser.as_ref().map(|t| &**t) @@ -1156,6 +1174,68 @@ impl<'ctx> BindgenContext<'ctx> { } } +/// A type that we are in the middle of parsing. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PartialType { + decl: Cursor, + id: ItemId, +} + +impl PartialType { + /// Construct a new `PartialType`. + pub fn new(decl: Cursor, id: ItemId) -> PartialType { + // assert!(decl == decl.canonical()); + PartialType { + decl: decl, + id: id, + } + } + + /// The cursor pointing to this partial type's declaration location. + pub fn decl(&self) -> &Cursor { + &self.decl + } + + /// The item ID allocated for this type. This is *NOT* a key for an entry in + /// the context's item set yet! + pub fn id(&self) -> ItemId { + self.id + } +} + +impl TemplateDeclaration for PartialType { + fn template_params(&self, _ctx: &BindgenContext) -> Option> { + // Maybe at some point we will eagerly parse named types, but for now we + // don't and this information is unavailable. + None + } + + fn num_template_params(&self, _ctx: &BindgenContext) -> Option { + // Wouldn't it be nice if libclang would reliably give us this + // information‽ + match self.decl().kind() { + clang_sys::CXCursor_ClassTemplate | + clang_sys::CXCursor_FunctionTemplate | + clang_sys::CXCursor_TypeAliasTemplateDecl => { + let mut num_params = 0; + self.decl().visit(|c| { + match c.kind() { + clang_sys::CXCursor_TemplateTypeParameter | + clang_sys::CXCursor_TemplateTemplateParameter | + clang_sys::CXCursor_NonTypeTemplateParameter => { + num_params += 1; + } + _ => {} + }; + clang_sys::CXChildVisit_Continue + }); + Some(num_params) + } + _ => None, + } + } +} + /// An iterator over whitelisted items. /// /// See `BindgenContext::whitelisted_items` for more information. diff --git a/src/ir/item.rs b/src/ir/item.rs index 3ce228b65e..83e2a41f4e 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -1,7 +1,7 @@ //! Bindgen's core intermediate representation type. use super::annotations::Annotations; -use super::context::{BindgenContext, ItemId}; +use super::context::{BindgenContext, ItemId, PartialType}; use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault}; use super::function::Function; use super::item_kind::ItemKind; @@ -1202,18 +1202,18 @@ impl ClangItemParser for Item { }; if valid_decl { - if let Some(&(_, item_id)) = - ctx.currently_parsed_types - .iter() - .find(|&&(d, _)| d == declaration_to_look_for) { + if let Some(partial) = ctx.currently_parsed_types() + .iter() + .find(|ty| *ty.decl() == declaration_to_look_for) { debug!("Avoiding recursion parsing type: {:?}", ty); - return Ok(item_id); + return Ok(partial.id()); } } let current_module = ctx.current_module(); + let partial_ty = PartialType::new(declaration_to_look_for, id); if valid_decl { - ctx.currently_parsed_types.push((declaration_to_look_for, id)); + ctx.begin_parsing(partial_ty); } let result = Type::from_clang_ty(id, ty, location, parent_id, ctx); @@ -1241,9 +1241,8 @@ impl ClangItemParser for Item { // declaration_to_look_for suspiciously shares a lot of // logic with ir::context, so we should refactor that. if valid_decl { - let (popped_decl, _) = - ctx.currently_parsed_types.pop().unwrap(); - assert_eq!(popped_decl, declaration_to_look_for); + let finished = ctx.finish_parsing(); + assert_eq!(*finished.decl(), declaration_to_look_for); } location.visit(|cur| { @@ -1251,8 +1250,9 @@ impl ClangItemParser for Item { }); if valid_decl { - ctx.currently_parsed_types - .push((declaration_to_look_for, id)); + let partial_ty = + PartialType::new(declaration_to_look_for, id); + ctx.begin_parsing(partial_ty); } } // If we have recursed into the AST all we know, and we still @@ -1279,8 +1279,8 @@ impl ClangItemParser for Item { }; if valid_decl { - let (popped_decl, _) = ctx.currently_parsed_types.pop().unwrap(); - assert_eq!(popped_decl, declaration_to_look_for); + let partial_ty = ctx.finish_parsing(); + assert_eq!(*partial_ty.decl(), declaration_to_look_for); } ret diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 0c90547e09..a98ca446a7 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -755,7 +755,7 @@ impl Type { potential_id, ty, location); - debug!("currently_parsed_types: {:?}", ctx.currently_parsed_types); + debug!("currently_parsed_types: {:?}", ctx.currently_parsed_types()); let canonical_ty = ty.canonical_type(); From 4348fb0c4c7e558705d573bf11d3d8262ccf39fc Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 14:54:02 -0800 Subject: [PATCH 08/12] Refactor how template instantiation is performed This commit renames `build_template_wrapper` to `instantiate_template` because that is what it is really doing. Additionally, it completely reworks its logic. Sometimes clang gives us rather sorry ASTs for template instantiations (particularly when they involve incomplete template declarations) and we need to manually reconstruct the template argument nesting. --- src/ir/context.rs | 377 +++++++++++++++++++++++++++++----------------- 1 file changed, 240 insertions(+), 137 deletions(-) diff --git a/src/ir/context.rs b/src/ir/context.rs index a2277b398f..f25eaf3ffb 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -11,12 +11,14 @@ use BindgenOptions; use cexpr; use chooser::TypeChooser; use clang::{self, Cursor}; +use clang_sys; use parse::ClangItemParser; use std::borrow::Cow; use std::cell::Cell; use std::collections::{HashMap, VecDeque, hash_map}; use std::collections::btree_map::{self, BTreeMap}; use std::fmt; +use std::iter::IntoIterator; use syntax::ast::Ident; use syntax::codemap::{DUMMY_SP, Span}; use syntax::ext::base::ExtCtxt; @@ -618,119 +620,239 @@ impl<'ctx> BindgenContext<'ctx> { self.current_module } - /// This is one of the hackiest methods in all the parsing code. This method - /// is used to allow having templates with another argument names instead of - /// the canonical ones. + /// Given a cursor pointing to the location of a template instantiation, + /// return a tuple of the form `(declaration_cursor, declaration_id, + /// num_expected_template_args)`. + /// + /// Note that `declaration_id` is not guaranteed to be in the context's item + /// set! It is possible that it is a partial type that we are still in the + /// middle of parsign. + fn get_declaration_info_for_template_instantiation + (&self, + instantiation: &Cursor) + -> (Cursor, ItemId, usize) { + instantiation.cur_type() + .canonical_declaration(Some(instantiation)) + .and_then(|canon_decl| { + self.get_resolved_type(&canon_decl) + .and_then(|template_decl_id| { + template_decl_id + .num_template_params(self) + .map(|num_params| { + (*canon_decl.cursor(), template_decl_id, num_params) + }) + }) + }) + .unwrap_or_else(|| { + // If we haven't already parsed the declaration of + // the template being instantiated, then it *must* + // be on the stack of types we are currently + // parsing. If it wasn't then clang would have + // already errored out before we started + // constructing our IR because you can't instantiate + // a template until it is fully defined. + let referenced = instantiation.referenced() + .expect("TemplateRefs should reference a template declaration"); + let template_decl = self.currently_parsed_types() + .iter() + .find(|partial_ty| *partial_ty.decl() == referenced) + .cloned() + .expect("template decl must be on the parse stack"); + let num_template_params = template_decl + .num_template_params(self) + .expect("and it had better be a template declaration"); + (*template_decl.decl(), template_decl.id(), num_template_params) + }) + } + + /// Parse a template instantiation, eg `Foo`. /// /// This is surprisingly difficult to do with libclang, due to the fact that - /// partial template specializations don't provide explicit template - /// argument information. + /// it doesn't provide explicit template argument information, except for + /// function template declarations(!?!??!). /// - /// The only way to do this as far as I know, is inspecting manually the - /// AST, looking for TypeRefs inside. This, unfortunately, doesn't work for + /// The only way to do this is manually inspecting the AST and looking for + /// TypeRefs and TemplateRefs inside. This, unfortunately, doesn't work for /// more complex cases, see the comment on the assertion below. /// - /// To see an example of what this handles: + /// To add insult to injury, the AST itself has structure that doesn't make + /// sense. Sometimes `Foo>` has an AST with nesting like you might + /// expect: `(Foo (Bar (int)))`. Other times, the AST we get is completely + /// flat: `(Foo Bar int)`. + /// + /// To see an example of what this method handles: /// /// ```c++ - /// template - /// class Incomplete { - /// T p; - /// }; + /// template + /// class Incomplete { + /// T p; + /// }; /// - /// template - /// class Foo { - /// Incomplete bar; - /// }; + /// template + /// class Foo { + /// Incomplete bar; + /// }; /// ``` - fn build_template_wrapper(&mut self, - with_id: ItemId, - wrapping: ItemId, - parent_id: ItemId, - ty: &clang::Type, - location: clang::Cursor, - declaration: clang::Cursor) - -> ItemId { - use clang_sys::*; + fn instantiate_template(&mut self, + with_id: ItemId, + template: ItemId, + parent_id: ItemId, + ty: &clang::Type, + location: clang::Cursor) + -> ItemId { + use clang_sys; + + let num_expected_args = self.resolve_type(template) + .num_template_params(self) + .expect("template types should have template params"); + let mut args = vec![]; - location.visit(|c| { - if c.kind() == CXCursor_TypeRef { - // The `with_id` id will potentially end up unused if we give up - // on this type (for example, its a tricky partial template - // specialization), so if we pass `with_id` as the parent, it is - // potentially a dangling reference. Instead, use the canonical - // template declaration as the parent. It is already parsed and - // has a known-resolvable `ItemId`. - let new_ty = Item::from_ty_or_ref(c.cur_type(), - Some(c), - Some(wrapping), - self); - args.push(new_ty); + let mut found_const_arg = false; + + let mut children = location.collect_children(); + + if children.iter().all(|c| !c.has_children()) { + // This is insanity... If clang isn't giving us a properly nested + // AST for which template arguments belong to which template we are + // instantiating, we'll need to construct it ourselves. However, + // there is an extra `NamespaceRef, NamespaceRef, ..., TemplateRef` + // representing a reference to the outermost template declaration + // that we need to filter out of the children. We need to do this + // filtering because we already know which template declaration is + // being specialized via the `location`'s type, and if we do not + // filter it out, we'll add an extra layer of template instantiation + // on accident. + let idx = children.iter() + .position(|c| c.kind() == clang_sys::CXCursor_TemplateRef); + if let Some(idx) = idx { + if children.iter() + .take(idx) + .all(|c| c.kind() == clang_sys::CXCursor_NamespaceRef) { + children = children.into_iter().skip(idx + 1).collect(); + } } - CXChildVisit_Continue - }); - - let item = { - let wrapping_type = self.resolve_type(wrapping); - if let TypeKind::Comp(ref ci) = *wrapping_type.kind() { - let old_args = ci.template_args(); + } - // The following assertion actually fails with partial template - // specialization. But as far as I know there's no way at all to - // grab the specialized types from neither the AST or libclang, - // which sucks. The same happens for specialized type alias - // template declarations, where we have that ugly hack up there. - // - // This flaw was already on the old parser, but I now think it - // has no clear solution (apart from patching libclang to - // somehow expose them, of course). - // - // For an easy example in which there's no way at all of getting - // the `int` type, except manually parsing the spelling: - // - // template - // class Incomplete { - // T d; - // U p; - // }; - // - // template - // class Foo { - // Incomplete bar; - // }; - // - // debug_assert_eq!(old_args.len(), args.len()); - // - // That being said, this is not so common, so just error! and - // hope for the best, returning the previous type, who knows. - if old_args.len() != args.len() { - error!("Found partial template specialization, \ - expect dragons!"); - return wrapping; + for child in children.iter().rev() { + match child.kind() { + clang_sys::CXCursor_TypeRef => { + // The `with_id` id will potentially end up unused if we give up + // on this type (for example, because it has const value + // template args), so if we pass `with_id` as the parent, it is + // potentially a dangling reference. Instead, use the canonical + // template declaration as the parent. It is already parsed and + // has a known-resolvable `ItemId`. + let ty = Item::from_ty_or_ref(child.cur_type(), + Some(*child), + Some(template), + self); + args.push(ty); + } + clang_sys::CXCursor_TemplateRef => { + let (template_decl_cursor, template_decl_id, num_expected_template_args) = + self.get_declaration_info_for_template_instantiation(child); + + if num_expected_template_args == 0 || + child.has_at_least_num_children(num_expected_template_args) { + // Do a happy little parse. See comment in the TypeRef + // match arm about parent IDs. + let ty = Item::from_ty_or_ref(child.cur_type(), + Some(*child), + Some(template), + self); + args.push(ty); + } else { + // This is the case mentioned in the doc comment where + // clang gives us a flattened AST and we have to + // reconstruct which template arguments go to which + // instantiation :( + let args_len = args.len(); + assert!(args_len >= num_expected_template_args); + let mut sub_args: Vec<_> = + args.drain(args_len - num_expected_template_args..) + .collect(); + sub_args.reverse(); + + let sub_name = Some(template_decl_cursor.spelling()); + let sub_kind = + TypeKind::TemplateInstantiation(template_decl_id, + sub_args); + let sub_ty = Type::new(sub_name, + template_decl_cursor.cur_type() + .fallible_layout() + .ok(), + sub_kind, + false); + let sub_id = self.next_item_id(); + let sub_item = Item::new(sub_id, + None, + None, + template_decl_id, + ItemKind::Type(sub_ty)); + + // Bypass all the validations in add_item explicitly. + debug!("instantiate_template: inserting nested \ + instantiation item: {:?}", + sub_item); + debug_assert!(sub_id == sub_item.id()); + self.items.insert(sub_id, sub_item); + args.push(sub_id); + } + } + _ => { + warn!("Found template arg cursor we can't handle: {:?}", + child); + found_const_arg = true; } - } else { - assert_eq!(declaration.kind(), - ::clang_sys::CXCursor_TypeAliasTemplateDecl, - "Expected wrappable type"); } + } - let type_kind = TypeKind::TemplateRef(wrapping, args); - let name = ty.spelling(); - let name = if name.is_empty() { None } else { Some(name) }; - let ty = Type::new(name, - ty.fallible_layout().ok(), - type_kind, - ty.is_const()); - Item::new(with_id, None, None, parent_id, ItemKind::Type(ty)) - }; + assert!(args.len() <= num_expected_args); + if found_const_arg || args.len() < num_expected_args { + // This is a dependently typed template instantiation. That is, an + // instantiation of a template with one or more const values as + // template arguments, rather than only types as template + // arguments. For example, `Foo` versus `Bar`. + // We can't handle these instantiations, so just punt in this + // situation... + warn!("Found template instantiated with a const value; \ + bindgen can't handle this kind of template instantiation!"); + return template; + } + + args.reverse(); + let type_kind = TypeKind::TemplateInstantiation(template, args); + let name = ty.spelling(); + let name = if name.is_empty() { None } else { Some(name) }; + let ty = Type::new(name, + ty.fallible_layout().ok(), + type_kind, + ty.is_const()); + let item = + Item::new(with_id, None, None, parent_id, ItemKind::Type(ty)); // Bypass all the validations in add_item explicitly. - debug!("build_template_wrapper: inserting item: {:?}", item); + debug!("instantiate_template: inserting item: {:?}", item); debug_assert!(with_id == item.id()); self.items.insert(with_id, item); with_id } + /// If we have already resolved the type for the given type declaration, + /// return its `ItemId`. Otherwise, return `None`. + fn get_resolved_type(&self, + decl: &clang::CanonicalTypeDeclaration) + -> Option { + self.types + .get(&TypeKey::Declaration(*decl.cursor())) + .or_else(|| { + decl.cursor() + .usr() + .and_then(|usr| self.types.get(&TypeKey::USR(usr))) + }) + .cloned() + } + /// Looks up for an already resolved type, either because it's builtin, or /// because we already have it in the map. pub fn builtin_or_resolved_ty(&mut self, @@ -744,45 +866,32 @@ impl<'ctx> BindgenContext<'ctx> { ty, location, parent_id); - let mut declaration = ty.declaration(); - if !declaration.is_valid() { - if let Some(location) = location { - if location.is_template_like() { - declaration = location; - } - } - } - let canonical_declaration = declaration.canonical(); - if canonical_declaration.is_valid() { - let id = self.types - .get(&TypeKey::Declaration(canonical_declaration)) - .map(|id| *id) - .or_else(|| { - canonical_declaration.usr() - .and_then(|usr| self.types.get(&TypeKey::USR(usr))) - .map(|id| *id) - }); - if let Some(id) = id { + + if let Some(decl) = ty.canonical_declaration(location.as_ref()) { + if let Some(id) = self.get_resolved_type(&decl) { debug!("Already resolved ty {:?}, {:?}, {:?} {:?}", id, - declaration, + decl, ty, location); - - // If the declaration existed, we *might* be done, but it's not - // the case for class templates, where the template arguments - // may vary. + // If the declaration already exists, then either: // - // In this case, we create a TemplateRef with the new template - // arguments, pointing to the canonical template. + // * the declaration is a template declaration of some sort, + // and we are looking at an instantiation or specialization + // of it, or + // * we have already parsed and resolved this type, and + // there's nothing left to do. // - // Note that we only do it if parent_id is some, and we have a - // location for building the new arguments, the template - // argument names don't matter in the global context. - if declaration.is_template_like() && - *ty != canonical_declaration.cur_type() && + // Note that we only do the former if the `parent_id` exists, + // and we have a location for building the new arguments. The + // template argument names don't matter in the global context. + if decl.cursor().is_template_like() && + *ty != decl.cursor().cur_type() && location.is_some() && parent_id.is_some() { + let location = location.unwrap(); + let parent_id = parent_id.unwrap(); + // 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). @@ -793,18 +902,17 @@ impl<'ctx> BindgenContext<'ctx> { // exposed. // // This is _tricky_, I know :( - if declaration.kind() == CXCursor_TypeAliasTemplateDecl && - !location.unwrap().contains_cursor(CXCursor_TypeRef) && + if decl.cursor().kind() == CXCursor_TypeAliasTemplateDecl && + !location.contains_cursor(CXCursor_TypeRef) && ty.canonical_type().is_valid_and_exposed() { return None; } - return Some(self.build_template_wrapper(with_id, - id, - parent_id.unwrap(), - ty, - location.unwrap(), - declaration)); + return Some(self.instantiate_template(with_id, + id, + parent_id, + ty, + location)); } return Some(self.build_ty_wrapper(with_id, id, parent_id, ty)); @@ -812,9 +920,7 @@ impl<'ctx> BindgenContext<'ctx> { } debug!("Not resolved, maybe builtin?"); - - // Else, build it. - self.build_builtin_ty(ty, declaration) + self.build_builtin_ty(ty) } // This is unfortunately a lot of bloat, but is needed to properly track @@ -849,10 +955,7 @@ impl<'ctx> BindgenContext<'ctx> { ret } - fn build_builtin_ty(&mut self, - ty: &clang::Type, - _declaration: Cursor) - -> Option { + fn build_builtin_ty(&mut self, ty: &clang::Type) -> Option { use clang_sys::*; let type_kind = match ty.kind() { CXType_NullPtr => TypeKind::NullPtr, From de35b8a4c510722480c7af2e02f4322aad45ec55 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 15:02:56 -0800 Subject: [PATCH 09/12] Add a test case for issue #446 --- tests/expectations/tests/issue-446.rs | 22 ++++++++++++++++++++++ tests/headers/issue-446.hpp | 11 +++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/expectations/tests/issue-446.rs create mode 100644 tests/headers/issue-446.hpp diff --git a/tests/expectations/tests/issue-446.rs b/tests/expectations/tests/issue-446.rs new file mode 100644 index 0000000000..fa736bccf6 --- /dev/null +++ b/tests/expectations/tests/issue-446.rs @@ -0,0 +1,22 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct List { + pub next: *mut List, +} +impl Default for List { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct PersistentRooted { + pub root_list: List>, +} +impl Default for PersistentRooted { + fn default() -> Self { unsafe { ::std::mem::zeroed() } } +} diff --git a/tests/headers/issue-446.hpp b/tests/headers/issue-446.hpp new file mode 100644 index 0000000000..2e09c2745b --- /dev/null +++ b/tests/headers/issue-446.hpp @@ -0,0 +1,11 @@ +// bindgen-flags: -- -std=c++14 + +template +class List { + List *next; +}; + +template +class PersistentRooted { + List> root_list; +}; From 41a109ac25d24863dc5d70cc7e96857a0a9de54c Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 7 Feb 2017 15:13:06 -0800 Subject: [PATCH 10/12] Run `cargo fmt` --- src/clang.rs | 17 +++++++++++++---- src/ir/function.rs | 5 +++-- src/ir/objc.rs | 4 ++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/clang.rs b/src/clang.rs index 86105fc1fc..15cd60fc1d 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -1464,7 +1464,10 @@ pub fn ast_dump(c: &Cursor, depth: isize) -> CXChildVisitResult { type_to_str(ty.kind()))); } if let Some(ty) = c.ret_type() { - print_indent(depth, format!(" {}ret-type = {}", prefix, type_to_str(ty.kind()))); + print_indent(depth, + format!(" {}ret-type = {}", + prefix, + type_to_str(ty.kind()))); } if let Some(refd) = c.referenced() { @@ -1473,7 +1476,9 @@ pub fn ast_dump(c: &Cursor, depth: isize) -> CXChildVisitResult { print_cursor(depth, String::from(prefix) + "referenced.", &refd); - print_cursor(depth, String::from(prefix) + "referenced.", &refd); + print_cursor(depth, + String::from(prefix) + "referenced.", + &refd); } } @@ -1483,7 +1488,9 @@ pub fn ast_dump(c: &Cursor, depth: isize) -> CXChildVisitResult { print_cursor(depth, String::from(prefix) + "canonical.", &canonical); - print_cursor(depth, String::from(prefix) + "canonical.", &canonical); + print_cursor(depth, + String::from(prefix) + "canonical.", + &canonical); } if let Some(specialized) = c.specialized() { @@ -1492,7 +1499,9 @@ pub fn ast_dump(c: &Cursor, depth: isize) -> CXChildVisitResult { print_cursor(depth, String::from(prefix) + "specialized.", &specialized); - print_cursor(depth, String::from(prefix) + "specialized.", &specialized); + print_cursor(depth, + String::from(prefix) + "specialized.", + &specialized); } } } diff --git a/src/ir/function.rs b/src/ir/function.rs index 9ef9c3a73c..6273de283b 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -229,8 +229,9 @@ impl FunctionSig { let abi = get_abi(ty.call_conv()); if abi.is_none() { - assert_eq!(cursor.kind(), CXCursor_ObjCInstanceMethodDecl, - "Invalid ABI for function signature") + assert_eq!(cursor.kind(), + CXCursor_ObjCInstanceMethodDecl, + "Invalid ABI for function signature") } Ok(Self::new(ret, args, ty.is_variadic(), abi)) diff --git a/src/ir/objc.rs b/src/ir/objc.rs index ea2cc0c887..b3c3688b37 100644 --- a/src/ir/objc.rs +++ b/src/ir/objc.rs @@ -1,10 +1,10 @@ //! Objective C types +use super::context::BindgenContext; +use super::function::FunctionSig; use clang; use clang_sys::CXChildVisit_Continue; use clang_sys::CXCursor_ObjCInstanceMethodDecl; -use super::context::BindgenContext; -use super::function::FunctionSig; /// Objective C interface as used in TypeKind /// From a8fedb498cf8bac29cea76f19ddc0d813a2ca9d3 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 8 Feb 2017 11:08:09 -0800 Subject: [PATCH 11/12] Make instantiate_template fallible --- src/ir/context.rs | 82 ++++++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/src/ir/context.rs b/src/ir/context.rs index f25eaf3ffb..ebb6520b1f 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -630,7 +630,7 @@ impl<'ctx> BindgenContext<'ctx> { fn get_declaration_info_for_template_instantiation (&self, instantiation: &Cursor) - -> (Cursor, ItemId, usize) { + -> Option<(Cursor, ItemId, usize)> { instantiation.cur_type() .canonical_declaration(Some(instantiation)) .and_then(|canon_decl| { @@ -643,7 +643,7 @@ impl<'ctx> BindgenContext<'ctx> { }) }) }) - .unwrap_or_else(|| { + .or_else(|| { // If we haven't already parsed the declaration of // the template being instantiated, then it *must* // be on the stack of types we are currently @@ -651,17 +651,19 @@ impl<'ctx> BindgenContext<'ctx> { // already errored out before we started // constructing our IR because you can't instantiate // a template until it is fully defined. - let referenced = instantiation.referenced() - .expect("TemplateRefs should reference a template declaration"); - let template_decl = self.currently_parsed_types() - .iter() - .find(|partial_ty| *partial_ty.decl() == referenced) - .cloned() - .expect("template decl must be on the parse stack"); - let num_template_params = template_decl - .num_template_params(self) - .expect("and it had better be a template declaration"); - (*template_decl.decl(), template_decl.id(), num_template_params) + instantiation.referenced() + .and_then(|referenced| { + self.currently_parsed_types() + .iter() + .find(|partial_ty| *partial_ty.decl() == referenced) + .cloned() + }) + .and_then(|template_decl| { + template_decl.num_template_params(self) + .map(|num_template_params| { + (*template_decl.decl(), template_decl.id(), num_template_params) + }) + }) }) } @@ -699,16 +701,20 @@ impl<'ctx> BindgenContext<'ctx> { parent_id: ItemId, ty: &clang::Type, location: clang::Cursor) - -> ItemId { + -> Option { use clang_sys; - let num_expected_args = self.resolve_type(template) - .num_template_params(self) - .expect("template types should have template params"); + let num_expected_args = match self.resolve_type(template).num_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 mut args = vec![]; let mut found_const_arg = false; - let mut children = location.collect_children(); if children.iter().all(|c| !c.has_children()) { @@ -735,7 +741,9 @@ impl<'ctx> BindgenContext<'ctx> { for child in children.iter().rev() { match child.kind() { - clang_sys::CXCursor_TypeRef => { + clang_sys::CXCursor_TypeRef | + clang_sys::CXCursor_TypedefDecl | + clang_sys::CXCursor_TypeAliasDecl => { // The `with_id` id will potentially end up unused if we give up // on this type (for example, because it has const value // template args), so if we pass `with_id` as the parent, it is @@ -750,7 +758,10 @@ impl<'ctx> BindgenContext<'ctx> { } clang_sys::CXCursor_TemplateRef => { let (template_decl_cursor, template_decl_id, num_expected_template_args) = - self.get_declaration_info_for_template_instantiation(child); + match self.get_declaration_info_for_template_instantiation(child) { + Some(info) => info, + None => return None, + }; if num_expected_template_args == 0 || child.has_at_least_num_children(num_expected_template_args) { @@ -767,7 +778,12 @@ impl<'ctx> BindgenContext<'ctx> { // reconstruct which template arguments go to which // instantiation :( let args_len = args.len(); - assert!(args_len >= num_expected_template_args); + if args_len < num_expected_template_args { + warn!("Found a template instantiation without \ + enough template arguments"); + return None; + } + let mut sub_args: Vec<_> = args.drain(args_len - num_expected_template_args..) .collect(); @@ -807,8 +823,7 @@ impl<'ctx> BindgenContext<'ctx> { } } - assert!(args.len() <= num_expected_args); - if found_const_arg || args.len() < num_expected_args { + if found_const_arg { // This is a dependently typed template instantiation. That is, an // instantiation of a template with one or more const values as // template arguments, rather than only types as template @@ -817,7 +832,13 @@ impl<'ctx> BindgenContext<'ctx> { // situation... warn!("Found template instantiated with a const value; \ bindgen can't handle this kind of template instantiation!"); - return template; + return None; + } + + if args.len() != num_expected_args { + warn!("Found a template with an unexpected number of template \ + arguments"); + return None; } args.reverse(); @@ -835,7 +856,7 @@ impl<'ctx> BindgenContext<'ctx> { debug!("instantiate_template: inserting item: {:?}", item); debug_assert!(with_id == item.id()); self.items.insert(with_id, item); - with_id + Some(with_id) } /// If we have already resolved the type for the given type declaration, @@ -908,11 +929,12 @@ impl<'ctx> BindgenContext<'ctx> { return None; } - return Some(self.instantiate_template(with_id, - id, - parent_id, - ty, - location)); + return self.instantiate_template(with_id, + id, + parent_id, + ty, + location) + .or_else(|| Some(id)); } return Some(self.build_ty_wrapper(with_id, id, parent_id, ty)); From 5b5d4f8783e82432441c11691712d57c3af5844a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 8 Feb 2017 11:08:33 -0800 Subject: [PATCH 12/12] Run `cargo fmt` --- src/codegen/helpers.rs | 4 +--- src/codegen/mod.rs | 11 ++++++----- src/ir/context.rs | 26 +++++++++++++++----------- src/ir/derive.rs | 2 +- src/main.rs | 7 +++---- src/options.rs | 5 +++-- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/codegen/helpers.rs b/src/codegen/helpers.rs index 9e19637c50..06dadab0c9 100644 --- a/src/codegen/helpers.rs +++ b/src/codegen/helpers.rs @@ -89,9 +89,7 @@ pub mod ast_ty { let prefix = ctx.rust_ident_raw(prefix); quote_ty!(ctx.ext_cx(), $prefix::$ident) } - None => { - quote_ty!(ctx.ext_cx(), ::std::os::raw::$ident) - } + None => quote_ty!(ctx.ext_cx(), ::std::os::raw::$ident), } } diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 0d605c1482..7469abb981 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1377,9 +1377,10 @@ impl CodeGenerator for CompInfo { // FIXME when [issue #465](https://github.com/servo/rust-bindgen/issues/465) ready let too_many_base_vtables = self.base_members() .iter() - .filter(|base| ctx.resolve_type(base.ty).has_vtable(ctx)) - .count() > - 1; + .filter(|base| { + ctx.resolve_type(base.ty).has_vtable(ctx) + }) + .count() > 1; let should_skip_field_offset_checks = item.is_opaque(ctx) || too_many_base_vtables; @@ -2189,7 +2190,7 @@ impl ToRustTy for Type { .map(|arg| arg.to_rust_ty(ctx)) .collect::>(); - path.segments.last_mut().unwrap().parameters = if + path.segments.last_mut().unwrap().parameters = if template_args.is_empty() { None } else { @@ -2509,8 +2510,8 @@ mod utils { use super::ItemToRustTy; use aster; use ir::context::{BindgenContext, ItemId}; - use ir::item::{Item, ItemCanonicalPath}; use ir::function::FunctionSig; + use ir::item::{Item, ItemCanonicalPath}; use ir::ty::TypeKind; use std::mem; use syntax::ast; diff --git a/src/ir/context.rs b/src/ir/context.rs index ebb6520b1f..91ce579c33 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -636,10 +636,11 @@ impl<'ctx> BindgenContext<'ctx> { .and_then(|canon_decl| { self.get_resolved_type(&canon_decl) .and_then(|template_decl_id| { - template_decl_id - .num_template_params(self) + template_decl_id.num_template_params(self) .map(|num_params| { - (*canon_decl.cursor(), template_decl_id, num_params) + (*canon_decl.cursor(), + template_decl_id, + num_params) }) }) }) @@ -661,7 +662,9 @@ impl<'ctx> BindgenContext<'ctx> { .and_then(|template_decl| { template_decl.num_template_params(self) .map(|num_template_params| { - (*template_decl.decl(), template_decl.id(), num_template_params) + (*template_decl.decl(), + template_decl.id(), + num_template_params) }) }) }) @@ -704,7 +707,8 @@ impl<'ctx> BindgenContext<'ctx> { -> Option { use clang_sys; - let num_expected_args = match self.resolve_type(template).num_template_params(self) { + let num_expected_args = match self.resolve_type(template) + .num_template_params(self) { Some(n) => n, None => { warn!("Tried to instantiate a template for which we could not \ @@ -929,12 +933,12 @@ impl<'ctx> BindgenContext<'ctx> { return None; } - return self.instantiate_template(with_id, - id, - parent_id, - ty, - location) - .or_else(|| Some(id)); + return self.instantiate_template(with_id, + id, + parent_id, + ty, + location) + .or_else(|| Some(id)); } return Some(self.build_ty_wrapper(with_id, id, parent_id, ty)); diff --git a/src/ir/derive.rs b/src/ir/derive.rs index 6d9f368ba2..0fc8debffa 100644 --- a/src/ir/derive.rs +++ b/src/ir/derive.rs @@ -85,4 +85,4 @@ pub trait CanDeriveDefault { ctx: &BindgenContext, extra: Self::Extra) -> bool; -} \ No newline at end of file +} diff --git a/src/main.rs b/src/main.rs index 68f3d0a865..fc1ef8e28c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -44,10 +44,9 @@ pub fn main() { match builder_from_flags(bind_args.into_iter()) { Ok((builder, output, verbose)) => { - let builder_result = - panic::catch_unwind(|| - builder.generate().expect("Unable to generate bindings") - ); + let builder_result = panic::catch_unwind(|| { + builder.generate().expect("Unable to generate bindings") + }); if builder_result.is_err() { if verbose { diff --git a/src/options.rs b/src/options.rs index 7a6fde964e..49bad841c3 100644 --- a/src/options.rs +++ b/src/options.rs @@ -4,8 +4,9 @@ use std::fs::File; use std::io::{self, Error, ErrorKind}; /// Construct a new [`Builder`](./struct.Builder.html) from command line flags. -pub fn builder_from_flags(args: I) - -> Result<(Builder, Box, bool), io::Error> +pub fn builder_from_flags + (args: I) + -> Result<(Builder, Box, bool), io::Error> where I: Iterator, { let matches = App::new("bindgen")