Point at struct field if possible

10 files changed, 194 insertions, 136 deletions

diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
index 7c4d812a1b6..15938b3c9b9 100644
--- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
+++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
@@ -1680,48 +1680,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             ty::PredicateKind::Projection(pred) => pred.projection_ty.substs,
             _ => ty::List::empty(),
         };
-        let mut param_to_point_at = predicate_substs.types().find_map(|ty| {
-            ty.walk().find_map(|arg| {
-                if let ty::GenericArgKind::Type(ty) = arg.unpack()
-                    && let ty::Param(param_ty) = ty.kind()
-                    // Look for a param ty that is local to this method/fn
-                    // and not inherited from an impl, for example.
-                    && self.tcx.parent(generics.type_param(param_ty, self.tcx).def_id) == def_id
-                {
-                    Some(arg)
-                } else {
-                    None
-                }
-            })
-        });
-        let mut fallback_param_to_point_at = predicate_substs.types().find_map(|ty| {
-            ty.walk().find_map(|arg| {
-                if let ty::GenericArgKind::Type(ty) = arg.unpack()
-                    && let ty::Param(param_ty) = ty.kind()
-                    && self.tcx.parent(generics.type_param(param_ty, self.tcx).def_id) != def_id
-                    && param_ty.name != rustc_span::symbol::kw::SelfUpper
-                {
-                    Some(arg)
-                } else {
-                    None
-                }
+
+        let find_param_matching = |matches: &dyn Fn(&ty::ParamTy) -> bool| {
+            predicate_substs.types().find_map(|ty| {
+                ty.walk().find_map(|arg| {
+                    if let ty::GenericArgKind::Type(ty) = arg.unpack()
+                        && let ty::Param(param_ty) = ty.kind()
+                        && matches(param_ty)
+                    {
+                        Some(arg)
+                    } else {
+                        None
+                    }
+                })
             })
+        };
+
+        // Prefer generics that are local to the fn item, since these are likely
+        // to be the cause of the unsatisfied predicaete.
+        let mut param_to_point_at = find_param_matching(&|param_ty| {
+            self.tcx.parent(generics.type_param(param_ty, self.tcx).def_id) == def_id
         });
-        let mut self_param_to_point_at = predicate_substs.types().find_map(|ty| {
-            ty.walk().find_map(|arg| {
-                if let ty::GenericArgKind::Type(ty) = arg.unpack()
-                    && let ty::Param(param_ty) = ty.kind()
-                    && param_ty.name == rustc_span::symbol::kw::SelfUpper
-                {
-                    Some(arg)
-                } else {
-                    None
-                }
-            })
+        // Fall back to generic that isn't local to the fn item. This will come
+        // from a trait or impl, for example.
+        let mut fallback_param_to_point_at = find_param_matching(&|param_ty| {
+            self.tcx.parent(generics.type_param(param_ty, self.tcx).def_id) != def_id
+                && param_ty.name != rustc_span::symbol::kw::SelfUpper
         });
-
-        // Also skip over ambiguity errors, which have their own machinery
-        // to print a relevant error.
+        // Finally, the `Self` parameter is possibly the reason that the predicate
+        // is unsatisfied. This is less likely to be true for methods, because the
+        // method probe means that we already kinda check that the predicates due
+        // to the `Self` type are true.
+        let mut self_param_to_point_at =
+            find_param_matching(&|param_ty| param_ty.name == rustc_span::symbol::kw::SelfUpper);
+
+        // For ambiguity errors, we actually want to look for a parameter that is
+        // the source of the inference type left over in this predicate.
         if let traits::FulfillmentErrorCode::CodeAmbiguity = error.code {
             fallback_param_to_point_at = None;
             self_param_to_point_at = None;
@@ -1737,99 +1731,101 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     = hir.get(hir.get_parent_node(*hir_id))
                     && callee.hir_id == *hir_id
                 {
-                    if let Some(param_to_point_at) = param_to_point_at
-                        && self.point_at_args_if_possible(error, def_id, param_to_point_at, *call_hir_id, callee.span, args) {
-                        return true;
-                    }
-
-                    if let Some(fallback_param_to_point_at) = fallback_param_to_point_at
-                        && self.point_at_args_if_possible(error, def_id, fallback_param_to_point_at, *call_hir_id, callee.span, args)
-                    {
-                        return true;
-                    }
-
-                    if let Some(self_param_to_point_at) = self_param_to_point_at
-                        && self.point_at_args_if_possible(error, def_id, self_param_to_point_at, *call_hir_id, callee.span, args)
-                    {
-                        return true;
-                    }
-
-                    if let hir::QPath::Resolved(_, path) = qpath
-                        && let Some(param_to_point_at) = param_to_point_at
-                        && let Some(segment) = path.segments.last()
-                        && self.point_at_generics_if_possible(error, def_id, param_to_point_at, segment)
-                    {
-                        return true;
+                    for param in [param_to_point_at, fallback_param_to_point_at, self_param_to_point_at] {
+                        if let Some(param) = param
+                            && self.point_at_arg_if_possible(error, def_id, param, *call_hir_id, callee.span, args)
+                        {
+                            return true;
+                        }
                     }
 
-                    if let hir::QPath::TypeRelative(_, segment) = qpath
-                        && let Some(param_to_point_at) = param_to_point_at
-                        && self.point_at_generics_if_possible(error, def_id, param_to_point_at, segment)
+                    // Notably, we only point to params that are local to the
+                    // item we're checking, since those are the ones we are able
+                    // to look in the hir::PathSegment for. Everything else
+                    // would require a deeper search into the qpath than I think
+                    // is worthwhile.
+                    if let Some(param_to_point_at) = param_to_point_at
+                        && self.point_at_path_if_possible(error, def_id, param_to_point_at, qpath)
                     {
                         return true;
                     }
                 }
             }
-            hir::Node::Expr(hir::Expr { kind: hir::ExprKind::MethodCall(segment, args, ..), .. }) => {
-                if let Some(param_to_point_at) = param_to_point_at
-                    && self.point_at_args_if_possible(error, def_id, param_to_point_at, hir_id, segment.ident.span, args)
+            hir::Node::Expr(hir::Expr {
+                kind: hir::ExprKind::MethodCall(segment, args, ..),
+                ..
+            }) => {
+                for param in [param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
                 {
-                    return true;
+                    if let Some(param) = param
+                        && self.point_at_arg_if_possible(error, def_id, param, hir_id, segment.ident.span, args)
+                    {
+                        return true;
+                    }
                 }
 
-                if let Some(fallback_param_to_point_at) = fallback_param_to_point_at
-                    && self.point_at_args_if_possible(error, def_id, fallback_param_to_point_at, hir_id, segment.ident.span, args)
+                if let Some(param_to_point_at) = param_to_point_at
+                    && self.point_at_generic_if_possible(error, def_id, param_to_point_at, segment)
                 {
                     return true;
                 }
-
-                if let Some(self_param_to_point_at) = self_param_to_point_at
-                    && self.point_at_args_if_possible(error, def_id, self_param_to_point_at, hir_id, segment.ident.span, args)
+            }
+            hir::Node::Expr(hir::Expr {
+                kind: hir::ExprKind::Struct(qpath, fields, ..), ..
+            }) => {
+                if let Res::Def(DefKind::Struct | DefKind::Variant, variant_def_id) =
+                    self.typeck_results.borrow().qpath_res(qpath, hir_id)
                 {
-                    return true;
+                    for param in
+                        [param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
+                    {
+                        if let Some(param) = param
+                            && self.point_at_field_if_possible(error, def_id, param, variant_def_id, fields)
+                        {
+                            return true;
+                        }
+                    }
                 }
 
                 if let Some(param_to_point_at) = param_to_point_at
-                    && self.point_at_generics_if_possible(error, def_id, param_to_point_at, segment)
+                    && self.point_at_path_if_possible(error, def_id, param_to_point_at, qpath)
                 {
                     return true;
                 }
             }
-            hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(..), .. }) => {
-                // fixme
-            }
             _ => {}
         }
 
         false
     }
 
-    fn find_ambiguous_parameter_in<T: TypeVisitable<'tcx>>(
+    fn point_at_path_if_possible(
         &self,
-        item_def_id: DefId,
-        t: T,
-    ) -> Option<ty::GenericArg<'tcx>> {
-        struct FindAmbiguousParameter<'a, 'tcx>(&'a FnCtxt<'a, 'tcx>, DefId);
-        impl<'tcx> TypeVisitor<'tcx> for FindAmbiguousParameter<'_, 'tcx> {
-            type BreakTy = ty::GenericArg<'tcx>;
-            fn visit_ty(&mut self, ty: Ty<'tcx>) -> std::ops::ControlFlow<Self::BreakTy> {
-                if let Some(origin) = self.0.type_var_origin(ty)
-                    && let TypeVariableOriginKind::TypeParameterDefinition(_, Some(def_id))
-                        = origin.kind
-                    && let generics = self.0.tcx.generics_of(self.1)
-                    && let Some(index) = generics.param_def_id_to_index(self.0.tcx, def_id)
-                    && let Some(subst) = ty::InternalSubsts::identity_for_item(self.0.tcx, self.1).get(index as usize)
+        error: &mut traits::FulfillmentError<'tcx>,
+        def_id: DefId,
+        param: ty::GenericArg<'tcx>,
+        qpath: &QPath<'tcx>,
+    ) -> bool {
+        match qpath {
+            hir::QPath::Resolved(_, path) => {
+                if let Some(segment) = path.segments.last()
+                    && self.point_at_generic_if_possible(error, def_id, param, segment)
                 {
-                    ControlFlow::Break(*subst)
-                } else {
-                    ty.super_visit_with(self)
+                    return true;
                 }
             }
+            hir::QPath::TypeRelative(_, segment) => {
+                if self.point_at_generic_if_possible(error, def_id, param, segment) {
+                    return true;
+                }
+            }
+            _ => {}
         }
-        t.visit_with(&mut FindAmbiguousParameter(self, item_def_id)).break_value()
+
+        false
     }
 
-    fn point_at_args_if_possible(
+    fn point_at_arg_if_possible(
         &self,
         error: &mut traits::FulfillmentError<'tcx>,
         def_id: DefId,
@@ -1843,25 +1839,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             .inputs()
             .iter()
             .enumerate()
-            .filter(|(_, ty)| {
-                let mut walk = ty.walk();
-                while let Some(arg) = walk.next() {
-                    if arg == param_to_point_at {
-                        return true;
-                    } else if let ty::GenericArgKind::Type(ty) = arg.unpack()
-                        && let ty::Projection(..) = ty.kind()
-                    {
-                        // This logic may seem a bit strange, but typically when
-                        // we have a projection type in a function signature, the
-                        // argument that's being passed into that signature is
-                        // not actually constraining that projection in a meaningful
-                        // way. So we skip it, and see improvements in some UI tests
-                        // due to it.
-                        walk.skip_current_subtree();
-                    }
-                }
-                false
-            })
+            .filter(|(_, ty)| find_param_in_ty(ty, param_to_point_at))
             .collect();
 
         if let [(idx, _)] = args_referencing_param.as_slice()
@@ -1886,7 +1864,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
-    fn point_at_generics_if_possible(
+    fn point_at_field_if_possible(
+        &self,
+        error: &mut traits::FulfillmentError<'tcx>,
+        def_id: DefId,
+        param_to_point_at: ty::GenericArg<'tcx>,
+        variant_def_id: DefId,
+        expr_fields: &[hir::ExprField<'tcx>],
+    ) -> bool {
+        let def = self.tcx.adt_def(def_id);
+        let identity_substs = ty::InternalSubsts::identity_for_item(self.tcx, def_id);
+        let fields_referencing_param: Vec<_> = def
+            .variant_with_id(variant_def_id)
+            .fields
+            .iter()
+            .filter(|field| {
+                let field_ty = field.ty(self.tcx, identity_substs);
+                match find_param_in_ty(field_ty, param_to_point_at) {
+                    Ok(value) => value,
+                    Err(value) => return value,
+                }
+            })
+            .collect();
+        if let [field] = fields_referencing_param.as_slice() {
+            for expr_field in expr_fields {
+                if self.tcx.adjust_ident(expr_field.ident, variant_def_id) == field.ident(self.tcx)
+                {
+                    error.obligation.cause.span = expr_field.span;
+                }
+            }
+            true
+        } else {
+            false
+        }
+    }
+
+    fn point_at_generic_if_possible(
         &self,
         error: &mut traits::FulfillmentError<'tcx>,
         def_id: DefId,
@@ -1912,6 +1925,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         true
     }
 
+    fn find_ambiguous_parameter_in<T: TypeVisitable<'tcx>>(
+        &self,
+        item_def_id: DefId,
+        t: T,
+    ) -> Option<ty::GenericArg<'tcx>> {
+        struct FindAmbiguousParameter<'a, 'tcx>(&'a FnCtxt<'a, 'tcx>, DefId);
+        impl<'tcx> TypeVisitor<'tcx> for FindAmbiguousParameter<'_, 'tcx> {
+            type BreakTy = ty::GenericArg<'tcx>;
+            fn visit_ty(&mut self, ty: Ty<'tcx>) -> std::ops::ControlFlow<Self::BreakTy> {
+                if let Some(origin) = self.0.type_var_origin(ty)
+                    && let TypeVariableOriginKind::TypeParameterDefinition(_, Some(def_id))
+                        = origin.kind
+                    && let generics = self.0.tcx.generics_of(self.1)
+                    && let Some(index) = generics.param_def_id_to_index(self.0.tcx, def_id)
+                    && let Some(subst) = ty::InternalSubsts::identity_for_item(self.0.tcx, self.1).get(index as usize)
+                {
+                    ControlFlow::Break(*subst)
+                } else {
+                    ty.super_visit_with(self)
+                }
+            }
+        }
+        t.visit_with(&mut FindAmbiguousParameter(self, item_def_id)).break_value()
+    }
+
     fn label_fn_like(
         &self,
         err: &mut Diagnostic,
@@ -2053,3 +2091,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 }
+
+fn find_param_in_ty(ty: Ty, param_to_point_at: ty::GenericArg) -> bool {
+    let mut walk = ty.walk();
+    while let Some(arg) = walk.next() {
+        if arg == param_to_point_at {
+        return true;
+    } else if let ty::GenericArgKind::Type(ty) = arg.unpack()
+        && let ty::Projection(..) = ty.kind()
+    {
+        // This logic may seem a bit strange, but typically when
+        // we have a projection type in a function signature, the
+        // argument that's being passed into that signature is
+        // not actually constraining that projection's substs in
+        // a meaningful way. So we skip it, and see improvements
+        // in some UI tests.
+        walk.skip_current_subtree();
+    }
+    }
+    false
+}
diff --git a/src/test/ui/chalkify/type_wf.rs b/src/test/ui/chalkify/type_wf.rs
index dd83a03fdf6..eeeefcfb7dd 100644
--- a/src/test/ui/chalkify/type_wf.rs
+++ b/src/test/ui/chalkify/type_wf.rs
@@ -15,8 +15,8 @@ fn main() {
        x: 5,
     };
 
-    let s = S { //~ ERROR the trait bound `{float}: Foo` is not satisfied
-        x: 5.0,
+    let s = S {
+        x: 5.0, //~ ERROR the trait bound `{float}: Foo` is not satisfied
     };
 
     let s = S {
diff --git a/src/test/ui/chalkify/type_wf.stderr b/src/test/ui/chalkify/type_wf.stderr
index 7f8566082cd..28314928a3c 100644
--- a/src/test/ui/chalkify/type_wf.stderr
+++ b/src/test/ui/chalkify/type_wf.stderr
@@ -1,8 +1,8 @@
 error[E0277]: the trait bound `{float}: Foo` is not satisfied
-  --> $DIR/type_wf.rs:18:13
+  --> $DIR/type_wf.rs:19:9
    |
-LL |     let s = S {
-   |             ^ the trait `Foo` is not implemented for `{float}`
+LL |         x: 5.0,
+   |         ^^^^^^ the trait `Foo` is not implemented for `{float}`
    |
    = help: the trait `Foo` is implemented for `i32`
 note: required by a bound in `S`
diff --git a/src/test/ui/range/range-1.stderr b/src/test/ui/range/range-1.stderr
index 0e3bb66ab61..aaea91ce0cb 100644
--- a/src/test/ui/range/range-1.stderr
+++ b/src/test/ui/range/range-1.stderr
@@ -27,7 +27,7 @@ error[E0277]: the size for values of type `[{integer}]` cannot be known at compi
   --> $DIR/range-1.rs:14:17
    |
 LL |     let range = *arr..;
-   |                 ^^^^^^ doesn't have a size known at compile-time
+   |                 ^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[{integer}]`
 note: required by a bound in `RangeFrom`
diff --git a/src/test/ui/traits/bound/on-structs-and-enums-locals.rs b/src/test/ui/traits/bound/on-structs-and-enums-locals.rs
index 21c0ce80f8a..60ba343bb0a 100644
--- a/src/test/ui/traits/bound/on-structs-and-enums-locals.rs
+++ b/src/test/ui/traits/bound/on-structs-and-enums-locals.rs
@@ -8,8 +8,8 @@ struct Foo<T:Trait> {
 
 fn main() {
     let foo = Foo {
-    //~^ ERROR E0277
         x: 3
+    //~^ ERROR E0277
     };
 
     let baz: Foo<usize> = loop { };
diff --git a/src/test/ui/traits/bound/on-structs-and-enums-locals.stderr b/src/test/ui/traits/bound/on-structs-and-enums-locals.stderr
index c9068a27002..4abff9fb049 100644
--- a/src/test/ui/traits/bound/on-structs-and-enums-locals.stderr
+++ b/src/test/ui/traits/bound/on-structs-and-enums-locals.stderr
@@ -11,10 +11,10 @@ LL | struct Foo<T:Trait> {
    |              ^^^^^ required by this bound in `Foo`
 
 error[E0277]: the trait bound `{integer}: Trait` is not satisfied
-  --> $DIR/on-structs-and-enums-locals.rs:10:15
+  --> $DIR/on-structs-and-enums-locals.rs:11:9
    |
-LL |     let foo = Foo {
-   |               ^^^ the trait `Trait` is not implemented for `{integer}`
+LL |         x: 3
+   |         ^^^^ the trait `Trait` is not implemented for `{integer}`
    |
 note: required by a bound in `Foo`
   --> $DIR/on-structs-and-enums-locals.rs:5:14
diff --git a/src/test/ui/traits/bound/on-structs-and-enums-xc1.rs b/src/test/ui/traits/bound/on-structs-and-enums-xc1.rs
index 8156868e048..5ef35b513e0 100644
--- a/src/test/ui/traits/bound/on-structs-and-enums-xc1.rs
+++ b/src/test/ui/traits/bound/on-structs-and-enums-xc1.rs
@@ -6,8 +6,8 @@ use on_structs_and_enums_xc::{Bar, Foo, Trait};
 
 fn main() {
     let foo = Foo {
-    //~^ ERROR E0277
         x: 3
+    //~^ ERROR E0277
     };
     let bar: Bar<f64> = return;
     //~^ ERROR E0277
diff --git a/src/test/ui/traits/bound/on-structs-and-enums-xc1.stderr b/src/test/ui/traits/bound/on-structs-and-enums-xc1.stderr
index f4cc64af94f..dd0de44f3b1 100644
--- a/src/test/ui/traits/bound/on-structs-and-enums-xc1.stderr
+++ b/src/test/ui/traits/bound/on-structs-and-enums-xc1.stderr
@@ -11,10 +11,10 @@ LL | pub enum Bar<T:Trait> {
    |                ^^^^^ required by this bound in `Bar`
 
 error[E0277]: the trait bound `{integer}: Trait` is not satisfied
-  --> $DIR/on-structs-and-enums-xc1.rs:8:15
+  --> $DIR/on-structs-and-enums-xc1.rs:9:9
    |
-LL |     let foo = Foo {
-   |               ^^^ the trait `Trait` is not implemented for `{integer}`
+LL |         x: 3
+   |         ^^^^ the trait `Trait` is not implemented for `{integer}`
    |
 note: required by a bound in `Foo`
   --> $DIR/auxiliary/on_structs_and_enums_xc.rs:5:18
diff --git a/src/test/ui/union/union-generic.mirunsafeck.stderr b/src/test/ui/union/union-generic.mirunsafeck.stderr
index a4f0c400d73..037022a91fc 100644
--- a/src/test/ui/union/union-generic.mirunsafeck.stderr
+++ b/src/test/ui/union/union-generic.mirunsafeck.stderr
@@ -11,10 +11,10 @@ LL | union U<T: Copy> {
    |            ^^^^ required by this bound in `U`
 
 error[E0277]: the trait bound `Rc<u32>: Copy` is not satisfied
-  --> $DIR/union-generic.rs:13:13
+  --> $DIR/union-generic.rs:13:17
    |
 LL |     let u = U::<Rc<u32>> { a: Default::default() };
-   |             ^^^^^^^^^^^^ the trait `Copy` is not implemented for `Rc<u32>`
+   |                 ^^^^^^^ the trait `Copy` is not implemented for `Rc<u32>`
    |
 note: required by a bound in `U`
   --> $DIR/union-generic.rs:6:12
diff --git a/src/test/ui/union/union-generic.thirunsafeck.stderr b/src/test/ui/union/union-generic.thirunsafeck.stderr
index a4f0c400d73..037022a91fc 100644
--- a/src/test/ui/union/union-generic.thirunsafeck.stderr
+++ b/src/test/ui/union/union-generic.thirunsafeck.stderr
@@ -11,10 +11,10 @@ LL | union U<T: Copy> {
    |            ^^^^ required by this bound in `U`
 
 error[E0277]: the trait bound `Rc<u32>: Copy` is not satisfied
-  --> $DIR/union-generic.rs:13:13
+  --> $DIR/union-generic.rs:13:17
    |
 LL |     let u = U::<Rc<u32>> { a: Default::default() };
-   |             ^^^^^^^^^^^^ the trait `Copy` is not implemented for `Rc<u32>`
+   |                 ^^^^^^^ the trait `Copy` is not implemented for `Rc<u32>`
    |
 note: required by a bound in `U`
   --> $DIR/union-generic.rs:6:12