Skip to content

Commit

Permalink
Do not derive Copy for blacklisted types
Browse files Browse the repository at this point in the history
Presumably they are blacklisted because we can't understand them properly, so be
pessimistic about what we can derive for it.

Fixes #944.
  • Loading branch information
fitzgen committed Sep 5, 2017
1 parent d23db77 commit 931edfc
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 8 deletions.
32 changes: 24 additions & 8 deletions src/ir/analysis/derive_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ impl<'ctx, 'gen> CannotDeriveCopy<'ctx, 'gen> {

ConstrainResult::Changed
}

/// A type is not `Copy` if we've determined it is not copy, or if it is
/// blacklisted.
fn is_not_copy(&self, id: ItemId) -> bool {
self.cannot_derive_copy.contains(&id) ||
!self.ctx.whitelisted_items().contains(&id)
}
}

impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
Expand Down Expand Up @@ -118,6 +125,15 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
return ConstrainResult::Same;
}

// If an item is reachable from the whitelisted items set, but isn't
// itself whitelisted, then it must be blacklisted. We assume that
// blacklisted items are not `Copy`, since they are presumably
// blacklisted because they are too complicated for us to understand.
if !self.ctx.whitelisted_items().contains(&id) {
trace!(" blacklisted items are assumed not to be Copy");
return ConstrainResult::Same;
}

let item = self.ctx.resolve_item(id);
let ty = match item.as_type() {
Some(ty) => ty,
Expand Down Expand Up @@ -163,7 +179,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
}

TypeKind::Array(t, len) => {
let cant_derive_copy = self.cannot_derive_copy.contains(&t);
let cant_derive_copy = self.is_not_copy(t);
if cant_derive_copy {
trace!(
" arrays of T for which we cannot derive Copy \
Expand All @@ -184,7 +200,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
TypeKind::ResolvedTypeRef(t) |
TypeKind::TemplateAlias(t, _) |
TypeKind::Alias(t) => {
let cant_derive_copy = self.cannot_derive_copy.contains(&t);
let cant_derive_copy = self.is_not_copy(t);
if cant_derive_copy {
trace!(
" arrays of T for which we cannot derive Copy \
Expand Down Expand Up @@ -237,7 +253,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {

let bases_cannot_derive =
info.base_members().iter().any(|base| {
self.cannot_derive_copy.contains(&base.ty)
self.is_not_copy(base.ty)
});
if bases_cannot_derive {
trace!(
Expand All @@ -250,11 +266,11 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
let fields_cannot_derive =
info.fields().iter().any(|f| match *f {
Field::DataMember(ref data) => {
self.cannot_derive_copy.contains(&data.ty())
self.is_not_copy(data.ty())
}
Field::Bitfields(ref bfu) => {
bfu.bitfields().iter().any(|b| {
self.cannot_derive_copy.contains(&b.ty())
self.is_not_copy(b.ty())
})
}
});
Expand All @@ -270,7 +286,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
TypeKind::TemplateInstantiation(ref template) => {
let args_cannot_derive =
template.template_arguments().iter().any(|arg| {
self.cannot_derive_copy.contains(&arg)
self.is_not_copy(*arg)
});
if args_cannot_derive {
trace!(
Expand All @@ -284,8 +300,8 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveCopy<'ctx, 'gen> {
!template.template_definition().is_opaque(self.ctx, &()),
"The early ty.is_opaque check should have handled this case"
);
let def_cannot_derive = self.cannot_derive_copy.contains(
&template.template_definition(),
let def_cannot_derive = self.is_not_copy(
template.template_definition(),
);
if def_cannot_derive {
trace!(
Expand Down
28 changes: 28 additions & 0 deletions tests/expectations/tests/issue-944-derive-copy-and-blacklisting.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

#[derive(Debug)] pub struct BlacklistMe(u8);

/// Because this type contains a blacklisted type, it should not derive Copy.
#[repr(C)]
#[derive(Debug)]
pub struct ShouldNotBeCopy {
pub a: BlacklistMe,
}
#[test]
fn bindgen_test_layout_ShouldNotBeCopy() {
assert_eq!(::std::mem::size_of::<ShouldNotBeCopy>() , 1usize , concat ! (
"Size of: " , stringify ! ( ShouldNotBeCopy ) ));
assert_eq! (::std::mem::align_of::<ShouldNotBeCopy>() , 1usize , concat !
( "Alignment of " , stringify ! ( ShouldNotBeCopy ) ));
assert_eq! (unsafe {
& ( * ( 0 as * const ShouldNotBeCopy ) ) . a as * const _ as
usize } , 0usize , concat ! (
"Alignment of field: " , stringify ! ( ShouldNotBeCopy ) ,
"::" , stringify ! ( a ) ));
}
impl Default for ShouldNotBeCopy {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
10 changes: 10 additions & 0 deletions tests/headers/issue-944-derive-copy-and-blacklisting.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// bindgen-flags: --blacklist-type BlacklistMe --raw-line '#[derive(Debug)] pub struct BlacklistMe(u8);'

struct BlacklistMe {};

/**
* Because this type contains a blacklisted type, it should not derive Copy.
*/
struct ShouldNotBeCopy {
BlacklistMe a;
};

0 comments on commit 931edfc

Please sign in to comment.