Skip to content

A bunch of little analysis cleanups #830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 20, 2017
Merged

A bunch of little analysis cleanups #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dc73bc6
Rearrange monotone framework and used templates analysis comments
fitzgen Jul 20, 2017
003cfe6
Move fix-point analyses to their own module
fitzgen Jul 20, 2017
60ea28e
Use "cannot" instead of "can't" in names
fitzgen Jul 20, 2017
497f028
Rename `CannotDeriveDebugAnalysis` to `CannotDeriveDebug`
fitzgen Jul 20, 2017
291f623
Fix variable name to reflect its semantics
fitzgen Jul 20, 2017
2960d86
Only traverse the IR graph to compute whitelisted items once
fitzgen Jul 20, 2017
3662f14
Also assert against dangling references after resolving typerefs and …
fitzgen Jul 20, 2017
7ff8306
Use object literal short hand for `CannotDeriveDebug`
fitzgen Jul 20, 2017
de71f9c
Capitalize, punctuate, and format a comment
fitzgen Jul 20, 2017
79a03e4
Define a type for communicating that the constraint function hasn't r…
fitzgen Jul 20, 2017
faebc0c
The `CannotDeriveDebug` analysis shouldn't special case blacklisting
fitzgen Jul 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/codegen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3275,19 +3275,17 @@ impl CodeGenerator for ObjCInterface {
}
}



