Be pessimistic about `derive(Debug)` and blacklisted types

11 files changed, 28 insertions, 44 deletions

diff --git a/src/ir/analysis/derive_debug.rs b/src/ir/analysis/derive_debug.rs
index 14630155..76b889df 100644
--- a/src/ir/analysis/derive_debug.rs
+++ b/src/ir/analysis/derive_debug.rs
@@ -90,6 +90,13 @@ impl<'ctx, 'gen> CannotDeriveDebug<'ctx, 'gen> {
 
         ConstrainResult::Changed
     }
+
+    /// A type is not `Debug` if we've determined it is not debug, or if it is
+    /// blacklisted.
+    fn is_not_debug(&self, id: ItemId) -> bool {
+        self.cannot_derive_debug.contains(&id) ||
+            !self.ctx.whitelisted_items().contains(&id)
+    }
 }
 
 impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
@@ -120,6 +127,15 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'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 Debug");
+            return ConstrainResult::Same;
+        }
+
         let item = self.ctx.resolve_item(id);
         let ty = match item.as_type() {
             Some(ty) => ty,
@@ -129,7 +145,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
             }
         };
 
-        if ty.is_opaque(self.ctx, item) {
+        if item.is_opaque(self.ctx, &()) {
             let layout_can_derive = ty.layout(self.ctx).map_or(true, |l| {
                 l.opaque().can_trivially_derive_debug()
             });
@@ -176,7 +192,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
             }
 
             TypeKind::Array(t, len) => {
-                if self.cannot_derive_debug.contains(&t) {
+                if self.is_not_debug(t) {
                     trace!(
                         "    arrays of T for which we cannot derive Debug \
                             also cannot derive Debug"
@@ -196,7 +212,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
             TypeKind::ResolvedTypeRef(t) |
             TypeKind::TemplateAlias(t, _) |
             TypeKind::Alias(t) => {
-                if self.cannot_derive_debug.contains(&t) {
+                if self.is_not_debug(t) {
                     trace!(
                         "    aliases and type refs to T which cannot derive \
                             Debug also cannot derive Debug"
@@ -237,7 +253,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
 
                 let bases_cannot_derive =
                     info.base_members().iter().any(|base| {
-                        self.cannot_derive_debug.contains(&base.ty)
+                        self.is_not_debug(base.ty)
                     });
                 if bases_cannot_derive {
                     trace!(
@@ -250,11 +266,11 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
                 let fields_cannot_derive =
                     info.fields().iter().any(|f| match *f {
                         Field::DataMember(ref data) => {
-                            self.cannot_derive_debug.contains(&data.ty())
+                            self.is_not_debug(data.ty())
                         }
                         Field::Bitfields(ref bfu) => {
                             bfu.bitfields().iter().any(|b| {
-                                self.cannot_derive_debug.contains(&b.ty())
+                                self.is_not_debug(b.ty())
                             })
                         }
                     });
@@ -287,7 +303,7 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
             TypeKind::TemplateInstantiation(ref template) => {
                 let args_cannot_derive =
                     template.template_arguments().iter().any(|arg| {
-                        self.cannot_derive_debug.contains(&arg)
+                        self.is_not_debug(*arg)
                     });
                 if args_cannot_derive {
                     trace!(
@@ -301,8 +317,8 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'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_debug.contains(
-                    &template.template_definition(),
+                let def_cannot_derive = self.is_not_debug(
+                    template.template_definition(),
                 );
                 if def_cannot_derive {
                     trace!(
diff --git a/tests/expectations/tests/derive-hash-blacklisting.rs b/tests/expectations/tests/derive-hash-blacklisting.rs
index d24586be..5d1a8d37 100644
--- a/tests/expectations/tests/derive-hash-blacklisting.rs
+++ b/tests/expectations/tests/derive-hash-blacklisting.rs
@@ -8,7 +8,6 @@
 /// This would derive(Hash, Eq, PartialEq) if it didn't contain a blacklisted type,
 /// causing us to conservatively avoid deriving hash/Eq/PartialEq for it.
 #[repr(C)]
-#[derive(Debug, Copy)]
 pub struct WhitelistedOne {
     pub a: Blacklisted<::std::os::raw::c_int>,
 }
@@ -24,15 +23,11 @@ fn bindgen_test_layout_WhitelistedOne() {
                 "Alignment of field: " , stringify ! ( WhitelistedOne ) , "::"
                 , stringify ! ( a ) ));
 }
-impl Clone for WhitelistedOne {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for WhitelistedOne {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 /// This can't derive(Hash/Eq) even if it didn't contain a blacklisted type.
 #[repr(C)]
-#[derive(Debug, Copy)]
 pub struct WhitelistedTwo {
     pub b: Blacklisted<f32>,
 }
@@ -48,9 +43,6 @@ fn bindgen_test_layout_WhitelistedTwo() {
                 "Alignment of field: " , stringify ! ( WhitelistedTwo ) , "::"
                 , stringify ! ( b ) ));
 }
-impl Clone for WhitelistedTwo {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for WhitelistedTwo {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
diff --git a/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs b/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs
index 540b5e7f..d0ccacef 100644
--- a/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs
+++ b/tests/expectations/tests/issue-584-stylo-template-analysis-panic.rs
@@ -22,7 +22,6 @@ impl Clone for A {
     fn clone(&self) -> Self { *self }
 }
 #[repr(C)]
-#[derive(Debug, Copy, Clone)]
 pub struct e<c> {
     pub d: RefPtr<c>,
     pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<c>>,
@@ -36,7 +35,6 @@ pub struct f {
     pub _address: u8,
 }
 #[repr(C)]
-#[derive(Debug, Copy)]
 pub struct g {
     pub h: f,
 }
@@ -51,14 +49,10 @@ fn bindgen_test_layout_g() {
                 "Alignment of field: " , stringify ! ( g ) , "::" , stringify
                 ! ( h ) ));
 }
-impl Clone for g {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for g {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[repr(C)]
-#[derive(Debug, Copy)]
 pub struct b {
     pub _base: g,
 }
@@ -69,9 +63,6 @@ fn bindgen_test_layout_b() {
     assert_eq! (::std::mem::align_of::<b>() , 1usize , concat ! (
                 "Alignment of " , stringify ! ( b ) ));
 }
-impl Clone for b {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for b {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
diff --git a/tests/expectations/tests/issue-645-cannot-find-type-T-in-this-scope.rs b/tests/expectations/tests/issue-645-cannot-find-type-T-in-this-scope.rs
index d705ffa3..87228ec6 100644
--- a/tests/expectations/tests/issue-645-cannot-find-type-T-in-this-scope.rs
+++ b/tests/expectations/tests/issue-645-cannot-find-type-T-in-this-scope.rs
@@ -6,7 +6,6 @@
 #[derive(Clone, Copy, Debug)] pub struct RefPtr<T>(T);
 
 #[repr(C)]
-#[derive(Debug, Copy, Clone)]
 pub struct HasRefPtr<T> {
     pub refptr_member: RefPtr<HasRefPtr_TypedefOfT<T>>,
     pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>>,
diff --git a/tests/expectations/tests/issue-662-part-2.rs b/tests/expectations/tests/issue-662-part-2.rs
index 8225cf18..a1b97ca6 100644
--- a/tests/expectations/tests/issue-662-part-2.rs
+++ b/tests/expectations/tests/issue-662-part-2.rs
@@ -15,7 +15,6 @@ impl <T> Default for nsMainThreadPtrHolder<T> {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
 #[repr(C)]
-#[derive(Debug, Copy, Clone)]
 pub struct nsMainThreadPtrHandle<U> {
     pub mPtr: RefPtr<nsMainThreadPtrHolder<U>>,
     pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<U>>,
diff --git a/tests/expectations/tests/issue-944-derive-copy-and-blacklisting.rs b/tests/expectations/tests/issue-944-derive-copy-and-blacklisting.rs
index ed696c94..10e54056 100644
--- a/tests/expectations/tests/issue-944-derive-copy-and-blacklisting.rs
+++ b/tests/expectations/tests/issue-944-derive-copy-and-blacklisting.rs
@@ -3,11 +3,10 @@
 
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
 
-#[derive(Debug)] pub struct BlacklistMe(u8);
+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,
 }
diff --git a/tests/expectations/tests/no-derive-debug.rs b/tests/expectations/tests/no-derive-debug.rs
index 311b3767..f1616e64 100644
--- a/tests/expectations/tests/no-derive-debug.rs
+++ b/tests/expectations/tests/no-derive-debug.rs
@@ -9,7 +9,6 @@
 /// and replacement for another type that doesn't implement it would prevent it
 /// from building if --no-derive-debug didn't work.
 #[repr(C)]
-#[derive(Copy)]
 pub struct bar {
     pub foo: foo,
     pub baz: ::std::os::raw::c_int,
@@ -31,9 +30,6 @@ fn bindgen_test_layout_bar() {
                 "Alignment of field: " , stringify ! ( bar ) , "::" ,
                 stringify ! ( baz ) ));
 }
-impl Clone for bar {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for bar {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
diff --git a/tests/expectations/tests/no-derive-default.rs b/tests/expectations/tests/no-derive-default.rs
index c212f4bd..cdb89eb1 100644
--- a/tests/expectations/tests/no-derive-default.rs
+++ b/tests/expectations/tests/no-derive-default.rs
@@ -9,7 +9,6 @@
 /// and replacement for another type that doesn't implement it would prevent it
 /// from building if --no-derive-default didn't work.
 #[repr(C)]
-#[derive(Debug, Copy)]
 pub struct bar {
     pub foo: foo,
     pub baz: ::std::os::raw::c_int,
@@ -31,6 +30,3 @@ fn bindgen_test_layout_bar() {
                 "Alignment of field: " , stringify ! ( bar ) , "::" ,
                 stringify ! ( baz ) ));
 }
-impl Clone for bar {
-    fn clone(&self) -> Self { *self }
-}
diff --git a/tests/expectations/tests/no-recursive-whitelisting.rs b/tests/expectations/tests/no-recursive-whitelisting.rs
index e2aba543..30dc9e69 100644
--- a/tests/expectations/tests/no-recursive-whitelisting.rs
+++ b/tests/expectations/tests/no-recursive-whitelisting.rs
@@ -6,7 +6,6 @@
 pub enum Bar {}
 
 #[repr(C)]
-#[derive(Debug, Copy)]
 pub struct Foo {
     pub baz: *mut Bar,
 }
@@ -22,9 +21,6 @@ fn bindgen_test_layout_Foo() {
                 "Alignment of field: " , stringify ! ( Foo ) , "::" ,
                 stringify ! ( baz ) ));
 }
-impl Clone for Foo {
-    fn clone(&self) -> Self { *self }
-}
 impl Default for Foo {
     fn default() -> Self { unsafe { ::std::mem::zeroed() } }
 }
diff --git a/tests/expectations/tests/opaque-template-inst-member.rs b/tests/expectations/tests/opaque-template-inst-member.rs
index 32995873..0bef4c86 100644
--- a/tests/expectations/tests/opaque-template-inst-member.rs
+++ b/tests/expectations/tests/opaque-template-inst-member.rs
@@ -5,7 +5,7 @@
 
 
 #[repr(C)]
-#[derive(Default, Copy, Clone, Hash, PartialEq, Eq)]
+#[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq)]
 pub struct OpaqueTemplate {
 }
 /// This should not end up deriving Debug/Hash/PartialEq because its `mBlah` field cannot derive
diff --git a/tests/headers/issue-944-derive-copy-and-blacklisting.hpp b/tests/headers/issue-944-derive-copy-and-blacklisting.hpp
index 7e033728..657e0d84 100644
--- a/tests/headers/issue-944-derive-copy-and-blacklisting.hpp
+++ b/tests/headers/issue-944-derive-copy-and-blacklisting.hpp
@@ -1,4 +1,4 @@
-// bindgen-flags: --blacklist-type BlacklistMe --raw-line '#[derive(Debug)] pub struct BlacklistMe(u8);'
+// bindgen-flags: --blacklist-type BlacklistMe --raw-line 'pub struct BlacklistMe(u8);'
 
 struct BlacklistMe {};