Rollup merge of rust-lang#90181 - notriddle:notriddle/error-pointer, …

…r=estebank

fix(rustc_typeck): report function argument errors on matching type

Fixes rust-lang#90101

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -1492,6 +1492,9 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
                     }
                 }
             }
+            ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
+                self.get_parent_trait_ref(&parent_code)
+            }
             _ => None,
         }
     }

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -1422,6 +1422,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         while let Some(code) = next_code {
             debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code);
             match code {
+                ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => {
+                    next_code = Some(parent_code.as_ref());
+                }
                 ObligationCauseCode::DerivedObligation(derived_obligation)
                 | ObligationCauseCode::BuiltinDerivedObligation(derived_obligation)
                 | ObligationCauseCode::ImplDerivedObligation(derived_obligation) => {

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

@@ -370,45 +370,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 //    `ExpectHasType(expected_ty)`, or the `formal_ty` otherwise.
                 let coerce_ty = expected.only_has_type(self).unwrap_or(formal_ty);
 
+                final_arg_types.push((i, checked_ty, coerce_ty));
+
                 // Cause selection errors caused by resolving a single argument to point at the
                 // argument and not the call. This is otherwise redundant with the `demand_coerce`
                 // call immediately after, but it lets us customize the span pointed to in the
                 // fulfillment error to be more accurate.
                 let _ = self.resolve_vars_with_obligations_and_mutate_fulfillment(
                     coerce_ty,
                     |errors| {
-                        // This is not coming from a macro or a `derive`.
-                        if sp.desugaring_kind().is_none()
-                        && !arg.span.from_expansion()
-                        // Do not change the spans of `async fn`s.
-                        && !matches!(
-                            expr.kind,
-                            hir::ExprKind::Call(
-                                hir::Expr {
-                                    kind: hir::ExprKind::Path(hir::QPath::LangItem(_, _)),
-                                    ..
-                                },
-                                _
-                            )
-                        ) {
-                            for error in errors {
-                                error.obligation.cause.make_mut().span = arg.span;
-                                let code = error.obligation.cause.code.clone();
-                                error.obligation.cause.make_mut().code =
-                                    ObligationCauseCode::FunctionArgumentObligation {
-                                        arg_hir_id: arg.hir_id,
-                                        call_hir_id: expr.hir_id,
-                                        parent_code: Lrc::new(code),
-                                    };
-                            }
-                        }
+                        self.point_at_type_arg_instead_of_call_if_possible(errors, expr);
+                        self.point_at_arg_instead_of_call_if_possible(
+                            errors,
+                            &final_arg_types,
+                            expr,
+                            sp,
+                            args,
+                        );
                     },
                 );
 
                 // We're processing function arguments so we definitely want to use
                 // two-phase borrows.
                 self.demand_coerce(&arg, checked_ty, coerce_ty, None, AllowTwoPhase::Yes);
-                final_arg_types.push((i, checked_ty, coerce_ty));
 
                 // 3. Relate the expected type and the formal one,
                 //    if the expected type was used for the coercion.
@@ -973,45 +957,79 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 continue;
             }
 
-            if let ty::PredicateKind::Trait(predicate) =
-                error.obligation.predicate.kind().skip_binder()
-            {
-                // Collect the argument position for all arguments that could have caused this
-                // `FulfillmentError`.
-                let mut referenced_in = final_arg_types
-                    .iter()
-                    .map(|&(i, checked_ty, _)| (i, checked_ty))
-                    .chain(final_arg_types.iter().map(|&(i, _, coerced_ty)| (i, coerced_ty)))
-                    .flat_map(|(i, ty)| {
-                        let ty = self.resolve_vars_if_possible(ty);
-                        // We walk the argument type because the argument's type could have
-                        // been `Option<T>`, but the `FulfillmentError` references `T`.
-                        if ty.walk(self.tcx).any(|arg| arg == predicate.self_ty().into()) {
-                            Some(i)
-                        } else {
-                            None
-                        }
-                    })
-                    .collect::<Vec<usize>>();
-
-                // Both checked and coerced types could have matched, thus we need to remove
-                // duplicates.
-
-                // We sort primitive type usize here and can use unstable sort
-                referenced_in.sort_unstable();
-                referenced_in.dedup();
-
-                if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) {
-                    // We make sure that only *one* argument matches the obligation failure
-                    // and we assign the obligation's span to its expression's.
-                    error.obligation.cause.make_mut().span = args[ref_in].span;
-                    let code = error.obligation.cause.code.clone();
-                    error.obligation.cause.make_mut().code =
-                        ObligationCauseCode::FunctionArgumentObligation {
-                            arg_hir_id: args[ref_in].hir_id,
-                            call_hir_id: expr.hir_id,
-                            parent_code: Lrc::new(code),
-                        };
+            // Peel derived obligation, because it's the type that originally
+            // started this inference chain that matters, not the one we wound
+            // up with at the end.
+            fn unpeel_to_top(
+                mut code: Lrc<ObligationCauseCode<'_>>,
+            ) -> Lrc<ObligationCauseCode<'_>> {
+                let mut result_code = code.clone();
+                loop {
+                    let parent = match &*code {
+                        ObligationCauseCode::BuiltinDerivedObligation(c)
+                        | ObligationCauseCode::ImplDerivedObligation(c)
+                        | ObligationCauseCode::DerivedObligation(c) => c.parent_code.clone(),
+                        _ => break,
+                    };
+                    result_code = std::mem::replace(&mut code, parent);
+                }
+                result_code
+            }
+            let self_: ty::subst::GenericArg<'_> = match &*unpeel_to_top(Lrc::new(error.obligation.cause.code.clone())) {
+                ObligationCauseCode::BuiltinDerivedObligation(code) |
+                ObligationCauseCode::ImplDerivedObligation(code) |
+                ObligationCauseCode::DerivedObligation(code) => {
+                    code.parent_trait_ref.self_ty().skip_binder().into()
+                }
+                _ if let ty::PredicateKind::Trait(predicate) =
+                    error.obligation.predicate.kind().skip_binder() => {
+                        predicate.self_ty().into()
+                    }
+                _ =>  continue,
+            };
+            let self_ = self.resolve_vars_if_possible(self_);
+
+            // Collect the argument position for all arguments that could have caused this
+            // `FulfillmentError`.
+            let mut referenced_in = final_arg_types
+                .iter()
+                .map(|&(i, checked_ty, _)| (i, checked_ty))
+                .chain(final_arg_types.iter().map(|&(i, _, coerced_ty)| (i, coerced_ty)))
+                .flat_map(|(i, ty)| {
+                    let ty = self.resolve_vars_if_possible(ty);
+                    // We walk the argument type because the argument's type could have
+                    // been `Option<T>`, but the `FulfillmentError` references `T`.
+                    if ty.walk(self.tcx).any(|arg| arg == self_) { Some(i) } else { None }
+                })
+                .collect::<Vec<usize>>();
+
+            // Both checked and coerced types could have matched, thus we need to remove
+            // duplicates.
+
+            // We sort primitive type usize here and can use unstable sort
+            referenced_in.sort_unstable();
+            referenced_in.dedup();
+
+            if let (Some(ref_in), None) = (referenced_in.pop(), referenced_in.pop()) {
+                // Do not point at the inside of a macro.
+                // That would often result in poor error messages.
+                if args[ref_in].span.from_expansion() {
+                    return;
+                }
+                // We make sure that only *one* argument matches the obligation failure
+                // and we assign the obligation's span to its expression's.
+                error.obligation.cause.make_mut().span = args[ref_in].span;
+                let code = error.obligation.cause.code.clone();
+                error.obligation.cause.make_mut().code =
+                    ObligationCauseCode::FunctionArgumentObligation {
+                        arg_hir_id: args[ref_in].hir_id,
+                        call_hir_id: expr.hir_id,
+                        parent_code: Lrc::new(code),
+                    };
+            } else if error.obligation.cause.make_mut().span == call_sp {
+                // Make function calls point at the callee, not the whole thing.
+                if let hir::ExprKind::Call(callee, _) = expr.kind {
+                    error.obligation.cause.make_mut().span = callee.span;
                 }
             }
         }

