diff --git a/src/ir/cant_derive_debug.rs b/src/ir/cant_derive_debug.rs new file mode 100644 index 0000000000..465f068c9b --- /dev/null +++ b/src/ir/cant_derive_debug.rs @@ -0,0 +1,303 @@ +//! Determining which types for which we can emit `#[derive(Debug)]`. +use super::analysis::MonotoneFramework; +use ir::context::{BindgenContext, ItemId}; +use ir::item::ItemSet; +use std::collections::HashSet; +use std::collections::HashMap; +use ir::traversal::EdgeKind; +use ir::ty::RUST_DERIVE_IN_ARRAY_LIMIT; +use ir::ty::TypeKind; +use ir::comp::Field; +use ir::traversal::Trace; +use ir::comp::FieldMethods; +use ir::layout::Layout; +use ir::derive::CanTriviallyDeriveDebug; +use ir::comp::CompKind; + +/// An analysis that finds for each IR item whether debug cannot be derived. +/// +/// We use the monotone constraint function `cant_derive_debug`, defined as +/// follows: +/// +/// * If T is Opaque and layout of the type is known, get this layout as opaque +/// type and check whether it can be derived using trivial checks. +/// * If T is Array type, debug cannot be derived if the length of the array is +/// larger than the limit or the type of data the array contains cannot derive +/// debug. +/// * If T is a type alias, a templated alias or an indirection to another type, +/// debug cannot be derived if the type T refers to cannot be derived debug. +/// * If T is a compound type, debug cannot be derived if any of its base member +/// or field cannot be derived debug. +/// * If T is a pointer, T cannot be derived debug if T is a function pointer +/// and the function signature cannot be derived debug. +/// * If T is an instantiation of an abstract template definition, T cannot be +/// derived debug if any of the template arguments or template definition +/// cannot derive debug. +#[derive(Debug, Clone)] +pub struct CantDeriveDebugAnalysis<'ctx, 'gen> + where 'gen: 'ctx +{ + ctx: &'ctx BindgenContext<'gen>, + + // The incremental result of this analysis's computation. Everything in this + // set cannot derive debug. + cant_derive_debug: HashSet, + + // Dependencies saying that if a key ItemId has been inserted into the + // `cant_derive_debug` set, then each of the ids in Vec need to be + // considered again. + // + // This is a subset of the natural IR graph with reversed edges, where we + // only include the edges from the IR graph that can affect whether a type + // can derive debug or not. + dependencies: HashMap>, +} + +impl<'ctx, 'gen> CantDeriveDebugAnalysis<'ctx, 'gen> { + fn consider_edge(kind: EdgeKind) -> bool { + match kind { + // These are the only edges that can affect whether a type can derive + // debug or not. + EdgeKind::BaseMember | + EdgeKind::Field | + EdgeKind::TypeReference | + EdgeKind::VarType | + EdgeKind::TemplateArgument | + EdgeKind::TemplateDeclaration | + EdgeKind::TemplateParameterDefinition => true, + + EdgeKind::Constructor | + EdgeKind::FunctionReturn | + EdgeKind::FunctionParameter | + EdgeKind::InnerType | + EdgeKind::InnerVar | + EdgeKind::Method => false, + EdgeKind::Generic => false, + } + } + + fn insert(&mut self, id: ItemId) -> bool { + let was_already_in = self.cant_derive_debug.insert(id); + assert!( + was_already_in, + format!("We shouldn't try and insert twice because if it was already in the set, \ + `constrain` would have exited early.: {:?}", id) + ); + true + } +} + +impl<'ctx, 'gen> MonotoneFramework for CantDeriveDebugAnalysis<'ctx, 'gen> { + type Node = ItemId; + type Extra = &'ctx BindgenContext<'gen>; + type Output = HashSet; + + fn new(ctx: &'ctx BindgenContext<'gen>) -> CantDeriveDebugAnalysis<'ctx, 'gen> { + let cant_derive_debug = HashSet::new(); + let mut dependencies = HashMap::new(); + let whitelisted_items: HashSet<_> = ctx.whitelisted_items().collect(); + + let whitelisted_and_blacklisted_items: ItemSet = whitelisted_items.iter() + .cloned() + .flat_map(|i| { + let mut reachable = vec![i]; + i.trace(ctx, &mut |s, _| { + reachable.push(s); + }, &()); + reachable + }) + .collect(); + + for item in whitelisted_and_blacklisted_items { + dependencies.entry(item).or_insert(vec![]); + + { + // We reverse our natural IR graph edges to find dependencies + // between nodes. + item.trace(ctx, &mut |sub_item: ItemId, edge_kind| { + if Self::consider_edge(edge_kind) { + dependencies.entry(sub_item) + .or_insert(vec![]) + .push(item); + } + }, &()); + } + } + + CantDeriveDebugAnalysis { + ctx: ctx, + cant_derive_debug: cant_derive_debug, + dependencies: dependencies, + } + } + + fn initial_worklist(&self) -> Vec { + self.ctx.whitelisted_items().collect() + } + + fn constrain(&mut self, id: ItemId) -> bool { + if self.cant_derive_debug.contains(&id) { + return false; + } + + let item = self.ctx.resolve_item(id); + let ty = match item.as_type() { + None => return false, + Some(ty) => ty + }; + + match *ty.kind() { + // handle the simple case + // These can derive debug without further information + TypeKind::Void | + TypeKind::NullPtr | + TypeKind::Int(..) | + TypeKind::Float(..) | + TypeKind::Complex(..) | + TypeKind::Function(..) | + TypeKind::Enum(..) | + TypeKind::Reference(..) | + TypeKind::BlockPointer | + TypeKind::Named | + TypeKind::UnresolvedTypeRef(..) | + TypeKind::ObjCInterface(..) | + TypeKind::ObjCId | + TypeKind::ObjCSel => { + return false; + }, + TypeKind::Opaque => { + if ty.layout(self.ctx) + .map_or(true, |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) { + return false; + } else { + return self.insert(id); + } + }, + TypeKind::Array(t, len) => { + if len <= RUST_DERIVE_IN_ARRAY_LIMIT { + if self.cant_derive_debug.contains(&t) { + return self.insert(id); + } + return false; + } else { + return self.insert(id); + } + }, + TypeKind::ResolvedTypeRef(t) | + TypeKind::TemplateAlias(t, _) | + TypeKind::Alias(t) => { + if self.cant_derive_debug.contains(&t) { + return self.insert(id); + } + return false; + }, + TypeKind::Comp(ref info) => { + if info.has_non_type_template_params() { + if ty.layout(self.ctx).map_or(true, + |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) { + return false; + } else { + return self.insert(id); + } + } + if info.kind() == CompKind::Union { + if self.ctx.options().unstable_rust { + return self.insert(id); + } + + if ty.layout(self.ctx).map_or(true, + |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) { + return false; + } else { + return self.insert(id); + } + } + let bases_cant_derive = info.base_members() + .iter() + .any(|base| self.cant_derive_debug.contains(&base.ty)); + if bases_cant_derive { + return self.insert(id); + } + let fields_cant_derive = info.fields() + .iter() + .any(|f| { + match f { + &Field::DataMember(ref data) => self.cant_derive_debug.contains(&data.ty()), + &Field::Bitfields(ref bfu) => bfu.bitfields() + .iter().any(|b| { + self.cant_derive_debug.contains(&b.ty()) + }) + } + }); + if fields_cant_derive { + return self.insert(id); + } + false + }, + TypeKind::Pointer(inner) => { + let inner_type = self.ctx.resolve_type(inner); + if let TypeKind::Function(ref sig) = + *inner_type.canonical_type(self.ctx).kind() { + if sig.can_trivially_derive_debug(&self.ctx, ()) { + return false; + } else { + return self.insert(id); + } + } + false + }, + TypeKind::TemplateInstantiation(ref template) => { + let args_cant_derive = template.template_arguments() + .iter() + .any(|arg| self.cant_derive_debug.contains(&arg)); + if args_cant_derive { + return self.insert(id); + } + let ty_cant_derive = template.template_definition() + .into_resolver() + .through_type_refs() + .through_type_aliases() + .resolve(self.ctx) + .as_type() + .expect("Instantiations of a non-type?") + .as_comp() + .and_then(|c| { + // For non-type template parameters, we generate an opaque + // blob, and in this case the instantiation has a better + // idea of the layout than the definition does. + if c.has_non_type_template_params() { + let opaque = ty.layout(self.ctx) + .or_else(|| self.ctx.resolve_type(template.template_definition()).layout(self.ctx)) + .unwrap_or(Layout::zero()) + .opaque(); + Some(!opaque.can_trivially_derive_debug(&self.ctx, ())) + } else { + None + } + }) + .unwrap_or_else(|| self.cant_derive_debug.contains(&template.template_definition())); + if ty_cant_derive { + return self.insert(id); + } + false + }, + } + } + + fn each_depending_on(&self, id: ItemId, mut f: F) + where F: FnMut(ItemId), + { + if let Some(edges) = self.dependencies.get(&id) { + for item in edges { + trace!("enqueue {:?} into worklist", item); + f(*item); + } + } + } +} + +impl<'ctx, 'gen> From> for HashSet { + fn from(analysis: CantDeriveDebugAnalysis<'ctx, 'gen>) -> Self { + analysis.cant_derive_debug + } +} diff --git a/src/ir/comp.rs b/src/ir/comp.rs index 28efd3215b..c7c2d82a19 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -2,7 +2,7 @@ use super::annotations::Annotations; use super::context::{BindgenContext, ItemId}; -use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault}; +use super::derive::{CanDeriveCopy, CanDeriveDefault}; use super::dot::DotAttributes; use super::item::{IsOpaque, Item}; use super::layout::Layout; @@ -701,19 +701,6 @@ impl FieldMethods for FieldData { } } -impl CanDeriveDebug for Field { - type Extra = (); - - fn can_derive_debug(&self, ctx: &BindgenContext, _: ()) -> bool { - match *self { - Field::DataMember(ref data) => data.ty.can_derive_debug(ctx, ()), - Field::Bitfields(BitfieldUnit { ref bitfields, .. }) => bitfields.iter().all(|b| { - b.ty().can_derive_debug(ctx, ()) - }), - } - } -} - impl CanDeriveDefault for Field { type Extra = (); @@ -857,10 +844,6 @@ pub struct CompInfo { /// and pray, or behave as an opaque type. found_unknown_attr: bool, - /// Used to detect if we've run in a can_derive_debug cycle while cycling - /// around the template arguments. - detect_derive_debug_cycle: Cell, - /// Used to detect if we've run in a can_derive_default cycle while cycling /// around the template arguments. detect_derive_default_cycle: Cell, @@ -893,7 +876,6 @@ impl CompInfo { has_non_type_template_params: false, packed: false, found_unknown_attr: false, - detect_derive_debug_cycle: Cell::new(false), detect_derive_default_cycle: Cell::new(false), detect_has_destructor_cycle: Cell::new(false), is_forward_declaration: false, @@ -1442,51 +1424,6 @@ impl TemplateParameters for 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(true, |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; - } - - return layout.unwrap_or_else(Layout::zero) - .opaque() - .can_derive_debug(ctx, ()); - } - - self.detect_derive_debug_cycle.set(true); - - let can_derive_debug = { - self.base_members - .iter() - .all(|base| base.ty.can_derive_debug(ctx, ())) && - self.fields() - .iter() - .all(|f| f.can_derive_debug(ctx, ())) - }; - - self.detect_derive_debug_cycle.set(false); - - can_derive_debug - } -} - impl CanDeriveDefault for CompInfo { type Extra = Option; diff --git a/src/ir/context.rs b/src/ir/context.rs index c3a0dd5a90..b9b5af3e08 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -10,6 +10,7 @@ use super::analysis::analyze; use super::template::{TemplateInstantiation, TemplateParameters}; use super::traversal::{self, Edge, ItemTraversal}; use super::ty::{FloatKind, Type, TypeKind}; +use super::cant_derive_debug::CantDeriveDebugAnalysis; use BindgenOptions; use cexpr; use callbacks::ParseCallbacks; @@ -18,7 +19,7 @@ use clang_sys; use parse::ClangItemParser; use std::borrow::Cow; use std::cell::Cell; -use std::collections::{HashMap, hash_map}; +use std::collections::{HashMap, hash_map, HashSet}; use std::collections::btree_map::{self, BTreeMap}; use std::fmt; use std::iter::IntoIterator; @@ -169,6 +170,11 @@ pub struct BindgenContext<'ctx> { /// Whether we need the mangling hack which removes the prefixing underscore. needs_mangling_hack: bool, + + /// Set of ItemId that can't derive debug. + /// Populated when we enter codegen by `compute_can_derive_debug`; always `None` + /// before that and `Some` after. + cant_derive_debug: Option>, } /// A traversal of whitelisted items. @@ -295,6 +301,7 @@ impl<'ctx> BindgenContext<'ctx> { used_template_parameters: None, need_bitfield_allocation: Default::default(), needs_mangling_hack: needs_mangling_hack, + cant_derive_debug: None, }; me.add_item(root_module, None, None); @@ -757,6 +764,7 @@ impl<'ctx> BindgenContext<'ctx> { self.assert_every_item_in_a_module(); self.find_used_template_parameters(); + self.compute_cant_derive_debug(); let ret = cb(self); self.gen_ctx = None; @@ -1652,6 +1660,23 @@ impl<'ctx> BindgenContext<'ctx> { pub fn need_bindegen_complex_type(&self) -> bool { self.generated_bindegen_complex.get() } + + /// Compute whether we can derive debug. + fn compute_cant_derive_debug(&mut self) { + assert!(self.cant_derive_debug.is_none()); + self.cant_derive_debug = Some(analyze::(self)); + } + + /// Look up whether the item with `id` can + /// derive debug or not. + pub fn lookup_item_id_can_derive_debug(&self, id: ItemId) -> bool { + assert!(self.in_codegen_phase(), + "We only compute can_derive_debug when we enter codegen"); + + // Look up the computed value for whether the item with `id` can + // derive debug or not. + !self.cant_derive_debug.as_ref().unwrap().contains(&id) + } } /// A builder struct for configuring item resolution options. diff --git a/src/ir/derive.rs b/src/ir/derive.rs index 0fc8debffa..5954816321 100644 --- a/src/ir/derive.rs +++ b/src/ir/derive.rs @@ -23,6 +23,23 @@ pub trait CanDeriveDebug { -> bool; } +/// A trait that encapsulates the logic for whether or not we can derive `Debug`. +/// The difference between this trait and the CanDeriveDebug is that the type +/// implementing this trait cannot use recursion or lookup result from fix point +/// analysis. It's a helper trait for fix point analysis. +pub trait CanTriviallyDeriveDebug { + + /// Serve the same purpose as the Extra in CanDeriveDebug. + type Extra; + + /// Return `true` if `Debug` can be derived for this thing, `false` + /// otherwise. + fn can_trivially_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> { diff --git a/src/ir/function.rs b/src/ir/function.rs index 5fa95203a7..40ac5f3267 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -7,7 +7,7 @@ use super::traversal::{EdgeKind, Trace, Tracer}; use super::ty::TypeKind; use clang; use clang_sys::CXCallingConv; -use ir::derive::CanDeriveDebug; +use ir::derive::CanTriviallyDeriveDebug; use parse::{ClangItemParser, ClangSubItemParser, ParseError, ParseResult}; use std::io; use syntax::abi; @@ -440,10 +440,10 @@ impl Trace for FunctionSig { // and https://github.com/rust-lang/rust/issues/40158 // // Note that copy is always derived, so we don't need to implement it. -impl CanDeriveDebug for FunctionSig { +impl CanTriviallyDeriveDebug for FunctionSig { type Extra = (); - fn can_derive_debug(&self, _ctx: &BindgenContext, _: ()) -> bool { + fn can_trivially_derive_debug(&self, _ctx: &BindgenContext, _: ()) -> bool { const RUST_DERIVE_FUNPTR_LIMIT: usize = 12; if self.argument_types.len() > RUST_DERIVE_FUNPTR_LIMIT { return false; diff --git a/src/ir/item.rs b/src/ir/item.rs index e65b7a99d1..6712c8a7f4 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -272,28 +272,7 @@ impl CanDeriveDebug for Item { type Extra = (); fn can_derive_debug(&self, ctx: &BindgenContext, _: ()) -> bool { - if self.detect_derive_debug_cycle.get() { - return true; - } - - self.detect_derive_debug_cycle.set(true); - - let result = ctx.options().derive_debug && - 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, - }; - - self.detect_derive_debug_cycle.set(false); - - result + ctx.options().derive_debug && ctx.lookup_item_id_can_derive_debug(self.id()) } } diff --git a/src/ir/layout.rs b/src/ir/layout.rs index 21382b2d99..91dd9d6cb1 100644 --- a/src/ir/layout.rs +++ b/src/ir/layout.rs @@ -1,7 +1,7 @@ //! Intermediate representation for the physical layout of some type. use super::context::BindgenContext; -use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault}; +use super::derive::{CanDeriveCopy, CanTriviallyDeriveDebug, CanDeriveDefault}; use super::ty::{RUST_DERIVE_IN_ARRAY_LIMIT, Type, TypeKind}; use clang; use std::{cmp, mem}; @@ -102,10 +102,10 @@ impl Opaque { } } -impl CanDeriveDebug for Opaque { +impl CanTriviallyDeriveDebug for Opaque { type Extra = (); - fn can_derive_debug(&self, _: &BindgenContext, _: ()) -> bool { + fn can_trivially_derive_debug(&self, _: &BindgenContext, _: ()) -> bool { self.array_size() .map_or(false, |size| size <= RUST_DERIVE_IN_ARRAY_LIMIT) } diff --git a/src/ir/mod.rs b/src/ir/mod.rs index 6d4d8a23e3..f8b9edfa63 100644 --- a/src/ir/mod.rs +++ b/src/ir/mod.rs @@ -23,3 +23,4 @@ pub mod traversal; pub mod ty; pub mod var; pub mod objc; +pub mod cant_derive_debug; diff --git a/src/ir/template.rs b/src/ir/template.rs index a541442777..25d1d43805 100644 --- a/src/ir/template.rs +++ b/src/ir/template.rs @@ -28,9 +28,8 @@ //! ``` use super::context::{BindgenContext, ItemId}; -use super::derive::{CanDeriveCopy, CanDeriveDebug}; +use super::derive::{CanDeriveCopy}; use super::item::{IsOpaque, Item, ItemAncestors, ItemCanonicalPath}; -use super::layout::Layout; use super::traversal::{EdgeKind, Trace, Tracer}; use clang; use parse::ClangItemParser; @@ -353,40 +352,6 @@ impl<'a> CanDeriveCopy<'a> for TemplateInstantiation { } } -impl CanDeriveDebug for TemplateInstantiation { - type Extra = Option; - - fn can_derive_debug(&self, - ctx: &BindgenContext, - layout: Option) - -> bool { - self.args.iter().all(|arg| arg.can_derive_debug(ctx, ())) && - self.definition - .into_resolver() - .through_type_refs() - .through_type_aliases() - .resolve(ctx) - .as_type() - .expect("Instantiation of a non-type?") - .as_comp() - .and_then(|c| { - // For non-type template parameters, we generate an opaque - // blob, and in this case the instantiation has a better - // idea of the layout than the definition does. - if c.has_non_type_template_params() { - let opaque = layout - .or_else(|| ctx.resolve_type(self.definition).layout(ctx)) - .unwrap_or(Layout::zero()) - .opaque(); - Some(opaque.can_derive_debug(ctx, ())) - } else { - None - } - }) - .unwrap_or_else(|| self.definition.can_derive_debug(ctx, ())) - } -} - impl Trace for TemplateInstantiation { type Extra = (); diff --git a/src/ir/traversal.rs b/src/ir/traversal.rs index f0b7159cba..cb27e8db65 100644 --- a/src/ir/traversal.rs +++ b/src/ir/traversal.rs @@ -58,7 +58,7 @@ pub enum EdgeKind { /// template /// class Foo { }; /// - /// using Bar = Foo; + /// using Bar = Foo; /// ``` TemplateDeclaration, diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 82fae6f5e7..78274d94fb 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -539,35 +539,10 @@ impl TemplateParameters for TypeKind { } impl CanDeriveDebug for Type { - type Extra = (); + type Extra = Item; - 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)) - } - TypeKind::Pointer(inner) => { - let inner = ctx.resolve_type(inner); - if let TypeKind::Function(ref sig) = - *inner.canonical_type(ctx).kind() { - return sig.can_derive_debug(ctx, ()); - } - return true; - } - TypeKind::TemplateInstantiation(ref inst) => { - inst.can_derive_debug(ctx, self.layout(ctx)) - } - TypeKind::Opaque => { - self.layout.map_or(true, |l| l.opaque().can_derive_debug(ctx, ())) - } - _ => true, - } + fn can_derive_debug(&self, ctx: &BindgenContext, item: Item) -> bool { + ctx.lookup_item_id_can_derive_debug(item.id()) } }