Auto merge of #88719 - estebank:point-at-arg-for-obligation, r=nagisa

Point at argument instead of call for their obligations

When an obligation is introduced by a specific `fn` argument, point at
the argument instead of the `fn` call if the obligation fails to be
fulfilled.

Move the information about pointing at the call argument expression in
an unmet obligation span from the `FulfillmentError` to a new
`ObligationCauseCode`.

When giving an error about an obligation introduced by a function call
that an argument doesn't fulfill, and that argument is a block, add a
span_label pointing at the innermost tail expression.

Current output:

```
error[E0425]: cannot find value `x` in this scope
 --> f10.rs:4:14
  |
4 |         Some(x * 2)
  |              ^ not found in this scope

error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
 --> f10.rs:2:31
  |
2 |       let p = Some(45).and_then({
  |  ______________________--------_^
  | |                      |
  | |                      required by a bound introduced by this call
3 | |         |x| println!("doubling {}", x);
4 | |         Some(x * 2)
  | |         -----------
5 | |     });
  | |_____^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
  |
  = help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`
```

Previous output:

```
error[E0425]: cannot find value `x` in this scope
 --> f10.rs:4:14
  |
4 |         Some(x * 2)
  |              ^ not found in this scope

error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
 --> f10.rs:2:22
  |
2 |     let p = Some(45).and_then({
  |                      ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
  |
  = help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`
```

Partially address #27300. Will require rebasing on top of #88546.

Author: bors

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -1998,7 +1998,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                                     &obligation,
                                     &traits::SelectionError::Unimplemented,
                                     false,
-                                    false,
                                 );
                             }
                         }

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mismatched_static_lifetime.rs

@@ -29,7 +29,13 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             SubregionOrigin::Subtype(box TypeTrace { ref cause, .. }) => cause,
             _ => return None,
         };
-        let (parent, impl_def_id) = match &cause.code {
+        // If we added a "points at argument expression" obligation, we remove it here, we care
+        // about the original obligation only.
+        let code = match &cause.code {
+            ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => &*parent_code,
+            _ => &cause.code,
+        };
+        let (parent, impl_def_id) = match code {
             ObligationCauseCode::MatchImpl(parent, impl_def_id) => (parent, impl_def_id),
             _ => return None,
         };

compiler/rustc_infer/src/traits/mod.rs

@@ -66,10 +66,6 @@ pub type Selection<'tcx> = ImplSource<'tcx, PredicateObligation<'tcx>>;
 pub struct FulfillmentError<'tcx> {
     pub obligation: PredicateObligation<'tcx>,
     pub code: FulfillmentErrorCode<'tcx>,
-    /// Diagnostics only: we opportunistically change the `code.span` when we encounter an
-    /// obligation error caused by a call argument. When this is the case, we also signal that in
-    /// this field to ensure accuracy of suggestions.
-    pub points_at_arg_span: bool,
     /// Diagnostics only: the 'root' obligation which resulted in
     /// the failure to process `obligation`. This is the obligation
     /// that was initially passed to `register_predicate_obligation`
@@ -128,7 +124,7 @@ impl<'tcx> FulfillmentError<'tcx> {
         code: FulfillmentErrorCode<'tcx>,
         root_obligation: PredicateObligation<'tcx>,
     ) -> FulfillmentError<'tcx> {
-        FulfillmentError { obligation, code, points_at_arg_span: false, root_obligation }
+        FulfillmentError { obligation, code, root_obligation }
     }
 }
 

compiler/rustc_middle/src/traits/mod.rs

