Rollup merge of #140711 - compiler-errors:combine-maybes, r=lcnr

Do not discard constraints on overflow if there was candidate ambiguity

Fixes rust-lang/trait-system-refactor-initiative#201.

There's a pretty chunky justification in the test.

r? lcnr

Author: matthiaskrgr

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs

@@ -132,12 +132,14 @@ where
                     (Certainty::Yes, NestedNormalizationGoals(goals))
                 }
                 _ => {
-                    let certainty = shallow_certainty.unify_with(goals_certainty);
+                    let certainty = shallow_certainty.and(goals_certainty);
                     (certainty, NestedNormalizationGoals::empty())
                 }
             };
 
-        if let Certainty::Maybe(cause @ MaybeCause::Overflow { .. }) = certainty {
+        if let Certainty::Maybe(cause @ MaybeCause::Overflow { keep_constraints: false, .. }) =
+            certainty
+        {
             // If we have overflow, it's probable that we're substituting a type
             // into itself infinitely and any partial substitutions in the query
             // response are probably not useful anyways, so just return an empty
@@ -193,6 +195,7 @@ where
                     debug!(?num_non_region_vars, "too many inference variables -> overflow");
                     return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow {
                         suggest_increasing_limit: true,
+                        keep_constraints: false,
                     }));
                 }
             }

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

@@ -661,7 +661,7 @@ where
                     Certainty::Yes => {}
                     Certainty::Maybe(_) => {
                         self.nested_goals.push((source, with_resolved_vars));
-                        unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
+                        unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
                     }
                 }
             } else {
@@ -675,7 +675,7 @@ where
                     Certainty::Yes => {}
                     Certainty::Maybe(_) => {
                         self.nested_goals.push((source, goal));
-                        unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
+                        unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
                     }
                 }
             }

compiler/rustc_next_trait_solver/src/solve/mod.rs

@@ -253,16 +253,18 @@ where
     }
 
     fn bail_with_ambiguity(&mut self, responses: &[CanonicalResponse<I>]) -> CanonicalResponse<I> {
-        debug_assert!(!responses.is_empty());
-        if let Certainty::Maybe(maybe_cause) =
-            responses.iter().fold(Certainty::AMBIGUOUS, |certainty, response| {
-                certainty.unify_with(response.value.certainty)
-            })
-        {
-            self.make_ambiguous_response_no_constraints(maybe_cause)
-        } else {
-            panic!("expected flounder response to be ambiguous")
-        }
+        debug_assert!(responses.len() > 1);
+        let maybe_cause = responses.iter().fold(MaybeCause::Ambiguity, |maybe_cause, response| {
+            // Pull down the certainty of `Certainty::Yes` to ambiguity when combining
+            // these responses, b/c we're combining more than one response and this we
+            // don't know which one applies.
+            let candidate = match response.value.certainty {
+                Certainty::Yes => MaybeCause::Ambiguity,
+                Certainty::Maybe(candidate) => candidate,
+            };
+            maybe_cause.or(candidate)
+        });
+        self.make_ambiguous_response_no_constraints(maybe_cause)
     }
 
     /// If we fail to merge responses we flounder and return overflow or ambiguity.

compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs

@@ -99,7 +99,13 @@ pub(super) fn fulfillment_error_for_stalled<'tcx>(
             Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => {
                 (FulfillmentErrorCode::Ambiguity { overflow: None }, true)
             }
-            Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => (
+            Ok((
+                _,
+                Certainty::Maybe(MaybeCause::Overflow {
+                    suggest_increasing_limit,
+                    keep_constraints: _,
+                }),
+            )) => (
                 FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) },
                 // Don't look into overflows because we treat overflows weirdly anyways.
                 // We discard the inference constraints from overflowing goals, so

compiler/rustc_trait_selection/src/solve/inspect/analyse.rs

@@ -382,7 +382,7 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
             if let Some(term_hack) = normalizes_to_term_hack {
                 infcx
                     .probe(|_| term_hack.constrain(infcx, DUMMY_SP, uncanonicalized_goal.param_env))
-                    .map(|certainty| ok.value.certainty.unify_with(certainty))
+                    .map(|certainty| ok.value.certainty.and(certainty))
             } else {
                 Ok(ok.value.certainty)
             }

compiler/rustc_type_ir/src/solve/mod.rs

@@ -273,17 +273,17 @@ impl Certainty {
     /// however matter for diagnostics. If `T: Foo` resulted in overflow and `T: Bar`
     /// in ambiguity without changing the inference state, we still want to tell the
     /// user that `T: Baz` results in overflow.
-    pub fn unify_with(self, other: Certainty) -> Certainty {
+    pub fn and(self, other: Certainty) -> Certainty {
         match (self, other) {
             (Certainty::Yes, Certainty::Yes) => Certainty::Yes,
             (Certainty::Yes, Certainty::Maybe(_)) => other,
             (Certainty::Maybe(_), Certainty::Yes) => self,
-            (Certainty::Maybe(a), Certainty::Maybe(b)) => Certainty::Maybe(a.unify_with(b)),
+            (Certainty::Maybe(a), Certainty::Maybe(b)) => Certainty::Maybe(a.and(b)),
         }
     }
 
     pub const fn overflow(suggest_increasing_limit: bool) -> Certainty {
-        Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit })
+        Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: false })
     }
 }
 
