Auto merge of rust-lang#847 - fitzgen:derive-debug-opaque-types, r=emilio

Derive debug and opaque types

See each commit for details.

Follow up to rust-lang#842.

r? @emilio or @Manishearth

Author: bors-servo

src/ir/analysis/derive_debug.rs

@@ -11,7 +11,6 @@ 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;
 
@@ -79,13 +78,16 @@ impl<'ctx, 'gen> CannotDeriveDebug<'ctx, 'gen> {
     }
 
     fn insert(&mut self, id: ItemId) -> ConstrainResult {
+        trace!("inserting {:?} into the cannot_derive_debug set", id);
+
         let was_not_already_in_set = self.cannot_derive_debug.insert(id);
         assert!(
             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
         );
+
         ConstrainResult::Changed
     }
 }
@@ -128,16 +130,35 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
     }
 
     fn constrain(&mut self, id: ItemId) -> ConstrainResult {
+        trace!("constrain: {:?}", id);
+
         if self.cannot_derive_debug.contains(&id) {
+            trace!("    already know it cannot derive Debug");
             return ConstrainResult::Same;
         }
 
         let item = self.ctx.resolve_item(id);
         let ty = match item.as_type() {
-            None => return ConstrainResult::Same,
-            Some(ty) => ty
+            Some(ty) => ty,
+            None => {
+                trace!("    not a type; ignoring");
+                return ConstrainResult::Same;
+            }
         };
 
+        if ty.is_opaque(self.ctx, item) {
+            let layout_can_derive = ty.layout(self.ctx).map_or(true, |l| {
+                l.opaque().can_trivially_derive_debug(self.ctx, ())
+            });
+            return if layout_can_derive {
+                trace!("    we can trivially derive Debug for the layout");
+                ConstrainResult::Same
+            } else {
+                trace!("    we cannot derive Debug for the layout");
+                self.insert(id)
+            };
+        }
+
         match *ty.kind() {
             // Handle the simple cases. These can derive debug without further
             // information.
@@ -155,61 +176,59 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
             TypeKind::ObjCInterface(..) |
             TypeKind::ObjCId |
             TypeKind::ObjCSel => {
+                trace!("    simple type that can always derive Debug");
                 ConstrainResult::Same
-            },
-
-            TypeKind::Opaque => {
-                if ty.layout(self.ctx)
-                    .map_or(true, |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
-                        ConstrainResult::Same
-                    } else {
-                        self.insert(id)
-                    }
-            },
+            }
 
             TypeKind::Array(t, len) => {
                 if self.cannot_derive_debug.contains(&t) {
+                    trace!("    arrays of T for which we cannot derive Debug \
+                            also cannot derive Debug");
                     return self.insert(id);
                 }
 
                 if len <= RUST_DERIVE_IN_ARRAY_LIMIT {
+                    trace!("    array is small enough to derive Debug");
                     ConstrainResult::Same
                 } else {
+                    trace!("    array is too large to derive Debug");
                     self.insert(id)
                 }
-            },
+            }
 
             TypeKind::ResolvedTypeRef(t) |
             TypeKind::TemplateAlias(t, _) |
             TypeKind::Alias(t) => {
                 if self.cannot_derive_debug.contains(&t) {
+                    trace!("    aliases and type refs to T which cannot derive \
+                            Debug also cannot derive Debug");
                     self.insert(id)
                 } else {
+                    trace!("    aliases and type refs to T which can derive \
+                            Debug can also derive Debug");
                     ConstrainResult::Same
                 }
-            },
+            }
 
             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 ConstrainResult::Same;
-                    } else {
-                        return self.insert(id);
-                    }
-                }
+                assert!(
+                    !info.has_non_type_template_params(),
+                    "The early ty.is_opaque check should have handled this case"
+                );
 
                 if info.kind() == CompKind::Union {
                     if self.ctx.options().unstable_rust {
+                        trace!("    cannot derive Debug for Rust unions");
                         return self.insert(id);
                     }
 
                     if ty.layout(self.ctx)
                         .map_or(true,
                                 |l| l.opaque().can_trivially_derive_debug(self.ctx, ())) {
+                        trace!("    union layout can trivially derive Debug");
                         return ConstrainResult::Same;
                     } else {
+                        trace!("    union layout cannot derive Debug");
                         return self.insert(id);
                     }
                 }
