Use a proof tree visitor to refine the Obligation for error reporting

Author: compiler-errors

compiler/rustc_trait_selection/src/solve/fulfill.rs

@@ -1,15 +1,19 @@
 use std::mem;
+use std::ops::ControlFlow;
 
 use rustc_infer::infer::InferCtxt;
-use rustc_infer::traits::solve::MaybeCause;
+use rustc_infer::traits::query::NoSolution;
+use rustc_infer::traits::solve::inspect::ProbeKind;
+use rustc_infer::traits::solve::{CandidateSource, GoalSource, MaybeCause};
 use rustc_infer::traits::{
-    query::NoSolution, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes,
+    self, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes, Obligation,
     PredicateObligation, SelectionError, TraitEngine,
 };
 use rustc_middle::ty;
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 
 use super::eval_ctxt::GenerateProofTree;
+use super::inspect::{ProofTreeInferCtxtExt, ProofTreeVisitor};
 use super::{Certainty, InferCtxtEvalExt};
 
 /// A trait engine using the new trait solver.
@@ -133,9 +137,9 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentCtxt<'tcx> {
             .collect();
 
         errors.extend(self.obligations.overflowed.drain(..).map(|obligation| FulfillmentError {
-            root_obligation: obligation.clone(),
+            obligation: find_best_leaf_obligation(infcx, &obligation),
             code: FulfillmentErrorCode::Ambiguity { overflow: Some(true) },
-            obligation,
+            root_obligation: obligation,
         }));
 
         errors
@@ -234,7 +238,12 @@ fn fulfillment_error_for_no_solution<'tcx>(
             bug!("unexpected goal: {obligation:?}")
         }
     };
-    FulfillmentError { root_obligation: obligation.clone(), code, obligation }
+
+    FulfillmentError {
+        obligation: find_best_leaf_obligation(infcx, &obligation),
+        code,
+        root_obligation: obligation,
+    }
 }
 
 fn fulfillment_error_for_stalled<'tcx>(
@@ -258,5 +267,149 @@ fn fulfillment_error_for_stalled<'tcx>(
         }
     });
 
-    FulfillmentError { obligation: obligation.clone(), code, root_obligation: obligation }
+    FulfillmentError {
+        obligation: find_best_leaf_obligation(infcx, &obligation),
+        code,
+        root_obligation: obligation,
+    }
+}
+
+struct BestObligation<'tcx> {
+    parent_kind: Option<ProbeKind<'tcx>>,
+    parent_obligation: PredicateObligation<'tcx>,
+    impl_where_bound_count: usize,
+}
+
+impl<'tcx> BestObligation<'tcx> {
+    fn with_derived_obligation(
+        &mut self,
+        new_kind: ProbeKind<'tcx>,
+        derive_obligation: impl FnOnce(&mut Self, ProbeKind<'tcx>) -> PredicateObligation<'tcx>,
+        and_then: impl FnOnce(&mut Self) -> <Self as ProofTreeVisitor<'tcx>>::Result,
+    ) -> <Self as ProofTreeVisitor<'tcx>>::Result {
+        let mut old_obligation = None;
+        let old_kind = self.parent_kind.replace(new_kind);
+        if let Some(old_kind) = old_kind {
+            let new_obligation = derive_obligation(self, old_kind);
+            old_obligation = Some(std::mem::replace(&mut self.parent_obligation, new_obligation));
+        }
+        let old_impl_where_bound_count = std::mem::replace(&mut self.impl_where_bound_count, 0);
+
+        let res = and_then(self);
+
+        if let Some(old_obligation) = old_obligation {
+            self.parent_obligation = old_obligation;
+        }
+        self.parent_kind = old_kind;
+        self.impl_where_bound_count = old_impl_where_bound_count;
+
+        res
+    }
+}
+
+impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
+    type Result = ControlFlow<PredicateObligation<'tcx>>;
+
+    fn span(&self) -> rustc_span::Span {
+        self.parent_obligation.cause.span
+    }
+
+    fn visit_goal(&mut self, goal: &super::inspect::InspectGoal<'_, 'tcx>) -> Self::Result {
+        if matches!(goal.source(), GoalSource::ImplWhereBound) {
+            self.impl_where_bound_count += 1;
+        }
+
+        if goal.result().is_ok() {
+            return ControlFlow::Continue(());
+        }
+
+        let candidates = goal.candidates();
+        // FIXME: We should try to throw out the candidates that are definitely
+        // not worthwhile, such as param-env and impl candidates whose headers
+        // won't even unify. We already do this with deep-reject for impls, but
+        // we shouldn't rely on this for diagnostic correctness.
+        let [candidate] = candidates.as_slice() else {
+            return ControlFlow::Break(self.parent_obligation.clone());
+        };
+
+        // FIXME: Could we extract a trait ref from a projection here too?
+        // FIXME: Also, what about considering >1 layer up the stack? May be necessary
+        // for normalizes-to.
+        let Some(parent_trait_pred) = self.parent_obligation.predicate.to_opt_poly_trait_pred()
+        else {
+            return ControlFlow::Break(self.parent_obligation.clone());
+        };
+
+        self.with_derived_obligation(
+            candidate.kind(),
+            |self_, parent_kind| {
+                let mut cause = self_.parent_obligation.cause.clone();
+                cause = match (parent_kind, goal.source()) {
+                    (
+                        ProbeKind::TraitCandidate {
+                            source: CandidateSource::Impl(impl_def_id),
+                            result: _,
+                        },
+                        GoalSource::ImplWhereBound,
+                    ) => {
+                        let idx = self_.impl_where_bound_count - 1;
+                        if let Some((_, span)) = goal
+                            .infcx()
+                            .tcx
+                            .predicates_of(impl_def_id)
+                            .instantiate_identity(goal.infcx().tcx)
+                            .iter()
+                            .nth(idx)
+                        {
+                            cause.derived_cause(parent_trait_pred, |derived| {
+                                traits::ImplDerivedObligation(Box::new(
+                                    traits::ImplDerivedObligationCause {
+                                        derived,
+                                        impl_or_alias_def_id: impl_def_id,
+                                        impl_def_predicate_index: Some(idx),
+                                        span,
+                                    },
+                                ))
+                            })
+                        } else {
+                            cause
+                        }
+                    }
+                    (_, GoalSource::ImplWhereBound) => {
+                        cause.derived_cause(parent_trait_pred, traits::BuiltinDerivedObligation)
+                    }
+                    _ => cause,
+                };
+
+                Obligation {
+                    cause,
+                    param_env: goal.goal().param_env,
+                    predicate: goal.goal().predicate,
+                    recursion_depth: self_.parent_obligation.recursion_depth + 1,
+                }
+            },
+            |self_| {
+                candidate.visit_nested_no_probe(self_)?;
+                ControlFlow::Break(self_.parent_obligation.clone())
+            },
+        )
+    }
+}
+
+fn find_best_leaf_obligation<'tcx>(
+    infcx: &InferCtxt<'tcx>,
+    obligation: &PredicateObligation<'tcx>,
+) -> PredicateObligation<'tcx> {
+    let obligation = infcx.resolve_vars_if_possible(obligation.clone());
+    infcx
+        .visit_proof_tree(
+            obligation.clone().into(),
+            &mut BestObligation {
+                parent_kind: None,
+                parent_obligation: obligation.clone(),
+                impl_where_bound_count: 0,
+            },
+        )
+        .break_value()
+        .unwrap_or(obligation)
 }

