diff options
| -rw-r--r-- | README.md | 2 | ||||
| -rwxr-xr-x | src/clang.rs | 8 | ||||
| -rw-r--r-- | src/ir/annotations.rs | 2 | ||||
| -rw-r--r-- | src/ir/context.rs | 23 | ||||
| -rw-r--r-- | src/ir/item.rs | 42 | ||||
| -rw-r--r-- | src/ir/ty.rs | 44 | ||||
| -rw-r--r-- | src/ir/var.rs | 15 | ||||
| -rwxr-xr-x | src/lib.rs | 2 | ||||
| -rw-r--r-- | src/regex_set.rs | 8 | ||||
| -rw-r--r-- | tests/expectations/elaborated.rs | 11 | ||||
| -rw-r--r-- | tests/expectations/empty_template_param_name.rs | 1 | ||||
| -rw-r--r-- | tests/expectations/type_alias_empty.rs | 7 | ||||
| -rw-r--r-- | tests/headers/elaborated.hpp | 5 | ||||
| -rw-r--r-- | tests/headers/type_alias_empty.hpp | 10 | ||||
| -rw-r--r-- | tests/tests.rs | 15 |
15 files changed, 139 insertions, 56 deletions
@@ -26,7 +26,7 @@ with more features (such as detection of inlined functions). ``` # apt-get install llvm-3.8-dev libclang-3.8-dev ``` - +Adding the LLVM repos to get version 3.8 may be necessary, see http://apt.llvm.org/. #### Arch ``` diff --git a/src/clang.rs b/src/clang.rs index 40cd0ad7..77cead95 100755 --- a/src/clang.rs +++ b/src/clang.rs @@ -796,9 +796,11 @@ impl Comment { } /// Get this comment's `idx`th child comment - pub fn get_child(&self, idx: c_uint) -> Comment { - unsafe { - Comment { x: clang_Comment_getChild(self.x, idx) } + pub fn get_child(&self, idx: c_uint) -> Option<Comment> { + if idx >= self.num_children() { + None + } else { + Some(Comment { x: unsafe { clang_Comment_getChild(self.x, idx) } }) } } diff --git a/src/ir/annotations.rs b/src/ir/annotations.rs index f0d9715e..d276608f 100644 --- a/src/ir/annotations.rs +++ b/src/ir/annotations.rs @@ -153,7 +153,7 @@ impl Annotations { } for i in 0..comment.num_children() { - self.parse(&comment.get_child(i), matched); + self.parse(&comment.get_child(i).unwrap(), matched); } } } diff --git a/src/ir/context.rs b/src/ir/context.rs index 77cd1182..65c62817 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -467,6 +467,7 @@ impl<'ctx> BindgenContext<'ctx> { /// }; /// ``` fn build_template_wrapper(&mut self, + with_id: ItemId, wrapping: ItemId, parent_id: ItemId, ty: &clang::Type, @@ -474,7 +475,6 @@ impl<'ctx> BindgenContext<'ctx> { use clangll::*; let mut args = vec![]; let mut found_invalid_template_ref = false; - let self_id = ItemId::next(); location.visit(|c, _| { if c.kind() == CXCursor_TemplateRef && c.cur_type().kind() == CXType_Invalid { @@ -482,7 +482,7 @@ impl<'ctx> BindgenContext<'ctx> { } if c.kind() == CXCursor_TypeRef { let new_ty = - Item::from_ty_or_ref(c.cur_type(), Some(*c), Some(self_id), self); + Item::from_ty_or_ref(c.cur_type(), Some(*c), Some(with_id), self); args.push(new_ty); } CXChildVisit_Continue @@ -528,17 +528,18 @@ impl<'ctx> BindgenContext<'ctx> { 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(self_id, None, None, parent_id, ItemKind::Type(ty)) + Item::new(with_id, None, None, parent_id, ItemKind::Type(ty)) }; // Bypass all the validations in add_item explicitly. - self.items.insert(self_id, item); - self_id + self.items.insert(with_id, item); + with_id } /// 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, + with_id: ItemId, parent_id: Option<ItemId>, ty: &clang::Type, location: Option<clang::Cursor>) -> Option<ItemId> { @@ -567,6 +568,7 @@ impl<'ctx> BindgenContext<'ctx> { if let Some(id) = id { debug!("Already resolved ty {:?}, {:?}, {:?} {:?}", id, declaration, 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. @@ -582,11 +584,12 @@ impl<'ctx> BindgenContext<'ctx> { *ty != canonical_declaration.cur_type() && location.is_some() && parent_id.is_some() { return Some( - self.build_template_wrapper(id, parent_id.unwrap(), ty, + self.build_template_wrapper(with_id, id, + parent_id.unwrap(), ty, location.unwrap())); } - return Some(self.build_ty_wrapper(id, parent_id, ty)); + return Some(self.build_ty_wrapper(with_id, id, parent_id, ty)); } } @@ -602,19 +605,19 @@ impl<'ctx> BindgenContext<'ctx> { // We should probably make the constness tracking separate, so it doesn't // bloat that much, but hey, we already bloat the heck out of builtin types. fn build_ty_wrapper(&mut self, + with_id: ItemId, wrapped_id: ItemId, parent_id: Option<ItemId>, ty: &clang::Type) -> ItemId { - let id = ItemId::next(); let spelling = ty.spelling(); let is_const = ty.is_const(); let layout = ty.fallible_layout().ok(); let type_kind = TypeKind::ResolvedTypeRef(wrapped_id); let ty = Type::new(Some(spelling), layout, type_kind, is_const); - let item = Item::new(id, None, None, + let item = Item::new(with_id, None, None, parent_id.unwrap_or(self.current_module), ItemKind::Type(ty)); self.add_builtin_item(item); - id + with_id } fn build_builtin_ty(&mut self, diff --git a/src/ir/item.rs b/src/ir/item.rs index c68ed5f7..0d409f04 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -7,7 +7,7 @@ use super::ty::{Type, TypeKind}; use super::function::Function; use super::module::Module; use super::annotations::Annotations; -use std::cell::Cell; +use std::cell::{Cell, RefCell}; use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; use parse::{ClangItemParser, ClangSubItemParser, ParseError, ParseResult}; use clang; @@ -97,11 +97,25 @@ impl ItemCanonicalPath for ItemId { pub struct Item { /// This item's id. id: ItemId, - /// The item's local id, unique only amongst its siblings. Only used - /// for anonymous items. Lazily initialized in local_id(). + + /// The item's local id, unique only amongst its siblings. Only used for + /// anonymous items. + /// + /// Lazily initialized in local_id(). + /// + /// Note that only structs, unions, and enums get a local type id. In any + /// case this is an implementation detail. local_id: Cell<Option<usize>>, - /// The next local id to use for a child. + + /// The next local id to use for a child.. next_child_local_id: Cell<usize>, + + /// A cached copy of the canonical name, as returned by `canonical_name`. + /// + /// This is a fairly used operation during codegen so this makes bindgen + /// considerably faster in those cases. + canonical_name_cache: RefCell<Option<String>>, + /// A doc comment over the item, if any. comment: Option<String>, /// Annotations extracted from the doc comment, or the default ones @@ -129,6 +143,7 @@ impl Item { id: id, local_id: Cell::new(None), next_child_local_id: Cell::new(1), + canonical_name_cache: RefCell::new(None), parent_id: parent_id, comment: comment, annotations: annotations.unwrap_or_default(), @@ -536,6 +551,10 @@ impl Item { _ => {} } } + + // Note that this `id_` prefix prevents (really unlikely) collisions + // between the global id and the local id of an item with the same + // parent. format!("id_{}", self.id().0) } @@ -717,7 +736,9 @@ impl ClangItemParser for Item { .expect("Unable to resolve type"); } - if let Some(ty) = context.builtin_or_resolved_ty(parent_id, &ty, location) { + if let Some(ty) = context.builtin_or_resolved_ty(potential_id, + parent_id, &ty, + location) { debug!("{:?} already resolved: {:?}", ty, location); return ty; } @@ -779,7 +800,8 @@ impl ClangItemParser for Item { context.replace(replaced, id); } - if let Some(ty) = context.builtin_or_resolved_ty(parent_id, ty, location) { + if let Some(ty) = context.builtin_or_resolved_ty(id, parent_id, + ty, location) { return Ok(ty); } @@ -917,7 +939,13 @@ impl ItemCanonicalName for Item { if let Some(other_canon_type) = self.annotations.use_instead_of() { return other_canon_type.to_owned(); } - self.real_canonical_name(ctx, ctx.options().enable_cxx_namespaces, false) + if self.canonical_name_cache.borrow().is_none() { + *self.canonical_name_cache.borrow_mut() = + Some(self.real_canonical_name(ctx, + ctx.options().enable_cxx_namespaces, + false)); + } + return self.canonical_name_cache.borrow().as_ref().unwrap().clone(); } } diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 46a8aa20..c55d7622 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -122,13 +122,10 @@ impl Type { } /// Is this an integer type? - pub fn is_integer(&self, ctx: &BindgenContext) -> bool { + pub fn is_integer(&self) -> bool { match self.kind { - TypeKind::UnresolvedTypeRef(..) => false, - _ => match self.canonical_type(ctx).kind { - TypeKind::Int(..) => true, - _ => false, - } + TypeKind::Int(..) => true, + _ => false, } } @@ -311,12 +308,17 @@ impl Type { } } + /// See safe_canonical_type. + pub fn canonical_type<'tr>(&'tr self, ctx: &'tr BindgenContext) -> &'tr Type { + self.safe_canonical_type(ctx).expect("Should have been resolved after parsing!") + } + /// Returns the canonical type of this type, that is, the "inner type". /// /// For example, for a `typedef`, the canonical type would be the /// `typedef`ed type, for a template specialization, would be the template - /// its specializing, and so on. - pub fn canonical_type<'tr>(&'tr self, ctx: &'tr BindgenContext) -> &'tr Type { + /// its specializing, and so on. Return None if the type is unresolved. + pub fn safe_canonical_type<'tr>(&'tr self, ctx: &'tr BindgenContext) -> Option<&'tr Type> { match self.kind { TypeKind::Named(..) | TypeKind::Array(..) | @@ -329,16 +331,15 @@ impl Type { TypeKind::Void | TypeKind::NullPtr | TypeKind::BlockPointer | - TypeKind::Pointer(..) => self, + TypeKind::Pointer(..) => Some(self), TypeKind::ResolvedTypeRef(inner) | TypeKind::Alias(_, inner) | TypeKind::TemplateAlias(inner, _) | TypeKind::TemplateRef(inner, _) - => ctx.resolve_type(inner).canonical_type(ctx), + => ctx.resolve_type(inner).safe_canonical_type(ctx), - TypeKind::UnresolvedTypeRef(..) - => unreachable!("Should have been resolved after parsing!"), + TypeKind::UnresolvedTypeRef(..) => None, } } } @@ -470,7 +471,8 @@ impl Type { parent_id: Option<ItemId>, ctx: &mut BindgenContext) -> Result<ParseResult<Self>, ParseError> { use clangll::*; - if let Some(ty) = ctx.builtin_or_resolved_ty(parent_id, ty, location) { + if let Some(ty) = ctx.builtin_or_resolved_ty(potential_id, parent_id, + ty, location) { debug!("{:?} already resolved: {:?}", ty, location); return Ok(ParseResult::AlreadyResolved(ty)); } @@ -564,11 +566,17 @@ impl Type { return Err(ParseError::Continue); } - if args.is_empty() { - error!("Failed to get any template parameter, maybe a specialization? {:?}", location); - return Err(ParseError::Continue); - } - + // NB: `args` may be empty here (if for example the + // template parameters are constants). + // + // We can't reject it here then because inner points + // to `potential_id` now, so either we remove + // `inner` and return an error, or carry on. + // + // In this case, we just carry on, since it seems + // easier if than removing every possible reference + // to `item` from `ctx`, and it doesn't give any + // problems that we didn't have anyway. TypeKind::TemplateAlias(inner.unwrap(), args) } CXCursor_TemplateRef => { diff --git a/src/ir/var.rs b/src/ir/var.rs index 3dd975ee..e9245e1a 100644 --- a/src/ir/var.rs +++ b/src/ir/var.rs @@ -119,13 +119,14 @@ impl ClangSubItemParser for Var { // tests/headers/inner_const.hpp // // That's fine because in that case we know it's not a literal. - let value = context.safe_resolve_type(ty).map_or(None, |t| { - if t.is_integer(context) { - get_integer_literal_from_cursor(&cursor, context.translation_unit()) - } else { - None - } - }); + let value = context.safe_resolve_type(ty) + .and_then(|t| t.safe_canonical_type(context)).and_then(|t| { + if t.is_integer() { + get_integer_literal_from_cursor(&cursor, context.translation_unit()) + } else { + None + } + }); let mangling = cursor_mangling(&cursor); @@ -56,7 +56,7 @@ mod clangll; doc_mod!(clang); doc_mod!(ir); doc_mod!(parse); -mod regex_set; +doc_mod!(regex_set); mod codegen { include!(concat!(env!("OUT_DIR"), "/codegen.rs")); diff --git a/src/regex_set.rs b/src/regex_set.rs index 20bc56bf..ff899d78 100644 --- a/src/regex_set.rs +++ b/src/regex_set.rs @@ -1,18 +1,24 @@ +//! A type that represents the union of a set of regular expressions. + use std::borrow::Borrow; use regex::Regex; // Yeah, I'm aware this is sorta crappy, should be cheaper to compile a regex // ORing all the patterns, I guess... + +/// A dynamic set of regular expressions. #[derive(Debug)] pub struct RegexSet { items: Vec<Regex> } impl RegexSet { + /// Is this set empty? pub fn is_empty(&self) -> bool { self.items.is_empty() } + /// Extend this set with every regex in the iterator. pub fn extend<I>(&mut self, iter: I) where I: IntoIterator<Item=String> { @@ -21,6 +27,7 @@ impl RegexSet { } } + /// Insert a new regex into this set. pub fn insert<S>(&mut self, string: &S) where S: Borrow<str> { @@ -35,6 +42,7 @@ impl RegexSet { } } + /// Does the given `string` match any of the regexes in this set? pub fn matches<S>(&self, string: &S) -> bool where S: Borrow<str> { diff --git a/tests/expectations/elaborated.rs b/tests/expectations/elaborated.rs new file mode 100644 index 00000000..db373d41 --- /dev/null +++ b/tests/expectations/elaborated.rs @@ -0,0 +1,11 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +pub type whatever_t = ::std::os::raw::c_int; +extern "C" { + #[link_name = "_Z9somethingPKi"] + pub fn something(wat: *const whatever_t); +} diff --git a/tests/expectations/empty_template_param_name.rs b/tests/expectations/empty_template_param_name.rs index d165df80..e10b56db 100644 --- a/tests/expectations/empty_template_param_name.rs +++ b/tests/expectations/empty_template_param_name.rs @@ -4,6 +4,7 @@ #![allow(non_snake_case)] +pub type __void_t = ::std::os::raw::c_void; #[repr(C)] #[derive(Debug, Copy, Clone)] pub struct __iterator_traits<_Iterator> { diff --git a/tests/expectations/type_alias_empty.rs b/tests/expectations/type_alias_empty.rs new file mode 100644 index 00000000..b4b7b2bc --- /dev/null +++ b/tests/expectations/type_alias_empty.rs @@ -0,0 +1,7 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + + diff --git a/tests/headers/elaborated.hpp b/tests/headers/elaborated.hpp new file mode 100644 index 00000000..4bfbff23 --- /dev/null +++ b/tests/headers/elaborated.hpp @@ -0,0 +1,5 @@ +namespace whatever { + typedef int whatever_t; +} + +void something(const whatever::whatever_t *wat); diff --git a/tests/headers/type_alias_empty.hpp b/tests/headers/type_alias_empty.hpp new file mode 100644 index 00000000..f0760c8f --- /dev/null +++ b/tests/headers/type_alias_empty.hpp @@ -0,0 +1,10 @@ +// bindgen-flags: --whitelist-type bool_constant -- -std=c++11 + +// NB: The --whitelist-type is done to trigger the traversal of the types on +// codegen in order to trigger #67. + +template<typename T, T Val> +struct integral_constant {}; + +template<bool B> +using bool_constant = integral_constant<bool, B>; diff --git a/tests/tests.rs b/tests/tests.rs index 66e70859..1f8864e0 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -85,16 +85,15 @@ fn run_bindgen_tests() { } }); - // First spawn all child processes and collect them, then wait on each - // one. This runs the tests in parallel rather than serially. + // Spawn one child at a time and wait on it as number of process + // is the number of test files. - let children: Vec<_> = tests.map(|entry| { - let child = spawn_run_bindgen(run_bindgen.clone(), bindgen.clone(), entry.path()); - (entry.path(), child) - }) - .collect(); + let children = tests.map(|entry| { + let child = spawn_run_bindgen(run_bindgen.clone(), bindgen.clone(), entry.path()); + (entry.path(), child) + }); - let failures: Vec<_> = children.into_iter() + let failures: Vec<_> = children .filter_map(|(path, mut child)| { let passed = child.wait() .expect("Should wait on child process") |