@@ -218,6 +237,8 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
                     .iter()
                     .any(|base| self.cannot_derive_debug.contains(&base.ty));
                 if bases_cannot_derive {
+                    trace!("    base members cannot derive Debug, so we can't \
+                            either");
                     return self.insert(id);
                 }
 
@@ -237,69 +258,57 @@ impl<'ctx, 'gen> MonotoneFramework for CannotDeriveDebug<'ctx, 'gen> {
                         }
                     });
                 if fields_cannot_derive {
+                    trace!("    fields cannot derive Debug, so we can't either");
                     return self.insert(id);
                 }
 
+                trace!("    comp can derive Debug");
                 ConstrainResult::Same
-            },
+            }
 
             TypeKind::Pointer(inner) => {
                 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, ()) {
+                        trace!("    function pointer that can't trivially derive Debug");
                         return self.insert(id);
                     }
                 }
+                trace!("    pointers can derive Debug");
                 ConstrainResult::Same
-            },
+            }
 
             TypeKind::TemplateInstantiation(ref template) => {
                 let args_cannot_derive = template.template_arguments()
                     .iter()
                     .any(|arg| self.cannot_derive_debug.contains(&arg));
                 if args_cannot_derive {
+                    trace!("    template args cannot derive Debug, so \
+                            insantiation can't either");
                     return self.insert(id);
                 }
 
-                let template_definition = template.template_definition()
-                    .into_resolver()
-                    .through_type_refs()
-                    .through_type_aliases()
-                    .resolve(self.ctx);
-
-                let ty_cannot_derive = template_definition
-                    .as_type()
-                    .expect("Instantiations of a non-type?")
-                    .as_comp()
-                    .and_then(|c| {
-                        // For non-type template parameters, or opaque template
-                        // definitions, we generate an opaque blob, and in this
-                        // case the instantiation has a better idea of the
-                        // layout than the definition does.
-                        if template_definition.is_opaque(self.ctx, &()) ||
-                            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.cannot_derive_debug.contains(&template.template_definition())
-                    });
-                if ty_cannot_derive {
+                assert!(
+                    !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());
+                if def_cannot_derive {
+                    trace!("    template definition cannot derive Debug, so \
+                            insantiation can't either");
                     return self.insert(id);
                 }
 
+                trace!("    template instantiation can derive Debug");
                 ConstrainResult::Same
-            },
+            }
+
+            TypeKind::Opaque => {
+                unreachable!(
+                    "The early ty.is_opaque check should have handled this case"
+                )
+            }
         }
     }
 

src/ir/ty.rs

@@ -359,6 +359,7 @@ impl IsOpaque for Type {
             TypeKind::Opaque => true,
             TypeKind::TemplateInstantiation(ref inst) => inst.is_opaque(ctx, item),
             TypeKind::Comp(ref comp) => comp.is_opaque(ctx, &()),
+            TypeKind::ResolvedTypeRef(to) => to.is_opaque(ctx, &()),
             _ => false,
         }
     }

tests/expectations/tests/issue-372.rs

@@ -83,7 +83,6 @@ pub mod root {
         ai = 11,
     }
     #[repr(C)]
-    #[derive(Copy)]
     pub struct F {
         pub w: [u64; 33usize],
     }
@@ -99,21 +98,7 @@ pub mod root {
                     "Alignment of field: " , stringify ! ( F ) , "::" ,
                     stringify ! ( w ) ));
     }
-    impl Clone for F {
-        fn clone(&self) -> Self { *self }
-    }
     impl Default for F {
         fn default() -> Self { unsafe { ::std::mem::zeroed() } }
     }
-    #[test]
-    fn __bindgen_test_layout_C_open0_n_close0_instantiation() {
-        assert_eq!(::std::mem::size_of::<[u64; 33usize]>() , 264usize , concat
-                   ! (
-                   "Size of template specialization: " , stringify ! (
-                   [u64; 33usize] ) ));
-        assert_eq!(::std::mem::align_of::<[u64; 33usize]>() , 8usize , concat
-                   ! (
-                   "Alignment of template specialization: " , stringify ! (
-                   [u64; 33usize] ) ));
-    }
 }

tests/expectations/tests/libclang-3.8/partial-specialization-and-inheritance.rs

@@ -45,12 +45,3 @@ impl Usage {
         __bindgen_tmp
     }
 }
