diff --git a/libbindgen/src/codegen/mod.rs b/libbindgen/src/codegen/mod.rs index 69d5f65136..12aa7223b0 100644 --- a/libbindgen/src/codegen/mod.rs +++ b/libbindgen/src/codegen/mod.rs @@ -6,6 +6,7 @@ use aster; use ir::annotations::FieldAccessorKind; use ir::comp::{CompInfo, CompKind, Field, Method, MethodKind}; use ir::context::{BindgenContext, ItemId}; +use ir::derive::{CanDeriveCopy, CanDeriveDebug}; use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue}; use ir::function::{Function, FunctionSig}; use ir::int::IntKind; @@ -765,12 +766,11 @@ impl CodeGenerator for CompInfo { let is_union = self.kind() == CompKind::Union; let mut derives = vec![]; - let ty = item.expect_type(); - if ty.can_derive_debug(ctx) { + if item.can_derive_debug(ctx, ()) { derives.push("Debug"); } - if item.can_derive_copy(ctx) && !item.annotations().disallow_copy() { + if item.can_derive_copy(ctx, ()) && !item.annotations().disallow_copy() { derives.push("Copy"); if !applicable_template_args.is_empty() { // FIXME: This requires extra logic if you have a big array in a diff --git a/libbindgen/src/ir/comp.rs b/libbindgen/src/ir/comp.rs index ea3850ccd4..5d09ddbc26 100644 --- a/libbindgen/src/ir/comp.rs +++ b/libbindgen/src/ir/comp.rs @@ -6,6 +6,7 @@ use std::cell::Cell; use std::cmp; use super::annotations::Annotations; use super::context::{BindgenContext, ItemId}; +use super::derive::{CanDeriveCopy, CanDeriveDebug}; use super::item::Item; use super::layout::Layout; use super::ty::{RUST_DERIVE_IN_ARRAY_LIMIT, Type}; @@ -154,6 +155,26 @@ impl Field { } } +impl CanDeriveDebug for Field { + type Extra = (); + + fn can_derive_debug(&self, ctx: &BindgenContext, _: ()) -> bool { + self.ty.can_derive_debug(ctx, ()) + } +} + +impl<'a> CanDeriveCopy<'a> for Field { + type Extra = (); + + fn can_derive_copy(&self, ctx: &BindgenContext, _: ()) -> bool { + self.ty.can_derive_copy(ctx, ()) + } + + fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool { + self.ty.can_derive_copy_in_array(ctx, ()) + } +} + /// A compound type. /// /// Either a struct or union, a compound type is built up from the combination @@ -263,50 +284,6 @@ impl CompInfo { } } - /// Can we derive the `Debug` trait for this compound type? - pub fn can_derive_debug(&self, - ctx: &BindgenContext, - layout: Option) - -> bool { - // We can reach here recursively via template parameters of a member, - // for example. - if self.detect_derive_debug_cycle.get() { - warn!("Derive debug cycle detected!"); - return true; - } - - if self.kind == CompKind::Union { - if ctx.options().unstable_rust { - return false; - } - - let layout = layout.unwrap_or_else(Layout::zero); - let size_divisor = cmp::max(1, layout.align); - return layout.size / size_divisor <= RUST_DERIVE_IN_ARRAY_LIMIT; - } - - self.detect_derive_debug_cycle.set(true); - - let can_derive_debug = { - self.base_members - .iter() - .all(|ty| ctx.resolve_type(*ty).can_derive_debug(ctx)) && - self.template_args - .iter() - .all(|ty| ctx.resolve_type(*ty).can_derive_debug(ctx)) && - self.fields - .iter() - .all(|f| ctx.resolve_type(f.ty).can_derive_debug(ctx)) && - self.ref_template.map_or(true, |template| { - ctx.resolve_type(template).can_derive_debug(ctx) - }) - }; - - self.detect_derive_debug_cycle.set(false); - - can_derive_debug - } - /// Is this compound type unsized? pub fn is_unsized(&self, ctx: &BindgenContext) -> bool { !self.has_vtable(ctx) && self.fields.is_empty() && @@ -356,41 +333,6 @@ impl CompInfo { has_destructor } - /// Can we derive the `Copy` trait for this type? - pub fn can_derive_copy(&self, ctx: &BindgenContext, item: &Item) -> bool { - // NOTE: Take into account that while unions in C and C++ are copied by - // default, the may have an explicit destructor in C++, so we can't - // defer this check just for the union case. - if self.has_destructor(ctx) { - return false; - } - - if self.kind == CompKind::Union { - if !ctx.options().unstable_rust { - return true; - } - - // https://github.com/rust-lang/rust/issues/36640 - if !self.template_args.is_empty() || self.ref_template.is_some() || - !item.applicable_template_args(ctx).is_empty() { - return false; - } - } - - // With template args, use a safe subset of the types, - // since copyability depends on the types itself. - self.ref_template - .as_ref() - .map_or(true, |t| ctx.resolve_item(*t).can_derive_copy(ctx)) && - self.base_members - .iter() - .all(|t| ctx.resolve_item(*t).can_derive_copy(ctx)) && - self.fields.iter().all(|field| { - ctx.resolve_item(field.ty) - .can_derive_copy(ctx) - }) - } - /// Is this type a template specialization? pub fn is_template_specialization(&self) -> bool { self.ref_template.is_some() @@ -858,6 +800,108 @@ impl CompInfo { } } +impl CanDeriveDebug for CompInfo { + type Extra = Option; + + fn can_derive_debug(&self, + ctx: &BindgenContext, + layout: Option) + -> bool { + if self.has_non_type_template_params() { + return layout.map_or(false, |l| l.opaque().can_derive_debug(ctx, ())); + } + + // We can reach here recursively via template parameters of a member, + // for example. + if self.detect_derive_debug_cycle.get() { + warn!("Derive debug cycle detected!"); + return true; + } + + if self.kind == CompKind::Union { + if ctx.options().unstable_rust { + return false; + } + + let layout = layout.unwrap_or_else(Layout::zero); + let size_divisor = cmp::max(1, layout.align); + return layout.size / size_divisor <= RUST_DERIVE_IN_ARRAY_LIMIT; + } + + self.detect_derive_debug_cycle.set(true); + + let can_derive_debug = { + self.base_members + .iter() + .all(|id| id.can_derive_debug(ctx, ())) && + self.template_args + .iter() + .all(|id| id.can_derive_debug(ctx, ())) && + self.fields + .iter() + .all(|f| f.can_derive_debug(ctx, ())) && + self.ref_template.map_or(true, |id| { + id.can_derive_debug(ctx, ()) + }) + }; + + self.detect_derive_debug_cycle.set(false); + + can_derive_debug + } +} + +impl<'a> CanDeriveCopy<'a> for CompInfo { + type Extra = (&'a Item, Option); + + fn can_derive_copy(&self, + ctx: &BindgenContext, + (item, layout): (&Item, Option)) + -> bool { + if self.has_non_type_template_params() { + return layout.map_or(false, |l| l.opaque().can_derive_copy(ctx, ())); + } + + // NOTE: Take into account that while unions in C and C++ are copied by + // default, the may have an explicit destructor in C++, so we can't + // defer this check just for the union case. + if self.has_destructor(ctx) { + return false; + } + + if self.kind == CompKind::Union { + if !ctx.options().unstable_rust { + return true; + } + + // https://github.com/rust-lang/rust/issues/36640 + if !self.template_args.is_empty() || self.ref_template.is_some() || + !item.applicable_template_args(ctx).is_empty() { + return false; + } + } + + // With template args, use a safe subset of the types, + // since copyability depends on the types itself. + self.ref_template + .as_ref() + .map_or(true, |t| t.can_derive_copy(ctx, ())) && + self.base_members + .iter() + .all(|t| t.can_derive_copy(ctx, ())) && + self.fields.iter().all(|field| { + field.can_derive_copy(ctx, ()) + }) + } + + fn can_derive_copy_in_array(&self, + ctx: &BindgenContext, + extra: (&Item, Option)) + -> bool { + self.can_derive_copy(ctx, extra) + } +} + impl TypeCollector for CompInfo { type Extra = Item; diff --git a/libbindgen/src/ir/context.rs b/libbindgen/src/ir/context.rs index 3ffe50c835..5dc7886beb 100644 --- a/libbindgen/src/ir/context.rs +++ b/libbindgen/src/ir/context.rs @@ -9,6 +9,7 @@ use std::cell::Cell; use std::collections::{HashMap, VecDeque, hash_map}; use std::collections::btree_map::{self, BTreeMap}; use std::fmt; +use super::derive::{CanDeriveCopy, CanDeriveDebug}; use super::int::IntKind; use super::item::{Item, ItemCanonicalPath}; use super::item_kind::ItemKind; @@ -32,6 +33,26 @@ impl ItemId { } } +impl CanDeriveDebug for ItemId { + type Extra = (); + + fn can_derive_debug(&self, ctx: &BindgenContext, _: ()) -> bool { + ctx.resolve_item(*self).can_derive_debug(ctx, ()) + } +} + +impl<'a> CanDeriveCopy<'a> for ItemId { + type Extra = (); + + fn can_derive_copy(&self, ctx: &BindgenContext, _: ()) -> bool { + ctx.resolve_item(*self).can_derive_copy(ctx, ()) + } + + fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool { + ctx.resolve_item(*self).can_derive_copy_in_array(ctx, ()) + } +} + /// A key used to index a resolved type, so we only process it once. /// /// This is almost always a USR string (an unique identifier generated by diff --git a/libbindgen/src/ir/derive.rs b/libbindgen/src/ir/derive.rs new file mode 100644 index 0000000000..d13a811708 --- /dev/null +++ b/libbindgen/src/ir/derive.rs @@ -0,0 +1,67 @@ +//! Traits for determining whether we can derive traits for a thing or not. + +use super::context::BindgenContext; + +/// A trait that encapsulates the logic for whether or not we can derive `Debug` +/// for a given thing. +/// +/// This should ideally be a no-op that just returns `true`, but instead needs +/// to be a recursive method that checks whether all the proper members can +/// derive debug or not, because of the limit rust has on 32 items as max in the +/// array. +pub trait CanDeriveDebug { + /// Implementations can define this type to get access to any extra + /// information required to determine whether they can derive `Debug`. If + /// extra information is unneeded, then this should simply be the unit type. + type Extra; + + /// Return `true` if `Debug` can be derived for this thing, `false` + /// otherwise. + fn can_derive_debug(&self, + ctx: &BindgenContext, + extra: Self::Extra) + -> bool; +} + +/// A trait that encapsulates the logic for whether or not we can derive `Copy` +/// for a given thing. +pub trait CanDeriveCopy<'a> { + /// Implementations can define this type to get access to any extra + /// information required to determine whether they can derive `Copy`. If + /// extra information is unneeded, then this should simply be the unit type. + type Extra; + + /// Return `true` if `Copy` can be derived for this thing, `false` + /// otherwise. + fn can_derive_copy(&'a self, + ctx: &'a BindgenContext, + extra: Self::Extra) + -> bool; + + /// For some reason, deriving copies of an array of a type that is not known + /// to be `Copy` is a compile error. e.g.: + /// + /// ```rust + /// #[derive(Copy, Clone)] + /// struct A { + /// member: T, + /// } + /// ``` + /// + /// is fine, while: + /// + /// ```rust,ignore + /// #[derive(Copy, Clone)] + /// struct A { + /// member: [T; 1], + /// } + /// ``` + /// + /// is an error. + /// + /// That's the whole point of the existence of `can_derive_copy_in_array`. + fn can_derive_copy_in_array(&'a self, + ctx: &'a BindgenContext, + extra: Self::Extra) + -> bool; +} diff --git a/libbindgen/src/ir/item.rs b/libbindgen/src/ir/item.rs index 932b06fc9f..913fb2f2f9 100644 --- a/libbindgen/src/ir/item.rs +++ b/libbindgen/src/ir/item.rs @@ -8,6 +8,7 @@ use std::fmt::Write; use std::iter; use super::annotations::Annotations; use super::context::{BindgenContext, ItemId}; +use super::derive::{CanDeriveCopy, CanDeriveDebug}; use super::function::Function; use super::item_kind::ItemKind; use super::module::Module; @@ -203,6 +204,56 @@ impl TypeCollector for Item { } } +impl CanDeriveDebug for Item { + type Extra = (); + + fn can_derive_debug(&self, ctx: &BindgenContext, _: ()) -> bool { + match self.kind { + ItemKind::Type(ref ty) => { + if self.is_opaque(ctx) { + ty.layout(ctx) + .map_or(true, |l| l.opaque().can_derive_debug(ctx, ())) + } else { + ty.can_derive_debug(ctx, ()) + } + }, + _ => false, + } + } +} + +impl<'a> CanDeriveCopy<'a> for Item { + type Extra = (); + + fn can_derive_copy(&self, ctx: &BindgenContext, _: ()) -> bool { + match self.kind { + ItemKind::Type(ref ty) => { + if self.is_opaque(ctx) { + ty.layout(ctx) + .map_or(true, |l| l.opaque().can_derive_copy(ctx, ())) + } else { + ty.can_derive_copy(ctx, self) + } + } + _ => false, + } + } + + fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool { + match self.kind { + ItemKind::Type(ref ty) => { + if self.is_opaque(ctx) { + ty.layout(ctx) + .map_or(true, |l| l.opaque().can_derive_copy_in_array(ctx, ())) + } else { + ty.can_derive_copy_in_array(ctx, self) + } + } + _ => false, + } + } +} + /// An item is the base of the bindgen representation, it can be either a /// module, a type, a function, or a variable (see `ItemKind` for more /// information). @@ -806,19 +857,6 @@ impl Item { _ => None, } } - - /// Can we derive an implementation of the `Copy` trait for this type? - pub fn can_derive_copy(&self, ctx: &BindgenContext) -> bool { - self.expect_type().can_derive_copy(ctx, self) - } - - /// Can we derive an implementation of the `Copy` trait for an array of this - /// type? - /// - /// See `Type::can_derive_copy_in_array` for details. - pub fn can_derive_copy_in_array(&self, ctx: &BindgenContext) -> bool { - self.expect_type().can_derive_copy_in_array(ctx, self) - } } // An utility function to handle recursing inside nested types. diff --git a/libbindgen/src/ir/layout.rs b/libbindgen/src/ir/layout.rs index 3ac4a5f417..075f42c904 100644 --- a/libbindgen/src/ir/layout.rs +++ b/libbindgen/src/ir/layout.rs @@ -1,5 +1,9 @@ //! Intermediate representation for the physical layout of some type. +use super::context::BindgenContext; +use super::derive::{CanDeriveCopy, CanDeriveDebug}; +use super::ty::RUST_DERIVE_IN_ARRAY_LIMIT; + /// A type that represents the struct layout of a type. #[derive(Debug, Clone, Copy)] pub struct Layout { @@ -31,4 +35,32 @@ impl Layout { pub fn zero() -> Self { Self::new(0, 0) } + + /// Get this layout as an opaque type. + pub fn opaque(&self) -> Opaque { + Opaque(*self) + } +} + +/// When we are treating a type as opaque, it is just a blob with a `Layout`. +pub struct Opaque(pub Layout); + +impl CanDeriveDebug for Opaque { + type Extra = (); + + fn can_derive_debug(&self, _: &BindgenContext, _: ()) -> bool { + self.0.size < RUST_DERIVE_IN_ARRAY_LIMIT + } +} + +impl<'a> CanDeriveCopy<'a> for Opaque { + type Extra = (); + + fn can_derive_copy(&self, _: &BindgenContext, _: ()) -> bool { + self.0.size < RUST_DERIVE_IN_ARRAY_LIMIT + } + + fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool { + self.can_derive_copy(ctx, ()) + } } diff --git a/libbindgen/src/ir/mod.rs b/libbindgen/src/ir/mod.rs index 3c658a4aad..73793b167f 100644 --- a/libbindgen/src/ir/mod.rs +++ b/libbindgen/src/ir/mod.rs @@ -6,6 +6,7 @@ pub mod annotations; pub mod comp; pub mod context; +pub mod derive; pub mod enum_ty; pub mod function; pub mod int; diff --git a/libbindgen/src/ir/ty.rs b/libbindgen/src/ir/ty.rs index 6859b75386..1c94ddf577 100644 --- a/libbindgen/src/ir/ty.rs +++ b/libbindgen/src/ir/ty.rs @@ -4,6 +4,7 @@ use clang::{self, Cursor}; use parse::{ClangItemParser, ParseError, ParseResult}; use super::comp::CompInfo; use super::context::{BindgenContext, ItemId}; +use super::derive::{CanDeriveCopy, CanDeriveDebug}; use super::enum_ty::Enum; use super::function::FunctionSig; use super::int::IntKind; @@ -207,83 +208,6 @@ impl Type { }) } - /// Wether we can derive rust's `Debug` annotation in Rust. This should - /// ideally be a no-op that just returns `true`, but instead needs to be a - /// recursive method that checks whether all the proper members can derive - /// debug or not, because of the limit rust has on 32 items as max in the - /// array. - pub fn can_derive_debug(&self, ctx: &BindgenContext) -> bool { - match self.kind { - TypeKind::Array(t, len) => { - len <= RUST_DERIVE_IN_ARRAY_LIMIT && - ctx.resolve_type(t).can_derive_debug(ctx) - } - TypeKind::ResolvedTypeRef(t) | - TypeKind::TemplateAlias(_, t, _) | - TypeKind::Alias(_, t) => ctx.resolve_type(t).can_derive_debug(ctx), - TypeKind::Comp(ref info) => { - info.can_derive_debug(ctx, self.layout(ctx)) - } - _ => true, - } - } - - /// For some reason, deriving copies of an array of a type that is not known - /// to be copy is a compile error. e.g.: - /// - /// ```rust - /// #[derive(Copy, Clone)] - /// struct A { - /// member: T, - /// } - /// ``` - /// - /// is fine, while: - /// - /// ```rust,ignore - /// #[derive(Copy, Clone)] - /// struct A { - /// member: [T; 1], - /// } - /// ``` - /// - /// is an error. - /// - /// That's the whole point of the existence of `can_derive_copy_in_array`. - pub fn can_derive_copy_in_array(&self, - ctx: &BindgenContext, - item: &Item) - -> bool { - match self.kind { - TypeKind::ResolvedTypeRef(t) | - TypeKind::TemplateAlias(_, t, _) | - TypeKind::Alias(_, t) | - TypeKind::Array(t, _) => { - ctx.resolve_item(t) - .can_derive_copy_in_array(ctx) - } - TypeKind::Named(..) => false, - _ => self.can_derive_copy(ctx, item), - } - } - - /// Wether we'd be able to derive the `Copy` trait in Rust or not. Same - /// rationale than `can_derive_debug`. - pub fn can_derive_copy(&self, ctx: &BindgenContext, item: &Item) -> bool { - match self.kind { - TypeKind::Array(t, len) => { - len <= RUST_DERIVE_IN_ARRAY_LIMIT && - ctx.resolve_item(t).can_derive_copy_in_array(ctx) - } - TypeKind::ResolvedTypeRef(t) | - TypeKind::TemplateAlias(_, t, _) | - TypeKind::TemplateRef(t, _) | - TypeKind::Alias(_, t) => ctx.resolve_item(t).can_derive_copy(ctx), - TypeKind::Comp(ref info) => info.can_derive_copy(ctx, item), - _ => true, - } - } - /// Whether this type has a vtable. pub fn has_vtable(&self, ctx: &BindgenContext) -> bool { // FIXME: Can we do something about template parameters? Huh... @@ -394,6 +318,61 @@ impl Type { } } +impl CanDeriveDebug for Type { + type Extra = (); + + fn can_derive_debug(&self, ctx: &BindgenContext, _: ()) -> bool { + match self.kind { + TypeKind::Array(t, len) => { + len <= RUST_DERIVE_IN_ARRAY_LIMIT && + t.can_derive_debug(ctx, ()) + } + TypeKind::ResolvedTypeRef(t) | + TypeKind::TemplateAlias(_, t, _) | + TypeKind::Alias(_, t) => t.can_derive_debug(ctx, ()), + TypeKind::Comp(ref info) => { + info.can_derive_debug(ctx, self.layout(ctx)) + } + _ => true, + } + } +} + +impl<'a> CanDeriveCopy<'a> for Type { + type Extra = &'a Item; + + fn can_derive_copy(&self, ctx: &BindgenContext, item: &Item) -> bool { + match self.kind { + TypeKind::Array(t, len) => { + len <= RUST_DERIVE_IN_ARRAY_LIMIT && + t.can_derive_copy_in_array(ctx, ()) + } + TypeKind::ResolvedTypeRef(t) | + TypeKind::TemplateAlias(_, t, _) | + TypeKind::TemplateRef(t, _) | + TypeKind::Alias(_, t) => t.can_derive_copy(ctx, ()), + TypeKind::Comp(ref info) => info.can_derive_copy(ctx, (item, self.layout(ctx))), + _ => true, + } + } + + fn can_derive_copy_in_array(&self, + ctx: &BindgenContext, + item: &Item) + -> bool { + match self.kind { + TypeKind::ResolvedTypeRef(t) | + TypeKind::TemplateAlias(_, t, _) | + TypeKind::Alias(_, t) | + TypeKind::Array(t, _) => { + t.can_derive_copy_in_array(ctx, ()) + } + TypeKind::Named(..) => false, + _ => self.can_derive_copy(ctx, item), + } + } +} + /// The kind of float this type represents. #[derive(Debug, Copy, Clone, PartialEq)] pub enum FloatKind { @@ -689,7 +668,8 @@ impl Type { Ok(inner) => inner, Err(..) => { error!("Failed to parse template alias \ - {:?}", location); + {:?}", + location); return Err(ParseError::Continue); } }; @@ -913,7 +893,7 @@ impl TypeCollector for Type { TypeKind::Function(ref sig) => { sig.collect_types(context, types, item) } - TypeKind::Named(_) => {}, + TypeKind::Named(_) => {} // FIXME: Pending types! ref other @ _ => { debug!("::collect_types: Ignoring: \ diff --git a/libbindgen/tests/expectations/tests/issue-372.rs b/libbindgen/tests/expectations/tests/issue-372.rs new file mode 100644 index 0000000000..c6d9209e26 --- /dev/null +++ b/libbindgen/tests/expectations/tests/issue-372.rs @@ -0,0 +1,63 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +pub mod root { + #[allow(unused_imports)] + use self::super::root; + #[repr(C)] + #[derive(Debug, Copy)] + pub struct d { + pub m: root::i, + } + #[test] + fn bindgen_test_layout_d() { + assert_eq!(::std::mem::size_of::() , 24usize); + assert_eq!(::std::mem::align_of::() , 8usize); + } + impl Clone for d { + fn clone(&self) -> Self { *self } + } + #[repr(C)] + #[derive(Debug, Copy)] + pub struct i { + pub j: *mut root::i, + pub k: *mut root::i, + pub l: bool, + } + #[test] + fn bindgen_test_layout_i() { + assert_eq!(::std::mem::size_of::() , 24usize); + assert_eq!(::std::mem::align_of::() , 8usize); + } + impl Clone for i { + fn clone(&self) -> Self { *self } + } + #[repr(u32)] + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] + pub enum n { + o = 0, + p = 1, + q = 2, + r = 3, + s = 4, + t = 5, + b = 6, + ae = 7, + e = 8, + ag = 9, + ah = 10, + ai = 11, + } + #[repr(C)] + pub struct F { + pub w: [u64; 33usize], + } + #[test] + fn bindgen_test_layout_F() { + assert_eq!(::std::mem::size_of::() , 264usize); + assert_eq!(::std::mem::align_of::() , 8usize); + } +} diff --git a/libbindgen/tests/headers/issue-372.hpp b/libbindgen/tests/headers/issue-372.hpp new file mode 100644 index 0000000000..a072f06127 --- /dev/null +++ b/libbindgen/tests/headers/issue-372.hpp @@ -0,0 +1,16 @@ +// bindgen-flags: --enable-cxx-namespaces +template class c { a e[b]; }; +class d; +template class C { c h; }; +class i { + i *j; + i *k; + bool l; +}; +class d { + i m; +}; +enum n { o, p, q, r, s, t, b, ae, e, ag, ah, ai }; +class F { + C w; +};