@@ -253,6 +253,15 @@ pub enum ObligationCauseCode<'tcx> {
 
     DerivedObligation(DerivedObligationCause<'tcx>),
 
+    FunctionArgumentObligation {
+        /// The node of the relevant argument in the function call.
+        arg_hir_id: hir::HirId,
+        /// The node of the function call.
+        call_hir_id: hir::HirId,
+        /// The obligation introduced by this argument.
+        parent_code: Lrc<ObligationCauseCode<'tcx>>,
+    },
+
     /// Error derived when matching traits/impls; see ObligationCause for more details
     CompareImplConstObligation,
 
@@ -368,11 +377,12 @@ impl ObligationCauseCode<'_> {
     // Return the base obligation, ignoring derived obligations.
     pub fn peel_derives(&self) -> &Self {
         let mut base_cause = self;
-        while let BuiltinDerivedObligation(cause)
-        | ImplDerivedObligation(cause)
-        | DerivedObligation(cause) = base_cause
+        while let BuiltinDerivedObligation(DerivedObligationCause { parent_code, .. })
+        | ImplDerivedObligation(DerivedObligationCause { parent_code, .. })
+        | DerivedObligation(DerivedObligationCause { parent_code, .. })
+        | FunctionArgumentObligation { parent_code, .. } = base_cause
         {
-            base_cause = &cause.parent_code;
+            base_cause = &parent_code;
         }
         base_cause
     }

compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs

@@ -57,7 +57,6 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
                 .map(|obligation| FulfillmentError {
                     obligation: obligation.clone(),
                     code: FulfillmentErrorCode::CodeAmbiguity,
-                    points_at_arg_span: false,
                     // FIXME - does Chalk have a notation of 'root obligation'?
                     // This is just for diagnostics, so it's okay if this is wrong
                     root_obligation: obligation.clone(),
@@ -112,7 +111,6 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
                                     code: FulfillmentErrorCode::CodeSelectionError(
                                         SelectionError::Unimplemented,
                                     ),
-                                    points_at_arg_span: false,
                                     // FIXME - does Chalk have a notation of 'root obligation'?
                                     // This is just for diagnostics, so it's okay if this is wrong
                                     root_obligation: obligation,
@@ -129,7 +127,6 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
                         code: FulfillmentErrorCode::CodeSelectionError(
                             SelectionError::Unimplemented,
                         ),
-                        points_at_arg_span: false,
                         // FIXME - does Chalk have a notation of 'root obligation'?
                         // This is just for diagnostics, so it's okay if this is wrong
                         root_obligation: obligation,

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

@@ -66,7 +66,6 @@ pub trait InferCtxtExt<'tcx> {
         root_obligation: &PredicateObligation<'tcx>,
         error: &SelectionError<'tcx>,
         fallback_has_occurred: bool,
-        points_at_arg: bool,
     );
 
     /// Given some node representing a fn-like thing in the HIR map,
@@ -237,7 +236,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         root_obligation: &PredicateObligation<'tcx>,
         error: &SelectionError<'tcx>,
         fallback_has_occurred: bool,
-        points_at_arg: bool,
     ) {
         let tcx = self.tcx;
         let mut span = obligation.cause.span;
@@ -387,7 +385,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             &obligation,
                             &mut err,
                             &trait_ref,
-                            points_at_arg,
                             have_alt_message,
                         ) {
                             self.note_obligation_cause(&mut err, &obligation);
@@ -430,8 +427,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             err.span_label(enclosing_scope_span, s.as_str());
                         }
 
-                        self.suggest_dereferences(&obligation, &mut err, trait_ref, points_at_arg);
-                        self.suggest_fn_call(&obligation, &mut err, trait_ref, points_at_arg);
+                        self.suggest_dereferences(&obligation, &mut err, trait_ref);
+                        self.suggest_fn_call(&obligation, &mut err, trait_ref);
                         self.suggest_remove_reference(&obligation, &mut err, trait_ref);
                         self.suggest_semicolon_removal(&obligation, &mut err, span, trait_ref);
                         self.note_version_mismatch(&mut err, &trait_ref);
@@ -500,12 +497,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                         // Changing mutability doesn't make a difference to whether we have
                         // an `Unsize` impl (Fixes ICE in #71036)
                         if !is_unsize {
-                            self.suggest_change_mut(
-                                &obligation,
-                                &mut err,
-                                trait_ref,
-                                points_at_arg,
-                            );
+                            self.suggest_change_mut(&obligation, &mut err, trait_ref);
                         }
 
                         // If this error is due to `!: Trait` not implemented but `(): Trait` is
@@ -1214,7 +1206,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
                     &error.root_obligation,
                     selection_error,
                     fallback_has_occurred,
-                    error.points_at_arg_span,
                 );
             }
             FulfillmentErrorCode::CodeProjectionError(ref e) => {

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

@@ -9,6 +9,7 @@ use crate::traits::normalize_projection_type;
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
+use rustc_data_structures::sync::Lrc;
 use rustc_errors::{error_code, struct_span_err, Applicability, DiagnosticBuilder, Style};
 use rustc_hir as hir;
 use rustc_hir::def::DefKind;
@@ -54,7 +55,6 @@ pub trait InferCtxtExt<'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'tcx>,
         trait_ref: ty::PolyTraitRef<'tcx>,
-        points_at_arg: bool,
     );
 
     fn get_closure_name(
@@ -69,15 +69,13 @@ pub trait InferCtxtExt<'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
-        points_at_arg: bool,
     );
 
     fn suggest_add_reference_to_arg(
         &self,
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: &ty::Binder<'tcx, ty::TraitRef<'tcx>>,
-        points_at_arg: bool,
         has_custom_message: bool,
     ) -> bool;
 