tests/ui/for/issue-20605.next.stderr

@@ -2,9 +2,14 @@ error[E0277]: `dyn Iterator<Item = &'a mut u8>` is not an iterator
   --> $DIR/issue-20605.rs:6:17
    |
 LL |     for item in *things { *item = 0 }
-   |                 ^^^^^^^ `dyn Iterator<Item = &'a mut u8>` is not an iterator
+   |                 ^^^^^^^ the trait `IntoIterator` is not implemented for `dyn Iterator<Item = &'a mut u8>`
    |
-   = help: the trait `IntoIterator` is not implemented for `dyn Iterator<Item = &'a mut u8>`
+   = note: the trait bound `dyn Iterator<Item = &'a mut u8>: IntoIterator` is not satisfied
+   = note: required for `dyn Iterator<Item = &'a mut u8>` to implement `IntoIterator`
+help: consider mutably borrowing here
+   |
+LL |     for item in &mut *things { *item = 0 }
+   |                 ++++
 
 error: the type `<dyn Iterator<Item = &'a mut u8> as IntoIterator>::IntoIter` is not well-formed
   --> $DIR/issue-20605.rs:6:17

tests/ui/traits/next-solver/auto-with-drop_tracking_mir.fail.stderr

@@ -1,18 +1,26 @@
-error[E0277]: `impl Future<Output = ()>` cannot be sent between threads safely
+error: future cannot be sent between threads safely
   --> $DIR/auto-with-drop_tracking_mir.rs:25:13
    |
 LL |     is_send(foo());
-   |     ------- ^^^^^ `impl Future<Output = ()>` cannot be sent between threads safely
-   |     |
-   |     required by a bound introduced by this call
+   |             ^^^^^ future returned by `foo` is not `Send`
    |
-   = help: the trait `Send` is not implemented for `impl Future<Output = ()>`
+   = help: the trait `Sync` is not implemented for `impl Future<Output = ()>`, which is required by `impl Future<Output = ()>: Send`
+note: future is not `Send` as this value is used across an await
+  --> $DIR/auto-with-drop_tracking_mir.rs:16:11
+   |
+LL |     let x = &NotSync;
+   |         - has type `&NotSync` which is not `Send`
+LL |     bar().await;
+   |           ^^^^^ await occurs here, with `x` maybe used later
 note: required by a bound in `is_send`
   --> $DIR/auto-with-drop_tracking_mir.rs:24:24
    |
 LL |     fn is_send(_: impl Send) {}
    |                        ^^^^ required by this bound in `is_send`
