Rollup merge of rust-lang#65318 - estebank:coherence, r=varkor

Call out the types that are non local on E0117

CC rust-lang#24745.

src/librustc/traits/coherence.rs

@@ -237,7 +237,7 @@ pub fn trait_ref_is_local_or_fundamental<'tcx>(
 }
 
 pub enum OrphanCheckErr<'tcx> {
-    NoLocalInputType,
+    NonLocalInputType(Vec<(Ty<'tcx>, bool /* Is this the first input type? */)>),
     UncoveredTy(Ty<'tcx>),
 }
 
@@ -355,7 +355,7 @@ pub fn orphan_check(
 /// Note that this function is never called for types that have both type
 /// parameters and inference variables.
 fn orphan_check_trait_ref<'tcx>(
-    tcx: TyCtxt<'_>,
+    tcx: TyCtxt<'tcx>,
     trait_ref: ty::TraitRef<'tcx>,
     in_crate: InCrate,
 ) -> Result<(), OrphanCheckErr<'tcx>> {
@@ -378,40 +378,51 @@ fn orphan_check_trait_ref<'tcx>(
         //      Let Ti be the first such type.
         //     - No uncovered type parameters P1..=Pn may appear in T0..Ti (excluding Ti)
         //
-        fn uncover_fundamental_ty<'a>(
-            tcx: TyCtxt<'_>,
-            ty: Ty<'a>,
+        fn uncover_fundamental_ty<'tcx>(
+            tcx: TyCtxt<'tcx>,
+            ty: Ty<'tcx>,
             in_crate: InCrate,
-        ) -> Vec<Ty<'a>> {
-            if fundamental_ty(ty) && !ty_is_local(tcx, ty, in_crate) {
+        ) -> Vec<Ty<'tcx>> {
+            if fundamental_ty(ty) && ty_is_non_local(tcx, ty, in_crate).is_some() {
                 ty.walk_shallow().flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)).collect()
             } else {
                 vec![ty]
             }
         }
 
-        for input_ty in
-            trait_ref.input_types().flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
+        let mut non_local_spans = vec![];
+        for (i, input_ty) in trait_ref
+            .input_types()
+            .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
+            .enumerate()
         {
             debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty);
-            if ty_is_local(tcx, input_ty, in_crate) {
+            let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
+            if non_local_tys.is_none() {
                 debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
                 return Ok(());
             } else if let ty::Param(_) = input_ty.kind {
                 debug!("orphan_check_trait_ref: uncovered ty: `{:?}`", input_ty);
                 return Err(OrphanCheckErr::UncoveredTy(input_ty))
             }
+            if let Some(non_local_tys) = non_local_tys {
+                for input_ty in non_local_tys {
+                    non_local_spans.push((input_ty, i == 0));
+                }
+            }
         }
         // If we exit above loop, never found a local type.
         debug!("orphan_check_trait_ref: no local type");
