Refactor FulfillmentError to track less data

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

Author: estebank

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

@@ -54,7 +54,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 +68,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 +90,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 +486,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
+        {
+            std::rc::Rc::clone(parent_code)
+        } 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 +583,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 +654,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,7 +675,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,
         has_custom_message: bool,
     ) -> bool {
         let span = obligation.cause.span;
@@ -686,9 +683,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
         );
 
-        if !points_at_arg && !points_at_for_iter {
-            return false;
-        }
+        let code =
+            if let (ObligationCauseCode::FunctionArgumentObligation { parent_code, .. }, false) =
+                (&obligation.cause.code, points_at_for_iter)
+            {
+                std::rc::Rc::clone(parent_code)
+            } 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,21 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     )
                 });
             }
+            ObligationCauseCode::FunctionArgumentObligation {
+                arg_hir_id: _,
+                call_hir_id: _,
+                ref parent_code,
+            } => {
+                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/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.

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

@@ -9,6 +9,7 @@ use crate::check::{
 };
 
 use rustc_ast as ast;
+use rustc_data_structures::sync::Lrc;
 use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticId};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorOf, DefKind, Res};
@@ -324,6 +325,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.point_at_arg_instead_of_call_if_possible(
                         errors,
                         &final_arg_types[..],
+                        expr,
                         sp,
                         &args,
                     );
@@ -391,7 +393,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         ) {
                             for error in errors {
                                 error.obligation.cause.make_mut().span = arg.span;
-                                error.points_at_arg_span = true;
+                                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),
+                                    };
                             }
                         }
                     },
@@ -937,6 +945,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         errors: &mut Vec<traits::FulfillmentError<'tcx>>,
         final_arg_types: &[(usize, Ty<'tcx>, Ty<'tcx>)],
+        expr: &'tcx hir::Expr<'tcx>,
         call_sp: Span,
         args: &'tcx [hir::Expr<'tcx>],
     ) {
@@ -986,7 +995,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     // 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;
-                    error.points_at_arg_span = true;
+                    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),
+                        };
                 }
             }
         }