better errors when a Copy impl is not coherent

Author: compiler-errors

compiler/rustc_trait_selection/src/traits/misc.rs

@@ -11,7 +11,7 @@ use crate::traits::error_reporting::InferCtxtExt;
 
 #[derive(Clone)]
 pub enum CopyImplementationError<'tcx> {
-    InfrigingFields(Vec<&'tcx ty::FieldDef>),
+    InfrigingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>)>),
     NotAnAdt,
     HasDestructor,
 }
@@ -67,7 +67,7 @@ pub fn can_type_implement_copy<'tcx>(
                 match traits::fully_normalize(&infcx, ctx, cause, param_env, ty) {
                     Ok(ty) => {
                         if !infcx.type_is_copy_modulo_regions(param_env, ty, span) {
-                            infringing.push(field);
+                            infringing.push((field, ty));
                         }
                     }
                     Err(errors) => {

compiler/rustc_typeck/src/coherence/builtin.rs

@@ -91,8 +91,40 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
                 E0204,
                 "the trait `Copy` may not be implemented for this type"
             );
-            for span in fields.iter().map(|f| tcx.def_span(f.did)) {
-                err.span_label(span, "this field does not implement `Copy`");
+            for (field, ty) in fields {
+                let field_span = tcx.def_span(field.did);
+                err.span_label(field_span, "this field does not implement `Copy`");
+                // Spin up a new FulfillmentContext, so we can get the _precise_ reason
+                // why this field does not implement Copy. This is useful because sometimes
+                // it is not immediately clear why Copy is not implemented for a field, since
+                // all we point at is the field itself.
+                tcx.infer_ctxt().enter(|infcx| {
+                    let mut fulfill_cx = traits::FulfillmentContext::new_ignoring_regions();
+                    fulfill_cx.register_bound(
+                        &infcx,
+                        param_env,
+                        ty,
+                        tcx.lang_items().copy_trait().unwrap(),
+                        traits::ObligationCause::dummy_with_span(field_span),
+                    );
+                    for error in fulfill_cx.select_all_or_error(&infcx) {
+                        let error_predicate = error.obligation.predicate;
+                        // Only note if it's not the root obligation, otherwise it's trivial and
+                        // should be self-explanatory (i.e. a field literally doesn't implement Copy).
+
+                        // FIXME: This error could be more descriptive, especially if the error_predicate
+                        // contains a foreign type or if it's a deeply nested type...
+                        if error_predicate != error.root_obligation.predicate {
+                            err.span_note(
+                                error.obligation.cause.span,
+                                &format!(
+                                    "the `Copy` impl for `{}` requires that `{}`",
+                                    ty, error_predicate
+                                ),
+                            );
+                        }
+                    }
+                });
             }
             err.emit();
         }

src/test/ui/coherence/deep-bad-copy-reason.rs

@@ -0,0 +1,40 @@
+#![feature(extern_types)]
+
+extern "Rust" {
+    type OpaqueListContents;
+}
+
+pub struct ListS<T> {
+    len: usize,
+    data: [T; 0],
+    opaque: OpaqueListContents,
+}
+
+pub struct Interned<'a, T>(&'a T);
+
+impl<'a, T> Clone for Interned<'a, T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<'a, T> Copy for Interned<'a, T> {}
+
+pub struct List<'tcx, T>(Interned<'tcx, ListS<T>>);
+//~^ NOTE this field does not implement `Copy`
+//~| NOTE the `Copy` impl for `Interned<'tcx, ListS<T>>` requires that `OpaqueListContents: Sized`
+
+impl<'tcx, T> Clone for List<'tcx, T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<'tcx, T> Copy for List<'tcx, T> {}
+//~^ ERROR the trait `Copy` may not be implemented for this type
+
+fn assert_is_copy<T: Copy>() {}
+
+fn main() {
+    assert_is_copy::<List<'static, ()>>();
+}

src/test/ui/coherence/deep-bad-copy-reason.stderr

@@ -0,0 +1,18 @@
+error[E0204]: the trait `Copy` may not be implemented for this type
+  --> $DIR/deep-bad-copy-reason.rs:33:15
+   |
+LL | pub struct List<'tcx, T>(Interned<'tcx, ListS<T>>);
+   |                          ------------------------ this field does not implement `Copy`
+...
+LL | impl<'tcx, T> Copy for List<'tcx, T> {}
+   |               ^^^^
+   |
+note: the `Copy` impl for `Interned<'tcx, ListS<T>>` requires that `OpaqueListContents: Sized`
+  --> $DIR/deep-bad-copy-reason.rs:23:26
+   |
+LL | pub struct List<'tcx, T>(Interned<'tcx, ListS<T>>);
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0204`.

src/test/ui/union/union-copy.stderr

@@ -6,6 +6,12 @@ LL |     a: std::mem::ManuallyDrop<String>
 ...
 LL | impl Copy for W {}
    |      ^^^^
+   |
+note: the `Copy` impl for `ManuallyDrop<String>` requires that `String: Copy`
+  --> $DIR/union-copy.rs:8:5
+   |
+LL |     a: std::mem::ManuallyDrop<String>
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error