Point at source of trait bound obligations in more places

Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).

In particular, we now point at trait bounds on method calls.

We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).

Address part of rust-lang#89418.

Author: estebank

compiler/rustc_infer/src/infer/error_reporting/mod.rs

@@ -2113,10 +2113,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                         None
                     },
                     self.tcx.generics_of(owner.to_def_id()),
+                    hir.span(hir_id),
                 )
             });
+
+        let span = match generics {
+            // This is to get around the trait identity obligation, that has a `DUMMY_SP` as signal
+            // for other diagnostics, so we need to recover it here.
+            Some((_, _, node)) if span.is_dummy() => node,
+            _ => span,
+        };
+
         let type_param_span = match (generics, bound_kind) {
-            (Some((_, ref generics)), GenericKind::Param(ref param)) => {
+            (Some((_, ref generics, _)), GenericKind::Param(ref param)) => {
                 // Account for the case where `param` corresponds to `Self`,
                 // which doesn't have the expected type argument.
                 if !(generics.has_self && param.index == 0) {
@@ -2153,7 +2162,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         };
         let new_lt = generics
             .as_ref()
-            .and_then(|(parent_g, g)| {
+            .and_then(|(parent_g, g, _)| {
                 let mut possible = (b'a'..=b'z').map(|c| format!("'{}", c as char));
                 let mut lts_names = g
                     .params
@@ -2175,7 +2184,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             .unwrap_or("'lt".to_string());
         let add_lt_sugg = generics
             .as_ref()
-            .and_then(|(_, g)| g.params.first())
+            .and_then(|(_, g, _)| g.params.first())
             .and_then(|param| param.def_id.as_local())
             .map(|def_id| {
                 (

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

@@ -192,14 +192,16 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                 ObligationCauseCode::MatchImpl(parent, ..) => &parent.code,
                 _ => &cause.code,
             };
-            if let ObligationCauseCode::ItemObligation(item_def_id) = *code {
+            if let (ObligationCauseCode::ItemObligation(item_def_id), None) =
+                (code, override_error_code)
+            {
                 // Same case of `impl Foo for dyn Bar { fn qux(&self) {} }` introducing a `'static`
                 // lifetime as above, but called using a fully-qualified path to the method:
                 // `Foo::qux(bar)`.
                 let mut v = TraitObjectVisitor(FxHashSet::default());
                 v.visit_ty(param.param_ty);
                 if let Some((ident, self_ty)) =
-                    self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0)
+                    self.get_impl_ident_and_self_ty_from_trait(*item_def_id, &v.0)
                 {
                     if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0, ident, self_ty) {
                         override_error_code = Some(ident);

compiler/rustc_trait_selection/src/traits/util.rs

@@ -9,7 +9,9 @@ use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef};
 use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness};
 
 use super::{Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext};
-pub use rustc_infer::traits::util::*;
+pub use rustc_infer::traits::{self, util::*};
+
+use std::iter;
 
 ///////////////////////////////////////////////////////////////////////////
 // `TraitAliasExpander` iterator
@@ -229,11 +231,16 @@ pub fn predicates_for_generics<'tcx>(
 ) -> impl Iterator<Item = PredicateObligation<'tcx>> {
     debug!("predicates_for_generics(generic_bounds={:?})", generic_bounds);
 
-    generic_bounds.predicates.into_iter().map(move |predicate| Obligation {
-        cause: cause.clone(),
-        recursion_depth,
-        param_env,
-        predicate,
+    iter::zip(generic_bounds.predicates, generic_bounds.spans).map(move |(predicate, span)| {
+        let cause = match cause.code {
+            traits::ItemObligation(def_id) if !span.is_dummy() => traits::ObligationCause::new(
+                cause.span,
+                cause.body_id,
+                traits::BindingObligation(def_id, span),
+            ),
+            _ => cause.clone(),
+        };
+        Obligation { cause, recursion_depth, param_env, predicate }
     })
 }
 

compiler/rustc_trait_selection/src/traits/wf.rs

@@ -709,7 +709,12 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
 
         iter::zip(iter::zip(predicates.predicates, predicates.spans), origins.into_iter().rev())
             .map(|((pred, span), origin_def_id)| {
-                let cause = self.cause(traits::BindingObligation(origin_def_id, span));
+                let code = if span.is_dummy() {
+                    traits::MiscObligation
+                } else {
+                    traits::BindingObligation(origin_def_id, span)
+                };
+                let cause = self.cause(code);
                 traits::Obligation::with_depth(cause, self.recursion_depth, self.param_env, pred)
             })
             .filter(|pred| !pred.has_escaping_bound_vars())

compiler/rustc_typeck/src/check/compare_method.rs

@@ -1385,12 +1385,13 @@ pub fn check_type_bounds<'tcx>(
 
         let impl_ty_hir_id = tcx.hir().local_def_id_to_hir_id(impl_ty.def_id.expect_local());
         let normalize_cause = traits::ObligationCause::misc(impl_ty_span, impl_ty_hir_id);
-        let mk_cause = |span| {
-            ObligationCause::new(
-                impl_ty_span,
-                impl_ty_hir_id,
-                ObligationCauseCode::BindingObligation(trait_ty.def_id, span),
-            )
+        let mk_cause = |span: Span| {
+            let code = if span.is_dummy() {
+                traits::MiscObligation
+            } else {
+                traits::BindingObligation(trait_ty.def_id, span)
+            };
+            ObligationCause::new(impl_ty_span, impl_ty_hir_id, code)
         };
 
         let obligations = tcx

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

@@ -586,38 +586,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
-    /// Given a fully substituted set of bounds (`generic_bounds`), and the values with which each
-    /// type/region parameter was instantiated (`substs`), creates and registers suitable
-    /// trait/region obligations.
-    ///
-    /// For example, if there is a function:
-    ///
-    /// ```
-    /// fn foo<'a,T:'a>(...)
-    /// ```
-    ///
-    /// and a reference:
-    ///
-    /// ```
-    /// let f = foo;
-    /// ```
-    ///
-    /// Then we will create a fresh region variable `'$0` and a fresh type variable `$1` for `'a`
-    /// and `T`. This routine will add a region obligation `$1:'$0` and register it locally.
-    pub fn add_obligations_for_parameters(
-        &self,
-        cause: traits::ObligationCause<'tcx>,
-        predicates: ty::InstantiatedPredicates<'tcx>,
-    ) {
-        assert!(!predicates.has_escaping_bound_vars());
-
-        debug!("add_obligations_for_parameters(predicates={:?})", predicates);
-
-        for obligation in traits::predicates_for_generics(cause, self.param_env, predicates) {
-            self.register_predicate(obligation);
-        }
-    }
-
     // FIXME(arielb1): use this instead of field.ty everywhere
     // Only for fields! Returns <none> for methods>
     // Indifferent to privacy flags
@@ -1522,20 +1490,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
     /// Add all the obligations that are required, substituting and normalized appropriately.
     #[tracing::instrument(level = "debug", skip(self, span, def_id, substs))]
-    fn add_required_obligations(&self, span: Span, def_id: DefId, substs: &SubstsRef<'tcx>) {
-        let (bounds, spans) = self.instantiate_bounds(span, def_id, &substs);
+    crate fn add_required_obligations(&self, span: Span, def_id: DefId, substs: &SubstsRef<'tcx>) {
+        let (bounds, _) = self.instantiate_bounds(span, def_id, &substs);
 
-        for (i, mut obligation) in traits::predicates_for_generics(
+        for obligation in traits::predicates_for_generics(
             traits::ObligationCause::new(span, self.body_id, traits::ItemObligation(def_id)),
             self.param_env,
             bounds,
-        )
-        .enumerate()
-        {
-            // This makes the error point at the bound, but we want to point at the argument
-            if let Some(span) = spans.get(i) {
-                obligation.cause.make_mut().code = traits::BindingObligation(def_id, *span);
-            }
+        ) {
             self.register_predicate(obligation);
         }
     }

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

@@ -509,10 +509,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             self.write_user_type_annotation_from_substs(hir_id, did, substs, None);
 
             // Check bounds on type arguments used in the path.
-            let (bounds, _) = self.instantiate_bounds(path_span, did, substs);
-            let cause =
-                traits::ObligationCause::new(path_span, self.body_id, traits::ItemObligation(did));
-            self.add_obligations_for_parameters(cause, bounds);
+            self.add_required_obligations(path_span, did, substs);
 
             Some((variant, ty))
         } else {

compiler/rustc_typeck/src/check/method/confirm.rs

@@ -120,7 +120,12 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         // We won't add these if we encountered an illegal sized bound, so that we can use
         // a custom error in that case.
         if illegal_sized_bound.is_none() {
-            self.add_obligations(self.tcx.mk_fn_ptr(method_sig), all_substs, method_predicates);
+            self.add_obligations(
+                self.tcx.mk_fn_ptr(method_sig),
+                all_substs,
+                method_predicates,
+                pick.item.def_id,
+            );
         }
 
         // Create the final `MethodCallee`.
@@ -471,16 +476,23 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         fty: Ty<'tcx>,
         all_substs: SubstsRef<'tcx>,
         method_predicates: ty::InstantiatedPredicates<'tcx>,
+        def_id: DefId,
     ) {
         debug!(
-            "add_obligations: fty={:?} all_substs={:?} method_predicates={:?}",
-            fty, all_substs, method_predicates
+            "add_obligations: fty={:?} all_substs={:?} method_predicates={:?} def_id={:?}",
+            fty, all_substs, method_predicates, def_id
         );
 
-        self.add_obligations_for_parameters(
-            traits::ObligationCause::misc(self.span, self.body_id),
+        // FIXME: could replace with the following, but we already calculated `method_predicates`,
+        // so we just call `predicates_for_generics` directly to avoid redoing work.
+        // `self.add_required_obligations(self.span, def_id, &all_substs);`
+        for obligation in traits::predicates_for_generics(
+            traits::ObligationCause::new(self.span, self.body_id, traits::ItemObligation(def_id)),
+            self.param_env,
             method_predicates,
-        );
+        ) {
+            self.register_predicate(obligation);
+        }
 
         // this is a projection from a trait reference, so we have to
         // make sure that the trait reference inputs are well-formed.

compiler/rustc_typeck/src/collect.rs

@@ -1990,16 +1990,12 @@ fn predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericPredicates<'_> {
         // prove that the trait applies to the types that were
         // used, and adding the predicate into this list ensures
         // that this is done.
-        let mut span = tcx.def_span(def_id);
-        if tcx.sess.source_map().is_local_span(span) {
-            // `guess_head_span` reads the actual source file from
-            // disk to try to determine the 'head' snippet of the span.
-            // Don't do this for a span that comes from a file outside
-            // of our crate, since this would make our query output
-            // (and overall crate metadata) dependent on the
-            // *current* state of an external file.
-            span = tcx.sess.source_map().guess_head_span(span);
-        }
+        //
+        // We use a DUMMY_SP here as a way to signal trait bounds that come
+        // from the trait itself that *shouldn't* be shown as the source of
+        // an obligation and instead be skipped. Otherwise we'd use
+        // `tcx.def_span(def_id);`
+        let span = rustc_span::DUMMY_SP;
         result.predicates =
             tcx.arena.alloc_from_iter(result.predicates.iter().copied().chain(std::iter::once((
                 ty::TraitRef::identity(tcx, def_id).without_const().to_predicate(tcx),

src/test/ui/associated-type-bounds/bad-bounds-on-assoc-in-trait.stderr

@@ -5,13 +5,6 @@ LL |     type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
    |                                    ^^^^ `<<Self as Case1>::C as Iterator>::Item` cannot be sent between threads safely
    |
    = help: the trait `Send` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
-note: required by a bound in `Send`
-  --> $SRC_DIR/core/src/marker.rs:LL:COL
-   |
-LL | / pub unsafe auto trait Send {
-LL | |     // empty.
-LL | | }
-   | |_^ required by this bound in `Send`
 help: consider further restricting the associated type
    |
 LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Send {
@@ -24,17 +17,6 @@ LL |     type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `<<Self as Case1>::C as Iterator>::Item` is not an iterator
    |
    = help: the trait `Iterator` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
-note: required by a bound in `Iterator`
-  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
-   |
-LL | / pub trait Iterator {
-LL | |     /// The type of the elements being iterated over.
-LL | |     #[stable(feature = "rust1", since = "1.0.0")]
-LL | |     type Item;
-...  |
-LL | |     }
-LL | | }
-   | |_^ required by this bound in `Iterator`
 help: consider further restricting the associated type
    |
 LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Iterator {
@@ -47,17 +29,6 @@ LL |     type C: Clone + Iterator<Item: Send + Iterator<Item: for<'a> Lam<&'a u8
    |                                                                                             ^^^^ `<<Self as Case1>::C as Iterator>::Item` cannot be shared between threads safely
    |
    = help: the trait `Sync` is not implemented for `<<Self as Case1>::C as Iterator>::Item`
-note: required by a bound in `Sync`
-  --> $SRC_DIR/core/src/marker.rs:LL:COL
-   |
-LL | / pub unsafe auto trait Sync {
-LL | |     // FIXME(estebank): once support to add notes in `rustc_on_unimplemented`
-LL | |     // lands in beta, and it has been extended to check whether a closure is
-LL | |     // anywhere in the requirement chain, extend it as such (#48534):
-...  |
-LL | |     // Empty
-LL | | }
-   | |_^ required by this bound in `Sync`
 help: consider further restricting the associated type
    |
 LL | trait Case1 where <<Self as Case1>::C as Iterator>::Item: Sync {