-#[test]
-fn __bindgen_test_layout__bindgen_ty_id_21_open0__bindgen_ty_id_19_close0_instantiation() {
-    assert_eq!(::std::mem::size_of::<[u32; 2usize]>() , 8usize , concat ! (
-               "Size of template specialization: " , stringify ! (
-               [u32; 2usize] ) ));
-    assert_eq!(::std::mem::align_of::<[u32; 2usize]>() , 4usize , concat ! (
-               "Alignment of template specialization: " , stringify ! (
-               [u32; 2usize] ) ));
-}

tests/expectations/tests/libclang-3.9/partial-specialization-and-inheritance.rs

@@ -33,12 +33,3 @@ fn bindgen_test_layout_Usage() {
 impl Clone for Usage {
     fn clone(&self) -> Self { *self }
 }
-#[test]
-fn __bindgen_test_layout__bindgen_ty_id_20_open0__bindgen_ty_id_18_close0_instantiation() {
-    assert_eq!(::std::mem::size_of::<[u32; 2usize]>() , 8usize , concat ! (
-               "Size of template specialization: " , stringify ! (
-               [u32; 2usize] ) ));
-    assert_eq!(::std::mem::align_of::<[u32; 2usize]>() , 4usize , concat ! (
-               "Alignment of template specialization: " , stringify ! (
-               [u32; 2usize] ) ));
-}

tests/expectations/tests/libclang-4/partial-specialization-and-inheritance.rs

@@ -33,12 +33,3 @@ fn bindgen_test_layout_Usage() {
 impl Clone for Usage {
     fn clone(&self) -> Self { *self }
 }
-#[test]
-fn __bindgen_test_layout__bindgen_ty_id_20_open0__bindgen_ty_id_18_close0_instantiation() {
-    assert_eq!(::std::mem::size_of::<[u32; 2usize]>() , 8usize , concat ! (
-               "Size of template specialization: " , stringify ! (
-               [u32; 2usize] ) ));
-    assert_eq!(::std::mem::align_of::<[u32; 2usize]>() , 4usize , concat ! (
-               "Alignment of template specialization: " , stringify ! (
-               [u32; 2usize] ) ));
-}

tests/expectations/tests/non-type-params.rs

@@ -7,7 +7,7 @@
 pub type Array16 = u8;
 pub type ArrayInt4 = [u32; 4usize];
 #[repr(C)]
-#[derive(Debug, Copy)]
+#[derive(Debug, Default, Copy)]
 pub struct UsesArray {
     pub array_char_16: [u8; 16usize],
     pub array_bool_8: [u8; 8usize],
@@ -38,33 +38,3 @@ fn bindgen_test_layout_UsesArray() {
 impl Clone for UsesArray {
     fn clone(&self) -> Self { *self }
 }
-impl Default for UsesArray {
-    fn default() -> Self { unsafe { ::std::mem::zeroed() } }
-}
-#[test]
-fn __bindgen_test_layout_Array_open0_int_close0_instantiation() {
-    assert_eq!(::std::mem::size_of::<[u32; 4usize]>() , 16usize , concat ! (
-               "Size of template specialization: " , stringify ! (
-               [u32; 4usize] ) ));
-    assert_eq!(::std::mem::align_of::<[u32; 4usize]>() , 4usize , concat ! (
-               "Alignment of template specialization: " , stringify ! (
-               [u32; 4usize] ) ));
-}
-#[test]
-fn __bindgen_test_layout_Array_open0_char_close0_instantiation() {
-    assert_eq!(::std::mem::size_of::<[u8; 16usize]>() , 16usize , concat ! (
-               "Size of template specialization: " , stringify ! (
-               [u8; 16usize] ) ));
-    assert_eq!(::std::mem::align_of::<[u8; 16usize]>() , 1usize , concat ! (
-               "Alignment of template specialization: " , stringify ! (
-               [u8; 16usize] ) ));
-}
-#[test]
-fn __bindgen_test_layout_Array_open0_bool__close0_instantiation() {
-    assert_eq!(::std::mem::size_of::<[u8; 8usize]>() , 8usize , concat ! (
-               "Size of template specialization: " , stringify ! (
-               [u8; 8usize] ) ));
-    assert_eq!(::std::mem::align_of::<[u8; 8usize]>() , 1usize , concat ! (
-               "Alignment of template specialization: " , stringify ! (
-               [u8; 8usize] ) ));
-}