-        Err(OrphanCheckErr::NoLocalInputType)
+        Err(OrphanCheckErr::NonLocalInputType(non_local_spans))
     } else {
+        let mut non_local_spans = vec![];
         // First, create an ordered iterator over all the type
         // parameters to the trait, with the self type appearing
         // first.  Find the first input type that either references a
         // type parameter OR some local type.
-        for input_ty in trait_ref.input_types() {
-            if ty_is_local(tcx, input_ty, in_crate) {
+        for (i, input_ty) in trait_ref.input_types().enumerate() {
+            let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
+            if non_local_tys.is_none() {
                 debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
 
                 // First local input type. Check that there are no
@@ -438,15 +449,21 @@ fn orphan_check_trait_ref<'tcx>(
                 debug!("orphan_check_trait_ref: uncovered type `{:?}`", param);
                 return Err(OrphanCheckErr::UncoveredTy(param));
             }
+
+            if let Some(non_local_tys) = non_local_tys {
+                for input_ty in non_local_tys {
+                    non_local_spans.push((input_ty, i == 0));
+                }
+            }
         }
         // If we exit above loop, never found a local type.
         debug!("orphan_check_trait_ref: no local type");
-        Err(OrphanCheckErr::NoLocalInputType)
+        Err(OrphanCheckErr::NonLocalInputType(non_local_spans))
     }
 }
 
-fn uncovered_tys<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec<Ty<'tcx>> {
-    if ty_is_local_constructor(tcx, ty, in_crate) {
+fn uncovered_tys<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec<Ty<'tcx>> {
+    if ty_is_non_local_constructor(tcx, ty, in_crate).is_none() {
         vec![]
     } else if fundamental_ty(ty) {
         ty.walk_shallow()
@@ -464,9 +481,23 @@ fn is_possibly_remote_type(ty: Ty<'_>, _in_crate: InCrate) -> bool {
     }
 }
 
-fn ty_is_local(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bool {
-    ty_is_local_constructor(tcx, ty, in_crate) ||
-        fundamental_ty(ty) && ty.walk_shallow().any(|t| ty_is_local(tcx, t, in_crate))
+fn ty_is_non_local<'t>(tcx: TyCtxt<'t>, ty: Ty<'t>, in_crate: InCrate) -> Option<Vec<Ty<'t>>> {
+    match ty_is_non_local_constructor(tcx, ty, in_crate) {
+        Some(ty) => if !fundamental_ty(ty) {
+            Some(vec![ty])
+        } else {
+            let tys: Vec<_> = ty.walk_shallow()
+                .filter_map(|t| ty_is_non_local(tcx, t, in_crate))
+                .flat_map(|i| i)
+                .collect();
+            if tys.is_empty() {
+                None
+            } else {
+                Some(tys)
+            }
+        },
+        None => None,
+    }
 }
 
 fn fundamental_ty(ty: Ty<'_>) -> bool {
@@ -486,8 +517,12 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool {
     }
 }
 
-fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bool {
-    debug!("ty_is_local_constructor({:?})", ty);
+fn ty_is_non_local_constructor<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    ty: Ty<'tcx>,
+    in_crate: InCrate,
+) -> Option<Ty<'tcx>> {
+    debug!("ty_is_non_local_constructor({:?})", ty);
 
     match ty.kind {
         ty::Bool |
@@ -506,37 +541,49 @@ fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bo
         ty::Tuple(..) |
         ty::Param(..) |
         ty::Projection(..) => {
-            false
+            Some(ty)
         }
 
         ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match in_crate {
-            InCrate::Local => false,
+            InCrate::Local => Some(ty),
             // The inference variable might be unified with a local
             // type in that remote crate.
-            InCrate::Remote => true,
+            InCrate::Remote => None,
         },
 
-        ty::Adt(def, _) => def_id_is_local(def.did, in_crate),
-        ty::Foreign(did) => def_id_is_local(did, in_crate),
+        ty::Adt(def, _) => if def_id_is_local(def.did, in_crate) {
+            None
+        } else {
+            Some(ty)
+        },
+        ty::Foreign(did) => if def_id_is_local(did, in_crate) {
+            None
+        } else {
+            Some(ty)
+        },
         ty::Opaque(did, _) => {
             // Check the underlying type that this opaque
             // type resolves to.
             // This recursion will eventually terminate,
             // since we've already managed to successfully
             // resolve all opaque types by this point
             let real_ty = tcx.type_of(did);
-            ty_is_local_constructor(tcx, real_ty, in_crate)
+            ty_is_non_local_constructor(tcx, real_ty, in_crate)
         }
 
         ty::Dynamic(ref tt, ..) => {
             if let Some(principal) = tt.principal() {
-                def_id_is_local(principal.def_id(), in_crate)
+                if def_id_is_local(principal.def_id(), in_crate) {
+                    None
+                } else {
+                    Some(ty)
+                }
             } else {
-                false
+                Some(ty)
             }
         }
 
-        ty::Error => true,
+        ty::Error => None,
 
         ty::UnnormalizedProjection(..) |
         ty::Closure(..) |

src/librustc_typeck/coherence/orphan.rs

@@ -24,7 +24,7 @@ impl ItemLikeVisitor<'v> for OrphanChecker<'tcx> {
     fn visit_item(&mut self, item: &hir::Item) {
         let def_id = self.tcx.hir().local_def_id(item.hir_id);
         // "Trait" impl
-        if let hir::ItemKind::Impl(.., Some(_), _, _) = item.kind {
+        if let hir::ItemKind::Impl(.., generics, Some(tr), impl_ty, _) = &item.kind {
             debug!("coherence2::orphan check: trait impl {}",
                    self.tcx.hir().node_to_string(item.hir_id));
             let trait_ref = self.tcx.impl_trait_ref(def_id).unwrap();
@@ -33,32 +33,74 @@ impl ItemLikeVisitor<'v> for OrphanChecker<'tcx> {
             let sp = cm.def_span(item.span);
             match traits::orphan_check(self.tcx, def_id) {
                 Ok(()) => {}
-                Err(traits::OrphanCheckErr::NoLocalInputType) => {
-                    struct_span_err!(self.tcx.sess,
-                                     sp,
-                                     E0117,
-                                     "only traits defined in the current crate can be \
-                                      implemented for arbitrary types")
-                        .span_label(sp, "impl doesn't use types inside crate")
-                        .note("the impl does not reference only types defined in this crate")
-                        .note("define and implement a trait or new type instead")
-                        .emit();
+                Err(traits::OrphanCheckErr::NonLocalInputType(tys)) => {
+                    let mut err = struct_span_err!(
+                        self.tcx.sess,
+                        sp,
+                        E0117,
+                        "only traits defined in the current crate can be implemented for \
+                         arbitrary types"
+                    );
+                    err.span_label(sp, "impl doesn't use only types from inside the current crate");
+                    for (ty, is_target_ty) in &tys {
+                        let mut ty = *ty;
+                        self.tcx.infer_ctxt().enter(|infcx| {
+                            // Remove the lifetimes unnecessary for this error.
+                            ty = infcx.freshen(ty);
+                        });
+                        ty = match ty.kind {
+                            // Remove the type arguments from the output, as they are not relevant.
+                            // You can think of this as the reverse of `resolve_vars_if_possible`.
+                            // That way if we had `Vec<MyType>`, we will properly attribute the
+                            // problem to `Vec<T>` and avoid confusing the user if they were to see
+                            // `MyType` in the error.
+                            ty::Adt(def, _) => self.tcx.mk_adt(def, ty::List::empty()),
+                            _ => ty,
+                        };
+                        let this = "this".to_string();
+                        let (ty, postfix) = match &ty.kind {
+                            ty::Slice(_) => (this, " because slices are always foreign"),
+                            ty::Array(..) => (this, " because arrays are always foreign"),
+                            ty::Tuple(..) => (this, " because tuples are always foreign"),
+                            _ => (format!("`{}`", ty), ""),
+                        };
+                        let msg = format!("{} is not defined in the current crate{}", ty, postfix);
+                        if *is_target_ty {
+                            // Point at `D<A>` in `impl<A, B> for C<B> in D<A>`
+                            err.span_label(impl_ty.span, &msg);
+                        } else {
+                            // Point at `C<B>` in `impl<A, B> for C<B> in D<A>`
+                            err.span_label(tr.path.span, &msg);
+                        }
+                    }
+                    err.note("define and implement a trait or new type instead");
+                    err.emit();
                     return;
                 }
                 Err(traits::OrphanCheckErr::UncoveredTy(param_ty)) => {
-                    struct_span_err!(self.tcx.sess,
-                                     sp,
-                                     E0210,
-                                     "type parameter `{}` must be used as the type parameter \
-                                      for some local type (e.g., `MyStruct<{}>`)",
-                                     param_ty,
-                                     param_ty)
-                        .span_label(sp,
-                                    format!("type parameter `{}` must be used as the type \
-                                             parameter for some local type", param_ty))
-                        .note("only traits defined in the current crate can be implemented \
-                               for a type parameter")
-                        .emit();
+                    let mut sp = sp;
+                    for param in &generics.params {
+                        if param.name.ident().to_string() == param_ty.to_string() {
+                            sp = param.span;
+                        }
+                    }
+                    let mut err = struct_span_err!(
+                        self.tcx.sess,
+                        sp,
+                        E0210,
+                        "type parameter `{}` must be used as the type parameter for some local \
+                         type (e.g., `MyStruct<{}>`)",
+                        param_ty,
+                        param_ty
+                    );
+                    err.span_label(sp, format!(
+                        "type parameter `{}` must be used as the type parameter for some local \
+                         type",
+                        param_ty,
+                    ));
+                    err.note("only traits defined in the current crate can be implemented for a \
+                              type parameter");
+                    err.emit();
                     return;
                 }
             }

src/test/ui/coherence/coherence-all-remote.old.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-all-remote.rs:9:1
+  --> $DIR/coherence-all-remote.rs:9:6
    |
 LL | impl<T> Remote1<T> for isize { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter
 

src/test/ui/coherence/coherence-all-remote.re.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-all-remote.rs:9:1
+  --> $DIR/coherence-all-remote.rs:9:6
    |
 LL | impl<T> Remote1<T> for isize { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter
 

src/test/ui/coherence/coherence-bigint-param.old.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-bigint-param.rs:11:1
+  --> $DIR/coherence-bigint-param.rs:11:6
    |
 LL | impl<T> Remote1<BigInt> for T { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter
 

src/test/ui/coherence/coherence-bigint-param.re.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-bigint-param.rs:11:1
+  --> $DIR/coherence-bigint-param.rs:11:6
    |
 LL | impl<T> Remote1<BigInt> for T { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter
 

src/test/ui/coherence/coherence-cow.a.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-cow.rs:18:1
+  --> $DIR/coherence-cow.rs:18:6
    |
 LL | impl<T> Remote for Pair<T,Cover<T>> { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter
 

src/test/ui/coherence/coherence-cow.b.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-cow.rs:23:1
+  --> $DIR/coherence-cow.rs:23:6
    |
 LL | impl<T> Remote for Pair<Cover<T>,T> { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter
 

src/test/ui/coherence/coherence-cow.c.stderr

@@ -1,8 +1,8 @@
 error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
-  --> $DIR/coherence-cow.rs:28:1
+  --> $DIR/coherence-cow.rs:28:6
    |
 LL | impl<T,U> Remote for Pair<Cover<T>,U> { }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
+   |      ^ type parameter `T` must be used as the type parameter for some local type
    |
    = note: only traits defined in the current crate can be implemented for a type parameter