From ce0ebd87ecdc526abafb1d66261c6ac80991bf6f Mon Sep 17 00:00:00 2001
From: lcnr <rust@lcnr.de>
Date: Thu, 9 May 2024 17:11:51 +0000
Subject: [PATCH] analyse visitor: build proof tree in probe

---
 .../src/solve/eval_ctxt/canonical.rs          |  1 +
 .../src/solve/inspect/analyse.rs              | 44 ++++++++++++++-----
 .../ambiguity-causes-canonical-state-ice.rs   | 44 +++++++++++++++++++
 3 files changed, 78 insertions(+), 11 deletions(-)
 create mode 100644 tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice.rs

diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
index 5f73432750d10..41b304ec53852 100644
--- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
+++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
@@ -409,6 +409,7 @@ pub(in crate::solve) fn make_canonical_state<'tcx, T: TypeFoldable<TyCtxt<'tcx>>
 /// This currently assumes that unifying the var values trivially succeeds.
 /// Adding any inference constraints which weren't present when originally
 /// computing the canonical query can result in bugs.
+#[instrument(level = "debug", skip(infcx, span, param_env))]
 pub(in crate::solve) fn instantiate_canonical_state<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
     infcx: &InferCtxt<'tcx>,
     span: Span,
diff --git a/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs b/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs
index fa4323a3a944d..71ae410a03f98 100644
--- a/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs
+++ b/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs
@@ -146,6 +146,11 @@ impl<'a, 'tcx> InspectCandidate<'a, 'tcx> {
     /// inference constraints, and optionally the args of an impl if this candidate
     /// came from a `CandidateSource::Impl`. This function modifies the state of the
     /// `infcx`.
+    #[instrument(
+        level = "debug",
+        skip_all,
+        fields(goal = ?self.goal.goal, nested_goals = ?self.nested_goals)
+    )]
     pub fn instantiate_nested_goals_and_opt_impl_args(
         &self,
         span: Span,
@@ -213,10 +218,23 @@ impl<'a, 'tcx> InspectCandidate<'a, 'tcx> {
                     };
                     let goal =
                         goal.with(infcx.tcx, ty::NormalizesTo { alias, term: unconstrained_term });
-                    let proof_tree = EvalCtxt::enter_root(infcx, GenerateProofTree::Yes, |ecx| {
-                        ecx.evaluate_goal_raw(GoalEvaluationKind::Root, GoalSource::Misc, goal)
-                    })
-                    .1;
+                    // We have to use a `probe` here as evaluating a `NormalizesTo` can constrain the
+                    // expected term. This means that candidates which only fail due to nested goals
+                    // and which normalize to a different term then the final result could ICE: when
+                    // building their proof tree, the expected term was unconstrained, but when
+                    // instantiating the candidate it is already constrained to the result of another
+                    // candidate.
+                    let proof_tree = infcx
+                        .probe(|_| {
+                            EvalCtxt::enter_root(infcx, GenerateProofTree::Yes, |ecx| {
+                                ecx.evaluate_goal_raw(
+                                    GoalEvaluationKind::Root,
+                                    GoalSource::Misc,
+                                    goal,
+                                )
+                            })
+                        })
+                        .1;
                     InspectGoal::new(
                         infcx,
                         self.goal.depth + 1,
@@ -225,13 +243,17 @@ impl<'a, 'tcx> InspectCandidate<'a, 'tcx> {
                         source,
                     )
                 }
-                _ => InspectGoal::new(
-                    infcx,
-                    self.goal.depth + 1,
-                    infcx.evaluate_root_goal(goal, GenerateProofTree::Yes).1.unwrap(),
-                    None,
-                    source,
-                ),
+                _ => {
+                    // We're using a probe here as evaluating a goal could constrain
+                    // inference variables by choosing one candidate. If we then recurse
+                    // into another candidate who ends up with different inference
+                    // constraints, we get an ICE if we already applied the constraints
+                    // from the chosen candidate.
+                    let proof_tree = infcx
+                        .probe(|_| infcx.evaluate_root_goal(goal, GenerateProofTree::Yes).1)
+                        .unwrap();
+                    InspectGoal::new(infcx, self.goal.depth + 1, proof_tree, None, source)
+                }
             })
             .collect();
 
diff --git a/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice.rs b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice.rs
new file mode 100644
index 0000000000000..2e82f081d8669
--- /dev/null
+++ b/tests/ui/traits/next-solver/coherence/ambiguity-causes-canonical-state-ice.rs
@@ -0,0 +1,44 @@
+//@ compile-flags: -Znext-solver=coherence
+//@ check-pass
+#![crate_type = "lib"]
+#![feature(min_specialization)]
+
+// A regression test for #124791. Computing ambiguity causes
+// for the overlap of the `ToString` impls caused an ICE.
+
+trait Display {}
+
+trait ToOwned {
+    type Owned;
+}
+
+impl<T> ToOwned for T {
+    type Owned = T;
+}
+
+struct Cow<B: ?Sized>(B);
+
+impl<B: ?Sized> Display for Cow<B>
+where
+    B: ToOwned,
+    B::Owned: Display,
+{
+}
+
+impl Display for () {}
+
+trait ToString {
+    fn to_string();
+}
+
+impl<T: Display + ?Sized> ToString for T {
+    default fn to_string() {}
+}
+
+impl ToString for Cow<str> {
+    fn to_string() {}
+}
+
+impl ToOwned for str {
+    type Owned = ();
+}