@@ -93,7 +91,6 @@ pub trait InferCtxtExt<'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
-        points_at_arg: bool,
     );
 
     fn suggest_semicolon_removal(
@@ -490,16 +487,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'tcx>,
         trait_ref: ty::PolyTraitRef<'tcx>,
-        points_at_arg: bool,
     ) {
         // It only make sense when suggesting dereferences for arguments
-        if !points_at_arg {
+        let code = if let ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } =
+            &obligation.cause.code
+        {
+            parent_code.clone()
+        } else {
             return;
-        }
+        };
         let param_env = obligation.param_env;
         let body_id = obligation.cause.body_id;
         let span = obligation.cause.span;
-        let real_trait_ref = match &obligation.cause.code {
+        let real_trait_ref = match &*code {
             ObligationCauseCode::ImplDerivedObligation(cause)
             | ObligationCauseCode::DerivedObligation(cause)
             | ObligationCauseCode::BuiltinDerivedObligation(cause) => cause.parent_trait_ref,
@@ -584,7 +584,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
-        points_at_arg: bool,
     ) {
         let self_ty = match trait_ref.self_ty().no_bound_vars() {
             None => return,
@@ -656,11 +655,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             }
             _ => return,
         };
-        if points_at_arg {
+        if matches!(obligation.cause.code, ObligationCauseCode::FunctionArgumentObligation { .. }) {
             // When the obligation error has been ensured to have been caused by
             // an argument, the `obligation.cause.span` points at the expression
-            // of the argument, so we can provide a suggestion. This is signaled
-            // by `points_at_arg`. Otherwise, we give a more general note.
+            // of the argument, so we can provide a suggestion. Otherwise, we give
+            // a more general note.
             err.span_suggestion_verbose(
                 obligation.cause.span.shrink_to_hi(),
                 &msg,
@@ -677,18 +676,21 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: &ty::Binder<'tcx, ty::TraitRef<'tcx>>,
-        points_at_arg: bool,
         has_custom_message: bool,
     ) -> bool {
         let span = obligation.cause.span;
-        let points_at_for_iter = matches!(
-            span.ctxt().outer_expn_data().kind,
-            ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
-        );
 
-        if !points_at_arg && !points_at_for_iter {
+        let code = if let ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } =
+            &obligation.cause.code
+        {
+            parent_code.clone()
+        } else if let ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter)) =
+            span.ctxt().outer_expn_data().kind
+        {
+            Lrc::new(obligation.cause.code.clone())
+        } else {
             return false;
-        }
+        };
 
         // List of traits for which it would be nonsensical to suggest borrowing.
         // For instance, immutable references are always Copy, so suggesting to
@@ -787,7 +789,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             return false;
         };
 
-        if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
+        if let ObligationCauseCode::ImplDerivedObligation(obligation) = &*code {
             let expected_trait_ref = obligation.parent_trait_ref.skip_binder();
             let new_imm_trait_ref =
                 ty::TraitRef::new(obligation.parent_trait_ref.def_id(), imm_substs);
@@ -799,7 +801,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 return try_borrowing(new_mut_trait_ref, expected_trait_ref, true, &[]);
             }
         } else if let ObligationCauseCode::BindingObligation(_, _)
