Auto merge of rust-lang#50536 - leodasvacas:report-fullfilment-errors-in-copy-derive, r=estebank

Better error reporting in Copy derive

In Copy derive, report all fulfillment erros when present and do not report errors for types tainted with `TyErr`. Also report all fields which are not Copy rather than just the first.

Also refactored `fn fully_normalize`, removing the not very useful helper function along with a FIXME to the closed issue rust-lang#26721 that looks out of context now.

Fixes rust-lang#50480

r? @estebank

Author: bors

src/librustc/middle/stability.rs

@@ -764,7 +764,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
                                      "unions with `Drop` implementations are unstable");
                 } else {
                     let param_env = self.tcx.param_env(def_id);
-                    if !param_env.can_type_implement_copy(self.tcx, ty, item.span).is_ok() {
+                    if !param_env.can_type_implement_copy(self.tcx, ty).is_ok() {
                         emit_feature_err(&self.tcx.sess.parse_sess,
                                         "untagged_unions", item.span, GateIssue::Language,
                                         "unions with non-`Copy` fields are unstable");

src/librustc/traits/mod.rs

@@ -674,7 +674,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         // we move over to lazy normalization *anyway*.
         let fulfill_cx = FulfillmentContext::new_ignoring_regions();
 
-        let predicates = match fully_normalize_with_fulfillcx(
+        let predicates = match fully_normalize(
             &infcx,
             fulfill_cx,
             cause,
@@ -734,31 +734,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     })
 }
 
-pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
-                                          cause: ObligationCause<'tcx>,
-                                          param_env: ty::ParamEnv<'tcx>,
-                                          value: &T)
-                                          -> Result<T, Vec<FulfillmentError<'tcx>>>
-    where T : TypeFoldable<'tcx>
-{
-    // FIXME (@jroesch) ISSUE 26721
-    // I'm not sure if this is a bug or not, needs further investigation.
-    // It appears that by reusing the fulfillment_cx here we incur more
-    // obligations and later trip an assertion on regionck.rs line 337.
-    //
-    // The two possibilities I see is:
-    //      - normalization is not actually fully happening and we
-    //        have a bug else where
-    //      - we are adding a duplicate bound into the list causing
-    //        its size to change.
-    //
-    // I think we should probably land this refactor and then come
-    // back to this is a follow-up patch.
-    let fulfillcx = FulfillmentContext::new();
-    fully_normalize_with_fulfillcx(infcx, fulfillcx, cause, param_env, value)
-}
-
-pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>(
+pub fn fully_normalize<'a, 'gcx, 'tcx, T>(
     infcx: &InferCtxt<'a, 'gcx, 'tcx>,
     mut fulfill_cx: FulfillmentContext<'tcx>,
     cause: ObligationCause<'tcx>,
@@ -779,13 +755,7 @@ pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>(
     }
 
     debug!("fully_normalize: select_all_or_error start");
-    match fulfill_cx.select_all_or_error(infcx) {
-        Ok(()) => { }
-        Err(e) => {
-            debug!("fully_normalize: error={:?}", e);
-            return Err(e);
-        }
-    }
+    fulfill_cx.select_all_or_error(infcx)?;
     debug!("fully_normalize: select_all_or_error complete");
     let resolved_value = infcx.resolve_type_vars_if_possible(&normalized_value);
     debug!("fully_normalize: resolved_value={:?}", resolved_value);

src/librustc/traits/specialize/mod.rs

@@ -196,6 +196,7 @@ pub(super) fn specializes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         // that this always succeeds.
         let impl1_trait_ref =
             match traits::fully_normalize(&infcx,
+                                          FulfillmentContext::new(),
                                           ObligationCause::dummy(),
                                           penv,
                                           &impl1_trait_ref) {

src/librustc/ty/util.rs

@@ -16,7 +16,7 @@ use hir::map::{DefPathData, Node};
 use hir;
 use ich::NodeIdHashingMode;
 use middle::const_val::ConstVal;
-use traits;
+use traits::{self, ObligationCause};
 use ty::{self, Ty, TyCtxt, TypeFoldable};
 use ty::fold::TypeVisitor;
 use ty::subst::UnpackedKind;
@@ -166,9 +166,9 @@ impl IntTypeExt for attr::IntType {
 }
 
 
-#[derive(Copy, Clone)]
+#[derive(Clone)]
 pub enum CopyImplementationError<'tcx> {
-    InfrigingField(&'tcx ty::FieldDef),
+    InfrigingFields(Vec<&'tcx ty::FieldDef>),
     NotAnAdt,
     HasDestructor,
 }
@@ -191,7 +191,7 @@ pub enum Representability {
 impl<'tcx> ty::ParamEnv<'tcx> {
     pub fn can_type_implement_copy<'a>(self,
                                        tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                                       self_type: Ty<'tcx>, span: Span)
+                                       self_type: Ty<'tcx>)
                                        -> Result<(), CopyImplementationError<'tcx>> {
         // FIXME: (@jroesch) float this code up
         tcx.infer_ctxt().enter(|infcx| {
@@ -207,22 +207,29 @@ impl<'tcx> ty::ParamEnv<'tcx> {
                 _ => return Err(CopyImplementationError::NotAnAdt),
             };
 
-            let field_implements_copy = |field: &ty::FieldDef| {
-                let cause = traits::ObligationCause::dummy();
-                match traits::fully_normalize(&infcx, cause, self, &field.ty(tcx, substs)) {
-                    Ok(ty) => !infcx.type_moves_by_default(self, ty, span),
-                    Err(..) => false,
-                }
-            };
-
+            let mut infringing = Vec::new();
             for variant in &adt.variants {
                 for field in &variant.fields {
-                    if !field_implements_copy(field) {
-                        return Err(CopyImplementationError::InfrigingField(field));
+                    let span = tcx.def_span(field.did);
+                    let ty = field.ty(tcx, substs);
+                    if ty.references_error() {
+                        continue;
                     }
+                    let cause = ObligationCause { span, ..ObligationCause::dummy() };
+                    let ctx = traits::FulfillmentContext::new();
+                    match traits::fully_normalize(&infcx, ctx, cause, self, &ty) {
+                        Ok(ty) => if infcx.type_moves_by_default(self, ty, span) {
+                            infringing.push(field);
+                        }
+                        Err(errors) => {
+                            infcx.report_fulfillment_errors(&errors, None, false);
+                        }
+                    };
                 }
             }
-
+            if !infringing.is_empty() {
+                return Err(CopyImplementationError::InfrigingFields(infringing));
+            }
             if adt.has_dtor(tcx) {
                 return Err(CopyImplementationError::HasDestructor);
             }

src/librustc_lint/builtin.rs

@@ -541,7 +541,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingCopyImplementations {
         if !ty.moves_by_default(cx.tcx, param_env, item.span) {
             return;
         }
-        if param_env.can_type_implement_copy(cx.tcx, ty, item.span).is_ok() {
+        if param_env.can_type_implement_copy(cx.tcx, ty).is_ok() {
             cx.span_lint(MISSING_COPY_IMPLEMENTATIONS,
                          item.span,
                          "type could implement `Copy`; consider adding `impl \

src/librustc_typeck/coherence/builtin.rs

@@ -111,24 +111,24 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     debug!("visit_implementation_of_copy: self_type={:?} (free)",
            self_type);
 
-    match param_env.can_type_implement_copy(tcx, self_type, span) {
+    match param_env.can_type_implement_copy(tcx, self_type) {
         Ok(()) => {}
-        Err(CopyImplementationError::InfrigingField(field)) => {
+        Err(CopyImplementationError::InfrigingFields(fields)) => {
             let item = tcx.hir.expect_item(impl_node_id);
             let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node {
                 tr.path.span
             } else {
                 span
             };
 
-            struct_span_err!(tcx.sess,
-                             span,
-                             E0204,
-                             "the trait `Copy` may not be implemented for this type")
-                .span_label(
-                    tcx.def_span(field.did),
-                    "this field does not implement `Copy`")
-                .emit()
+            let mut err = struct_span_err!(tcx.sess,
+                                          span,
+                                          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`");
+            }
+            err.emit()
         }
         Err(CopyImplementationError::NotAnAdt) => {
             let item = tcx.hir.expect_item(impl_node_id);

src/test/ui/issue-50480.rs

@@ -0,0 +1,17 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[derive(Clone, Copy)]
+//~^ ERROR the trait `Copy` may not be implemented for this type
+struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+//~^ ERROR cannot find type `NotDefined` in this scope
+//~| ERROR the trait bound `i32: std::iter::Iterator` is not satisfied
+
+fn main() {}

src/test/ui/issue-50480.stderr

@@ -0,0 +1,29 @@
+error[E0412]: cannot find type `NotDefined` in this scope
+  --> $DIR/issue-50480.rs:13:12
+   |
+LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |            ^^^^^^^^^^ not found in this scope
+
+error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied
+  --> $DIR/issue-50480.rs:13:24
+   |
+LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method
+   |
+   = help: the trait `std::iter::Iterator` is not implemented for `i32`
+
+error[E0204]: the trait `Copy` may not be implemented for this type
+  --> $DIR/issue-50480.rs:11:17
+   |
+LL | #[derive(Clone, Copy)]
+   |                 ^^^^
+LL | //~^ ERROR the trait `Copy` may not be implemented for this type
+LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                                                 --------  ------ this field does not implement `Copy`
+   |                                                 |
+   |                                                 this field does not implement `Copy`
+
+error: aborting due to 3 previous errors
+
+Some errors occurred: E0204, E0277, E0412.
+For more information about an error, try `rustc --explain E0204`.