From 6714216eaa8e3431706e451e2ff26401551207ea Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Fri, 3 May 2024 20:08:35 -0400
Subject: [PATCH] Only consider ambiguous goals when finding best obligation
 for ambiguities

---
 .../src/solve/fulfill.rs                      | 20 +++++++++--------
 .../incompleteness-unstable-result.rs         |  2 +-
 .../incompleteness-unstable-result.stderr     | 22 ++++++++++---------
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs
index 796222129f180..c0ece9655a5af 100644
--- a/compiler/rustc_trait_selection/src/solve/fulfill.rs
+++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs
@@ -137,7 +137,7 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentCtxt<'tcx> {
             .collect();
 
         errors.extend(self.obligations.overflowed.drain(..).map(|obligation| FulfillmentError {
-            obligation: find_best_leaf_obligation(infcx, &obligation),
+            obligation: find_best_leaf_obligation(infcx, &obligation, true),
             code: FulfillmentErrorCode::Ambiguity { overflow: Some(true) },
             root_obligation: obligation,
         }));
@@ -198,7 +198,7 @@ fn fulfillment_error_for_no_solution<'tcx>(
     infcx: &InferCtxt<'tcx>,
     root_obligation: PredicateObligation<'tcx>,
 ) -> FulfillmentError<'tcx> {
-    let obligation = find_best_leaf_obligation(infcx, &root_obligation);
+    let obligation = find_best_leaf_obligation(infcx, &root_obligation, false);
 
     let code = match obligation.predicate.kind().skip_binder() {
         ty::PredicateKind::Clause(ty::ClauseKind::Projection(_)) => {
@@ -266,7 +266,7 @@ fn fulfillment_error_for_stalled<'tcx>(
     });
 
     FulfillmentError {
-        obligation: find_best_leaf_obligation(infcx, &obligation),
+        obligation: find_best_leaf_obligation(infcx, &obligation, true),
         code,
         root_obligation: obligation,
     }
@@ -275,12 +275,13 @@ fn fulfillment_error_for_stalled<'tcx>(
 fn find_best_leaf_obligation<'tcx>(
     infcx: &InferCtxt<'tcx>,
     obligation: &PredicateObligation<'tcx>,
+    consider_ambiguities: bool,
 ) -> PredicateObligation<'tcx> {
     let obligation = infcx.resolve_vars_if_possible(obligation.clone());
     infcx
         .visit_proof_tree(
             obligation.clone().into(),
-            &mut BestObligation { obligation: obligation.clone() },
+            &mut BestObligation { obligation: obligation.clone(), consider_ambiguities },
         )
         .break_value()
         .unwrap_or(obligation)
@@ -288,6 +289,7 @@ fn find_best_leaf_obligation<'tcx>(
 
 struct BestObligation<'tcx> {
     obligation: PredicateObligation<'tcx>,
+    consider_ambiguities: bool,
 }
 
 impl<'tcx> BestObligation<'tcx> {
@@ -355,11 +357,11 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
                 }
             }
 
-            // Skip nested goals that hold.
-            //FIXME: We should change the max allowed certainty based on if we're
-            // visiting an ambiguity or error obligation.
-            if matches!(nested_goal.result(), Ok(Certainty::Yes)) {
-                continue;
+            // Skip nested goals that aren't the *reason* for our goal's failure.
+            match self.consider_ambiguities {
+                true if matches!(nested_goal.result(), Ok(Certainty::Maybe(_))) => {}
+                false if matches!(nested_goal.result(), Err(_)) => {}
+                _ => continue,
             }
 
             self.with_derived_obligation(obligation, |this| nested_goal.visit_with(this))?;
diff --git a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs
index bc9bb6ce2d79e..8bb4ff469076c 100644
--- a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs
+++ b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.rs
@@ -61,7 +61,7 @@ where
     // entering the cycle from `A` fails, but would work if we were to use the cache
     // result of `B<X>`.
     impls_trait::<A<X>, _, _, _>();
-    //~^ ERROR the trait bound `X: IncompleteGuidance<_, _>` is not satisfied
+    //~^ ERROR the trait bound `A<X>: Trait<i32, u8, u8>` is not satisfied
 }
 
 fn main() {
diff --git a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr
index 78116ebba8274..cdb4ff4588f75 100644
--- a/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr
+++ b/tests/ui/traits/next-solver/cycles/coinduction/incompleteness-unstable-result.stderr
@@ -1,26 +1,28 @@
-error[E0277]: the trait bound `X: IncompleteGuidance<_, _>` is not satisfied
+error[E0277]: the trait bound `A<X>: Trait<i32, u8, u8>` is not satisfied
   --> $DIR/incompleteness-unstable-result.rs:63:19
    |
 LL |     impls_trait::<A<X>, _, _, _>();
-   |                   ^^^^ the trait `IncompleteGuidance<_, _>` is not implemented for `X`, which is required by `A<X>: Trait<_, _, _>`
+   |                   ^^^^ the trait `Trait<i32, u8, u8>` is not implemented for `A<X>`, which is required by `A<X>: Trait<_, _, _>`
    |
-   = help: the following other types implement trait `IncompleteGuidance<T, V>`:
-             <T as IncompleteGuidance<U, i16>>
-             <T as IncompleteGuidance<U, i8>>
-             <T as IncompleteGuidance<U, u8>>
-note: required for `A<X>` to implement `Trait<_, _, u8>`
+note: required for `A<X>` to implement `Trait<i32, u8, u8>`
   --> $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 | where
-LL |     T: IncompleteGuidance<U, V>,
-   |        ------------------------ unsatisfied trait bound introduced here
+...
+LL |     A<T>: Trait<U, D, V>,
+   |           -------------- unsatisfied trait bound introduced here
+   = note: 8 redundant requirements hidden
+   = note: required for `A<X>` to implement `Trait<i32, u8, u8>`
 note: required by a bound in `impls_trait`
   --> $DIR/incompleteness-unstable-result.rs:51:28
    |
 LL | fn impls_trait<T: ?Sized + Trait<U, V, D>, U: ?Sized, V: ?Sized, D: ?Sized>() {}
    |                            ^^^^^^^^^^^^^^ required by this bound in `impls_trait`
+help: consider extending the `where` clause, but there might be an alternative better way to express this requirement
+   |
+LL |     X: IncompleteGuidance<u32, i16>, A<X>: Trait<i32, u8, u8>
+   |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: aborting due to 1 previous error