@@ -296,19 +296,58 @@ pub enum MaybeCause {
     /// or we hit a case where we just don't bother, e.g. `?x: Trait` goals.
     Ambiguity,
     /// We gave up due to an overflow, most often by hitting the recursion limit.
-    Overflow { suggest_increasing_limit: bool },
+    Overflow { suggest_increasing_limit: bool, keep_constraints: bool },
 }
 
 impl MaybeCause {
-    fn unify_with(self, other: MaybeCause) -> MaybeCause {
+    fn and(self, other: MaybeCause) -> MaybeCause {
         match (self, other) {
             (MaybeCause::Ambiguity, MaybeCause::Ambiguity) => MaybeCause::Ambiguity,
             (MaybeCause::Ambiguity, MaybeCause::Overflow { .. }) => other,
             (MaybeCause::Overflow { .. }, MaybeCause::Ambiguity) => self,
             (
-                MaybeCause::Overflow { suggest_increasing_limit: a },
-                MaybeCause::Overflow { suggest_increasing_limit: b },
-            ) => MaybeCause::Overflow { suggest_increasing_limit: a || b },
+                MaybeCause::Overflow {
+                    suggest_increasing_limit: limit_a,
+                    keep_constraints: keep_a,
+                },
+                MaybeCause::Overflow {
+                    suggest_increasing_limit: limit_b,
+                    keep_constraints: keep_b,
+                },
+            ) => MaybeCause::Overflow {
+                suggest_increasing_limit: limit_a && limit_b,
+                keep_constraints: keep_a && keep_b,
+            },
+        }
+    }
+
+    pub fn or(self, other: MaybeCause) -> MaybeCause {
+        match (self, other) {
+            (MaybeCause::Ambiguity, MaybeCause::Ambiguity) => MaybeCause::Ambiguity,
+
+            // When combining ambiguity + overflow, we can keep constraints.
+            (
+                MaybeCause::Ambiguity,
+                MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: _ },
+            ) => MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: true },
+            (
+                MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: _ },
+                MaybeCause::Ambiguity,
+            ) => MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: true },
+
+            (
+                MaybeCause::Overflow {
+                    suggest_increasing_limit: limit_a,
+                    keep_constraints: keep_a,
+                },
+                MaybeCause::Overflow {
+                    suggest_increasing_limit: limit_b,
+                    keep_constraints: keep_b,
+                },
+            ) => MaybeCause::Overflow {
+                suggest_increasing_limit: limit_a || limit_b,
+                keep_constraints: keep_a || keep_b,
+            },
         }
     }
 }

tests/ui/traits/next-solver/overflow-plus-ambiguity-normalizes-to-response.rs

@@ -0,0 +1,56 @@
+//@ check-pass
+//@ compile-flags: -Znext-solver
+
+// Regression test for <https://github.com/rust-lang/trait-system-refactor-initiative/issues/201>.
+// See comment below on `fn main`.
+
+trait Intersect<U> {
+    type Output;
+}
+
+impl<T, U> Intersect<Vec<U>> for T
+where
+    T: Intersect<U>,
+{
+    type Output = T;
+}
+
+impl Intersect<Cuboid> for Cuboid {
+    type Output = Cuboid;
+}
+
+fn intersect<T, U>(_: &T, _: &U) -> T::Output
+where
+    T: Intersect<U>,
+{
+    todo!()
+}
+
+struct Cuboid;
+impl Cuboid {
+    fn method(&self) {}
+}
+
+fn main() {
+    let x = vec![];
+    // Here we end up trying to normalize `<Cuboid as Intersect<Vec<?0>>>::Output`
+    // for the return type of the function, to constrain `y`. The impl then requires
+    // `Cuboid: Intersect<?0>`, which has two candidates. One that constrains
+    // `?0 = Vec<?1>`, which itself has the same two candidates and ends up leading
+    // to a recursion depth overflow. In the second impl, we constrain `?0 = Cuboid`.
+    //
+    // Floundering leads to us combining the overflow candidate and yes candidate to
+    // overflow. Because the response was overflow, we used to bubble it up to the
+    // parent normalizes-to goal, which caused us to drop its constraint that would
+    // guide us to normalize the associated type mentioned before.
+    //
+    // After this PR, we implement a new floundering "algebra" such that `Overflow OR Maybe`
+    // returns anew `Overflow { keep_constraints: true }`, which means that we don't
+    // need to drop constraints in the parent normalizes-to goal. This allows us to
+    // normalize `y` to `Cuboid`, and allows us to call the method successfully. We
+    // then constrain the `?0` in `let x: Vec<Cuboid> = x` below, so that we don't have
+    // a left over ambiguous goal.
+    let y = intersect(&Cuboid, &x);
+    y.method();
+    let x: Vec<Cuboid> = x;
+}