+help: consider dereferencing here
+   |
+LL |     is_send(*foo());
+   |             +
 
 error: aborting due to 1 previous error
 
-For more information about this error, try `rustc --explain E0277`.

tests/ui/traits/next-solver/auto-with-drop_tracking_mir.rs

@@ -23,5 +23,5 @@ async fn bar() {}
 fn main() {
     fn is_send(_: impl Send) {}
     is_send(foo());
-    //[fail]~^ ERROR `impl Future<Output = ()>` cannot be sent between threads safely
+    //[fail]~^ ERROR future cannot be sent between threads safely
 }

tests/ui/traits/next-solver/builtin-fn-must-return-sized.rs

@@ -13,5 +13,5 @@ fn foo<F: Fn<T>, T: Tuple>(f: Option<F>, t: T) {
 
 fn main() {
     foo::<fn() -> str, _>(None, ());
-    //~^ expected a `Fn<_>` closure, found `fn() -> str`
+    //~^ the size for values of type `str` cannot be known at compilation time
 }

tests/ui/traits/next-solver/builtin-fn-must-return-sized.stderr

@@ -1,10 +1,10 @@
-error[E0277]: expected a `Fn<_>` closure, found `fn() -> str`
+error[E0277]: the size for values of type `str` cannot be known at compilation time
   --> $DIR/builtin-fn-must-return-sized.rs:15:11
    |
 LL |     foo::<fn() -> str, _>(None, ());
-   |           ^^^^^^^^^^^ expected an `Fn<_>` closure, found `fn() -> str`
+   |           ^^^^^^^^^^^ doesn't have a size known at compile-time
    |
-   = help: the trait `Fn<_>` is not implemented for `fn() -> str`
+   = help: the trait `Sized` is not implemented for `str`
 note: required by a bound in `foo`
   --> $DIR/builtin-fn-must-return-sized.rs:10:11
    |

tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr

@@ -5,6 +5,16 @@ LL |     impls_trait::<A<X>, _, _, _>();
    |                   ^^^^ the trait `Trait<_, _, _>` is not implemented for `A<X>`
    |
    = help: the trait `Trait<U, V, D>` is implemented for `A<T>`
+note: required for `A<X>` to implement `Trait<_, _, _>`
+  --> $DIR/incompleteness-unstable-result.rs:32:50
+   |
+LL | impl<T: ?Sized, U: ?Sized, V: ?Sized, D: ?Sized> Trait<U, V, D> for A<T>
+   |                                                  ^^^^^^^^^^^^^^     ^^^^
+...
+LL |     A<T>: Trait<U, D, V>,
+   |           -------------- unsatisfied trait bound introduced here
+   = note: 9 redundant requirements hidden
+   = note: required for `A<X>` to implement `Trait<_, _, _>`
 note: required by a bound in `impls_trait`
   --> $DIR/incompleteness-unstable-result.rs:51:28
    |

tests/ui/traits/next-solver/object-unsafety.rs

@@ -11,9 +11,9 @@ fn copy<U: Setup + ?Sized>(from: &U::From) -> U::From {
 pub fn copy_any<T>(t: &T) -> T {
     copy::<dyn Setup<From=T>>(t)
     //~^ ERROR the type `&<dyn Setup<From = T> as Setup>::From` is not well-formed
-    //~| ERROR the trait bound `dyn Setup<From = T>: Setup` is not satisfied
     //~| ERROR mismatched types
     //~| ERROR the type `<dyn Setup<From = T> as Setup>::From` is not well-formed
+    //~| ERROR the trait bound `T: Copy` is not satisfied
 
     // FIXME(-Znext-solver): These error messages are horrible and some of them
     // are even simple fallout from previous error.

tests/ui/traits/next-solver/object-unsafety.stderr

@@ -1,18 +1,18 @@
-error[E0277]: the trait bound `dyn Setup<From = T>: Setup` is not satisfied
+error[E0277]: the trait bound `T: Copy` is not satisfied
   --> $DIR/object-unsafety.rs:12:12
    |
 LL |     copy::<dyn Setup<From=T>>(t)
-   |            ^^^^^^^^^^^^^^^^^ the trait `Setup` is not implemented for `dyn Setup<From = T>`
+   |            ^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `T`
    |
 note: required by a bound in `copy`
   --> $DIR/object-unsafety.rs:7:12
    |
 LL | fn copy<U: Setup + ?Sized>(from: &U::From) -> U::From {
    |            ^^^^^ required by this bound in `copy`
-help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
+help: consider restricting type parameter `T`
    |
-LL | pub fn copy_any<T>(t: &T) -> T where dyn Setup<From = T>: Setup {
-   |                                ++++++++++++++++++++++++++++++++
+LL | pub fn copy_any<T: std::marker::Copy>(t: &T) -> T {
+   |                  +++++++++++++++++++
 
 error: the type `&<dyn Setup<From = T> as Setup>::From` is not well-formed
   --> $DIR/object-unsafety.rs:12:31