-        | ObligationCauseCode::ItemObligation(_) = &obligation.cause.code
+        | ObligationCauseCode::ItemObligation(_) = &*code
         {
             if try_borrowing(
                 ty::TraitRef::new(trait_ref.def_id, imm_substs),
@@ -891,8 +893,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
         err: &mut DiagnosticBuilder<'_>,
         trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
-        points_at_arg: bool,
     ) {
+        let points_at_arg = matches!(
+            obligation.cause.code,
+            ObligationCauseCode::FunctionArgumentObligation { .. },
+        );
+
         let span = obligation.cause.span;
         if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
             let refs_number =
@@ -2289,6 +2295,56 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     )
                 });
             }
+            ObligationCauseCode::FunctionArgumentObligation {
+                arg_hir_id,
+                call_hir_id,
+                ref parent_code,
+            } => {
+                let hir = self.tcx.hir();
+                if let Some(Node::Expr(expr @ hir::Expr { kind: hir::ExprKind::Block(..), .. })) =
+                    hir.find(arg_hir_id)
+                {
+                    let in_progress_typeck_results =
+                        self.in_progress_typeck_results.map(|t| t.borrow());
+                    let parent_id = hir.local_def_id(hir.get_parent_item(arg_hir_id));
+                    let typeck_results: &TypeckResults<'tcx> = match &in_progress_typeck_results {
+                        Some(t) if t.hir_owner == parent_id => t,
+                        _ => self.tcx.typeck(parent_id),
+                    };
+                    let ty = typeck_results.expr_ty_adjusted(expr);
+                    let span = expr.peel_blocks().span;
+                    if Some(span) != err.span.primary_span() {
+                        err.span_label(
+                            span,
+                            &if ty.references_error() {
+                                String::new()
+                            } else {
+                                format!("this tail expression is of type `{:?}`", ty)
+                            },
+                        );
+                    }
+                }
+                if let Some(Node::Expr(hir::Expr {
+                    kind:
+                        hir::ExprKind::Call(hir::Expr { span, .. }, _)
+                        | hir::ExprKind::MethodCall(_, span, ..),
+                    ..
+                })) = hir.find(call_hir_id)
+                {
+                    if Some(*span) != err.span.primary_span() {
+                        err.span_label(*span, "required by a bound introduced by this call");
+                    }
+                }
+                ensure_sufficient_stack(|| {
+                    self.note_obligation_cause_code(
+                        err,
+                        predicate,
+                        &parent_code,
+                        obligated_types,
+                        seen_requirements,
+                    )
+                });
+            }
             ObligationCauseCode::CompareImplMethodObligation {
                 item_name,
                 trait_item_def_id,

compiler/rustc_typeck/src/check/callee.rs

@@ -72,7 +72,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         arg_exprs: &'tcx [hir::Expr<'tcx>],
         expected: Expectation<'tcx>,
     ) -> Ty<'tcx> {
-        let original_callee_ty = self.check_expr(callee_expr);
+        let original_callee_ty = match &callee_expr.kind {
+            hir::ExprKind::Path(hir::QPath::Resolved(..) | hir::QPath::TypeRelative(..)) => self
+                .check_expr_with_expectation_and_args(
+                    callee_expr,
+                    Expectation::NoExpectation,
+                    arg_exprs,
+                ),
+            _ => self.check_expr(callee_expr),
+        };
+
         let expr_ty = self.structurally_resolved_type(call_expr.span, original_callee_ty);
 
         let mut autoderef = self.autoderef(callee_expr.span, expr_ty);

compiler/rustc_typeck/src/check/coercion.rs

@@ -707,13 +707,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
 
                 // Object safety violations or miscellaneous.
                 Err(err) => {
-                    self.report_selection_error(
-                        obligation.clone(),
-                        &obligation,
-                        &err,
-                        false,
-                        false,
-                    );
+                    self.report_selection_error(obligation.clone(), &obligation, &err, false);
                     // Treat this like an obligation and follow through
                     // with the unsizing - the lack of a coercion should
                     // be silent, as it causes a type mismatch later.