src/test/ui/associated-types/associated-types-path-2.rs

@@ -28,11 +28,13 @@ pub fn f1_int_uint() {
 pub fn f1_uint_uint() {
     f1(2u32, 4u32);
     //~^ ERROR `u32: Foo` is not satisfied
+    //~| ERROR `u32: Foo` is not satisfied
 }
 
 pub fn f1_uint_int() {
     f1(2u32, 4i32);
     //~^ ERROR `u32: Foo` is not satisfied
+    //~| ERROR `u32: Foo` is not satisfied
 }
 
 pub fn f2_int() {

src/test/ui/associated-types/associated-types-path-2.stderr

@@ -10,12 +10,10 @@ LL |     f1(2i32, 4u32);
    |               ~~~
 
 error[E0277]: the trait bound `u32: Foo` is not satisfied
-  --> $DIR/associated-types-path-2.rs:29:14
+  --> $DIR/associated-types-path-2.rs:29:5
    |
 LL |     f1(2u32, 4u32);
-   |     --       ^^^^ the trait `Foo` is not implemented for `u32`
-   |     |
-   |     required by a bound introduced by this call
+   |     ^^ the trait `Foo` is not implemented for `u32`
    |
 note: required by a bound in `f1`
   --> $DIR/associated-types-path-2.rs:13:14
@@ -24,10 +22,16 @@ LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
    |              ^^^ required by this bound in `f1`
 
 error[E0277]: the trait bound `u32: Foo` is not satisfied
-  --> $DIR/associated-types-path-2.rs:34:14
+  --> $DIR/associated-types-path-2.rs:29:14
+   |
+LL |     f1(2u32, 4u32);
+   |              ^^^^ the trait `Foo` is not implemented for `u32`
+
+error[E0277]: the trait bound `u32: Foo` is not satisfied
+  --> $DIR/associated-types-path-2.rs:35:8
    |
 LL |     f1(2u32, 4i32);
-   |     --       ^^^^ the trait `Foo` is not implemented for `u32`
+   |     -- ^^^^ the trait `Foo` is not implemented for `u32`
    |     |
    |     required by a bound introduced by this call
    |
@@ -37,8 +41,14 @@ note: required by a bound in `f1`
 LL | pub fn f1<T: Foo>(a: T, x: T::A) {}
    |              ^^^ required by this bound in `f1`
 
+error[E0277]: the trait bound `u32: Foo` is not satisfied
+  --> $DIR/associated-types-path-2.rs:35:14
+   |
+LL |     f1(2u32, 4i32);
+   |              ^^^^ the trait `Foo` is not implemented for `u32`
+
 error[E0308]: mismatched types
-  --> $DIR/associated-types-path-2.rs:39:18
+  --> $DIR/associated-types-path-2.rs:41:18
    |
 LL |     let _: i32 = f2(2i32);
    |            ---   ^^^^^^^^ expected `i32`, found `u32`
@@ -50,7 +60,7 @@ help: you can convert a `u32` to an `i32` and panic if the converted value doesn
 LL |     let _: i32 = f2(2i32).try_into().unwrap();
    |                          ++++++++++++++++++++
 
-error: aborting due to 4 previous errors
+error: aborting due to 6 previous errors
 
 Some errors have detailed explanations: E0277, E0308.
 For more information about an error, try `rustc --explain E0277`.

src/test/ui/async-await/async-fn-nonsend.stderr

@@ -1,8 +1,8 @@
 error: future cannot be sent between threads safely
-  --> $DIR/async-fn-nonsend.rs:49:5
+  --> $DIR/async-fn-nonsend.rs:49:17
    |
 LL |     assert_send(local_dropped_before_await());
-   |     ^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `local_dropped_before_await` is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>`
 note: future is not `Send` as this value is used across an await
@@ -22,10 +22,10 @@ LL | fn assert_send(_: impl Send) {}
    |                        ^^^^ required by this bound in `assert_send`
 
 error: future cannot be sent between threads safely
-  --> $DIR/async-fn-nonsend.rs:51:5
+  --> $DIR/async-fn-nonsend.rs:51:17
    |
 LL |     assert_send(non_send_temporary_in_match());
-   |     ^^^^^^^^^^^ future returned by `non_send_temporary_in_match` is not `Send`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_send_temporary_in_match` is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `Rc<()>`
 note: future is not `Send` as this value is used across an await
@@ -45,10 +45,10 @@ LL | fn assert_send(_: impl Send) {}
    |                        ^^^^ required by this bound in `assert_send`
 
 error: future cannot be sent between threads safely
-  --> $DIR/async-fn-nonsend.rs:53:5
+  --> $DIR/async-fn-nonsend.rs:53:17
    |
 LL |     assert_send(non_sync_with_method_call());
-   |     ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send`
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send`
    |
    = help: the trait `Send` is not implemented for `dyn std::fmt::Write`
 note: future is not `Send` as this value is used across an await

src/test/ui/async-await/issue-64130-1-sync.stderr

@@ -1,8 +1,8 @@
 error: future cannot be shared between threads safely
-  --> $DIR/issue-64130-1-sync.rs:21:5
+  --> $DIR/issue-64130-1-sync.rs:21:13
    |
 LL |     is_sync(bar());
-   |     ^^^^^^^ future returned by `bar` is not `Sync`
+   |             ^^^^^ future returned by `bar` is not `Sync`
    |
    = help: within `impl Future`, the trait `Sync` is not implemented for `Foo`
 note: future is not `Sync` as this value is used across an await

src/test/ui/async-await/issue-64130-2-send.stderr

@@ -1,8 +1,8 @@
 error: future cannot be sent between threads safely
-  --> $DIR/issue-64130-2-send.rs:21:5
+  --> $DIR/issue-64130-2-send.rs:21:13
    |
 LL |     is_send(bar());
-   |     ^^^^^^^ future returned by `bar` is not `Send`
+   |             ^^^^^ future returned by `bar` is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `Foo`
 note: future is not `Send` as this value is used across an await

src/test/ui/async-await/issue-64130-3-other.stderr

@@ -1,11 +1,11 @@
 error[E0277]: the trait bound `Foo: Qux` is not satisfied in `impl Future`
-  --> $DIR/issue-64130-3-other.rs:24:5
+  --> $DIR/issue-64130-3-other.rs:24:12
    |
 LL | async fn bar() {
    |                - within this `impl Future`
 ...
 LL |     is_qux(bar());
-   |     ^^^^^^ within `impl Future`, the trait `Qux` is not implemented for `Foo`
+   |            ^^^^^ within `impl Future`, the trait `Qux` is not implemented for `Foo`
    |
 note: future does not implement `Qux` as this value is used across an await
   --> $DIR/issue-64130-3-other.rs:18:5

src/test/ui/async-await/issue-64130-non-send-future-diags.stderr

@@ -1,8 +1,8 @@
 error: future cannot be sent between threads safely
-  --> $DIR/issue-64130-non-send-future-diags.rs:21:5
+  --> $DIR/issue-64130-non-send-future-diags.rs:21:13
    |
 LL |     is_send(foo());
-   |     ^^^^^^^ future returned by `foo` is not `Send`
+   |             ^^^^^ future returned by `foo` is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, u32>`
 note: future is not `Send` as this value is used across an await

src/test/ui/async-await/issue-71137.stderr

@@ -1,8 +1,8 @@
 error: future cannot be sent between threads safely
-  --> $DIR/issue-71137.rs:20:3
+  --> $DIR/issue-71137.rs:20:14
    |
 LL |   fake_spawn(wrong_mutex());
-   |   ^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
+   |              ^^^^^^^^^^^^^ future returned by `wrong_mutex` is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, i32>`
 note: future is not `Send` as this value is used across an await

src/test/ui/async-await/issues/issue-67893.stderr

@@ -1,8 +1,8 @@
 error: generator cannot be sent between threads safely
-  --> $DIR/issue-67893.rs:9:5
+  --> $DIR/issue-67893.rs:9:7
    |
 LL |     g(issue_67893::run())
-   |     ^ generator is not `Send`
+   |       ^^^^^^^^^^^^^^^^^^ generator is not `Send`
    |
    = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, ()>`
 note: required by a bound in `g`

src/test/ui/async-await/pin-needed-to-poll-2.stderr

@@ -1,8 +1,10 @@
 error[E0277]: `PhantomPinned` cannot be unpinned
-  --> $DIR/pin-needed-to-poll-2.rs:43:9
+  --> $DIR/pin-needed-to-poll-2.rs:43:18
    |
 LL |         Pin::new(&mut self.sleep).poll(cx)
-   |         ^^^^^^^^ within `Sleep`, the trait `Unpin` is not implemented for `PhantomPinned`
+   |         -------- ^^^^^^^^^^^^^^^ within `Sleep`, the trait `Unpin` is not implemented for `PhantomPinned`
+   |         |
+   |         required by a bound introduced by this call
    |
    = note: consider using `Box::pin`
 note: required because it appears within the type `Sleep`