pub fn codegen(context: &mut BindgenContext) -> Vec<P<ast::Item>> {
context.gen(|context| {
let counter = Cell::new(0);
let mut result = CodegenResult::new(&counter);

debug!("codegen: {:?}", context.options());

let whitelisted_items: ItemSet = context.whitelisted_items().collect();
let whitelisted_items = context.whitelisted_items();

if context.options().emit_ir {
for &id in whitelisted_items.iter() {
for &id in whitelisted_items {
let item = context.resolve_item(id);
println!("ir: {:?} = {:#?}", id, item);
}
Expand All @@ -3301,7 +3299,7 @@ pub fn codegen(context: &mut BindgenContext) -> Vec<P<ast::Item>> {
}

context.resolve_item(context.root_module())
.codegen(context, &mut result, &whitelisted_items, &());
.codegen(context, &mut result, whitelisted_items, &());

result.items
})
Expand Down
201 changes: 107 additions & 94 deletions src/ir/cant_derive_debug.rs → src/ir/analysis/derive_debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Determining which types for which we can emit `#[derive(Debug)]`.
use super::analysis::MonotoneFramework;
use ir::context::{BindgenContext, ItemId};
use ir::item::ItemSet;

use super::{ConstrainResult, MonotoneFramework};
use std::collections::HashSet;
use std::collections::HashMap;
use ir::context::{BindgenContext, ItemId};
use ir::traversal::EdgeKind;
use ir::ty::RUST_DERIVE_IN_ARRAY_LIMIT;
use ir::ty::TypeKind;
Expand All @@ -16,7 +16,7 @@ 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
/// We use the monotone constraint function `cannot_derive_debug`, defined as
/// follows:
///
/// * If T is Opaque and layout of the type is known, get this layout as opaque
Expand All @@ -34,17 +34,17 @@ use ir::comp::CompKind;
/// derived debug if any of the template arguments or template definition
/// cannot derive debug.
#[derive(Debug, Clone)]
pub struct CantDeriveDebugAnalysis<'ctx, 'gen>
pub struct CannotDeriveDebug<'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<ItemId>,
cannot_derive_debug: HashSet<ItemId>,

// Dependencies saying that if a key ItemId has been inserted into the
// `cant_derive_debug` set, then each of the ids in Vec<ItemId> need to be
// `cannot_derive_debug` set, then each of the ids in Vec<ItemId> need to be
// considered again.
//
// This is a subset of the natural IR graph with reversed edges, where we
Expand All @@ -53,7 +53,7 @@ pub struct CantDeriveDebugAnalysis<'ctx, 'gen>
dependencies: HashMap<ItemId, Vec<ItemId>>,
}

impl<'ctx, 'gen> CantDeriveDebugAnalysis<'ctx, 'gen> {
impl<'ctx, 'gen> CannotDeriveDebug<'ctx, 'gen> {
fn consider_edge(kind: EdgeKind) -> bool {
match kind {
// These are the only edges that can affect whether a type can derive
Expand All @@ -77,79 +77,69 @@ impl<'ctx, 'gen> CantDeriveDebugAnalysis<'ctx, 'gen> {
}
}

fn insert(&mut self, id: ItemId) -> bool {
let was_already_in = self.cant_derive_debug.insert(id);
fn insert(&mut self, id: ItemId) -> ConstrainResult {
let was_not_already_in_set = self.cannot_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)
was_not_already_in_set,
"We shouldn't try and insert {:?} twice because if it was \
already in the set, `constrain` should have exited early.",
id
);
true
ConstrainResult::Changed
}
}

impl<'ctx, 'gen> MonotoneFramework for CantDeriveDebugAnalysis<'ctx, 'gen> {
impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
type Node = ItemId;
type Extra = &'ctx BindgenContext<'gen>;
type Output = HashSet<ItemId>;

fn new(ctx: &'ctx BindgenContext<'gen>) -> CantDeriveDebugAnalysis<'ctx, 'gen> {
let cant_derive_debug = HashSet::new();
fn new(ctx: &'ctx BindgenContext<'gen>) -> CannotDeriveDebug<'ctx, 'gen> {
let cannot_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 {
for &item in ctx.whitelisted_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);
if ctx.whitelisted_items().contains(&sub_item) &&
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,
CannotDeriveDebug {
ctx,
cannot_derive_debug,
dependencies,
}
}

fn initial_worklist(&self) -> Vec<ItemId> {
self.ctx.whitelisted_items().collect()
self.ctx.whitelisted_items().iter().cloned().collect()
}

fn constrain(&mut self, id: ItemId) -> bool {
if self.cant_derive_debug.contains(&id) {
return false;
fn constrain(&mut self, id: ItemId) -> ConstrainResult {
if self.cannot_derive_debug.contains(&id) {
return ConstrainResult::Same;
}

let item = self.ctx.resolve_item(id);
let ty = match item.as_type() {
None => return false,
None => return ConstrainResult::Same,
Some(ty) => ty
};

match *ty.kind() {
// handle the simple case
// These can derive debug without further information
// Handle the simple cases. These can derive debug without further
// information.
TypeKind::Void |
TypeKind::NullPtr |
TypeKind::Int(..) |
Expand All @@ -164,97 +154,113 @@ impl<'ctx, 'gen> MonotoneFramework for CantDeriveDebugAnalysis<'ctx, 'gen> {
TypeKind::ObjCInterface(..) |
TypeKind::ObjCId |
TypeKind::ObjCSel => {
return false;
ConstrainResult::Same
},

TypeKind::Opaque => {
if ty.layout(self.ctx)
.map_or(true, |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
return false;
ConstrainResult::Same
} else {
return self.insert(id);
self.insert(id)
}
},

TypeKind::Array(t, len) => {
if self.cannot_derive_debug.contains(&t) {
return self.insert(id);
}

if len <= RUST_DERIVE_IN_ARRAY_LIMIT {
if self.cant_derive_debug.contains(&t) {
return self.insert(id);
}
return false;
ConstrainResult::Same
} else {
return self.insert(id);
self.insert(id)
}
},

TypeKind::ResolvedTypeRef(t) |
TypeKind::TemplateAlias(t, _) |
TypeKind::Alias(t) => {
if self.cant_derive_debug.contains(&t) {
return self.insert(id);
if self.cannot_derive_debug.contains(&t) {
self.insert(id)
} else {
ConstrainResult::Same
}
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;
if ty.layout(self.ctx)
.map_or(true,
|l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
return ConstrainResult::Same;
} 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;
if ty.layout(self.ctx)
.map_or(true,
|l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
return ConstrainResult::Same;
} else {
return self.insert(id);
}
}
let bases_cant_derive = info.base_members()

let bases_cannot_derive = info.base_members()
.iter()
.any(|base| self.cant_derive_debug.contains(&base.ty));
if bases_cant_derive {
.any(|base| self.cannot_derive_debug.contains(&base.ty));
if bases_cannot_derive {
return self.insert(id);
}
let fields_cant_derive = info.fields()

let fields_cannot_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())
})
match *f {
Field::DataMember(ref data) => {
self.cannot_derive_debug.contains(&data.ty())
}
Field::Bitfields(ref bfu) => {
bfu.bitfields()
.iter().any(|b| {
self.cannot_derive_debug.contains(&b.ty())
})
}
}
});
if fields_cant_derive {
if fields_cannot_derive {
return self.insert(id);
}
false

ConstrainResult::Same
},

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);
}
let inner_type = self.ctx.resolve_type(inner).canonical_type(self.ctx);
if let TypeKind::Function(ref sig) = *inner_type.kind() {
if !sig.can_trivially_derive_debug(&self.ctx, ()) {
return self.insert(id);
}
false
}
ConstrainResult::Same
},

TypeKind::TemplateInstantiation(ref template) => {
let args_cant_derive = template.template_arguments()
let args_cannot_derive = template.template_arguments()
.iter()
.any(|arg| self.cant_derive_debug.contains(&arg));
if args_cant_derive {
.any(|arg| self.cannot_derive_debug.contains(&arg));
if args_cannot_derive {
return self.insert(id);
}
let ty_cant_derive = template.template_definition()

let ty_cannot_derive = template.template_definition()
.into_resolver()
.through_type_refs()
.through_type_aliases()
Expand All @@ -268,19 +274,26 @@ impl<'ctx, 'gen> MonotoneFramework for CantDeriveDebugAnalysis<'ctx, 'gen> {
// 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))
.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 {
.unwrap_or_else(|| {
self.cannot_derive_debug.contains(&template.template_definition())
});
if ty_cannot_derive {
return self.insert(id);
}
false

ConstrainResult::Same
},
}
}
Expand All @@ -297,8 +310,8 @@ impl<'ctx, 'gen> MonotoneFramework for CantDeriveDebugAnalysis<'ctx, 'gen> {
}
}

impl<'ctx, 'gen> From<CantDeriveDebugAnalysis<'ctx, 'gen>> for HashSet<ItemId> {
fn from(analysis: CantDeriveDebugAnalysis<'ctx, 'gen>) -> Self {
analysis.cant_derive_debug
impl<'ctx, 'gen> From<CannotDeriveDebug<'ctx, 'gen>> for HashSet<ItemId> {
fn from(analysis: CannotDeriveDebug<'ctx, 'gen>) -> Self {
analysis.cannot_derive_debug
}
}
Loading