-Znext-solver : Abort on the first overflowed obligation in fulfillment

compiler/rustc_trait_selection/src/solve/fulfill.rs

@@ -17,6 +17,7 @@ use tracing::instrument;
 use self::derive_errors::*;
 use super::Certainty;
 use super::delegate::SolverDelegate;
+use crate::error_reporting::InferCtxtErrorExt;
 use crate::traits::{FulfillmentError, FulfillmentErrorCode, ScrubbedTraitError};
 
 mod derive_errors;
@@ -53,12 +54,6 @@ pub struct FulfillmentCtxt<'tcx, E: 'tcx> {
 
 #[derive(Default, Debug)]
 struct ObligationStorage<'tcx> {
-    /// Obligations which resulted in an overflow in fulfillment itself.
-    ///
-    /// We cannot eagerly return these as error so we instead store them here
-    /// to avoid recomputing them each time `try_evaluate_obligations` is called.
-    /// This also allows us to return the correct `FulfillmentError` for them.
-    overflowed: Vec<PredicateObligation<'tcx>>,
     pending: PendingObligations<'tcx>,
 }
 
@@ -72,24 +67,18 @@ impl<'tcx> ObligationStorage<'tcx> {
     }
 
     fn has_pending_obligations(&self) -> bool {
-        !self.pending.is_empty() || !self.overflowed.is_empty()
+        !self.pending.is_empty()
     }
 
     fn clone_pending(&self) -> PredicateObligations<'tcx> {
-        let mut obligations: PredicateObligations<'tcx> =
-            self.pending.iter().map(|(o, _)| o.clone()).collect();
-        obligations.extend(self.overflowed.iter().cloned());
-        obligations
+        self.pending.iter().map(|(o, _)| o.clone()).collect()
     }
 
     fn clone_pending_filtered<F>(&self, f: F) -> PredicateObligations<'tcx>
     where
         F: FnMut(&&(PredicateObligation<'tcx>, Option<GoalStalledOn<TyCtxt<'tcx>>>)) -> bool,
     {
-        let mut obligations: PredicateObligations<'tcx> =
-            self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect();
-        obligations.extend(self.overflowed.iter().cloned());
-        obligations
+        self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect()
     }
 
     fn drain_pending(
@@ -101,29 +90,6 @@ impl<'tcx> ObligationStorage<'tcx> {
         self.pending = pending;
         unstalled
     }
-
-    fn on_fulfillment_overflow(&mut self, infcx: &InferCtxt<'tcx>) {
-        infcx.probe(|_| {
-            // IMPORTANT: we must not use solve any inference variables in the obligations
-            // as this is all happening inside of a probe. We use a probe to make sure
-            // we get all obligations involved in the overflow. We pretty much check: if
-            // we were to do another step of `try_evaluate_obligations`, which goals would
-            // change.
-            self.overflowed.extend(
-                self.pending
-                    .extract_if(.., |(o, stalled_on)| {
-                        let goal = o.as_goal();
-                        let result = <&SolverDelegate<'tcx>>::from(infcx).evaluate_root_goal(
-                            goal,
-                            o.cause.span,
-                            stalled_on.take(),
-                        );
-                        matches!(result, Ok(GoalEvaluation { has_changed: HasChanged::Yes, .. }))
-                    })
-                    .map(|(o, _)| o),
-            );
-        })
-    }
 }
 
 impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> {
@@ -186,7 +152,7 @@ where
 
     #[inline]
     fn collect_remaining_errors(&mut self, infcx: &InferCtxt<'tcx>) -> TraitErrors<E> {
-        if self.obligations.pending.is_empty() && self.obligations.overflowed.is_empty() {
+        if self.obligations.pending.is_empty() {
             // Typically in more than 99.9% of cases this condition is true, therefore we outline
             // the other case.
             TraitErrors::NoErrors
@@ -202,13 +168,8 @@ where
         let delegate = <&SolverDelegate<'tcx>>::from(infcx);
         loop {
             let mut any_changed = false;
-            let mut overflowed = false;
 
             self.obligations.pending.retain_mut(|(obligation, opt_stalled_on)| {
-                if overflowed {
-                    return false;
-                }
-
                 // Common case: still stalled; keep the obligation. This path is extremely hot in
                 // some cases; there can be thousands of pending obligations.
                 if let Some(stalled_on) = opt_stalled_on
@@ -239,21 +200,26 @@ where
                 // constrained by evaluating the goal.
                 obligation.predicate = goal.predicate;
                 if has_changed == HasChanged::Yes {
-                    // We increment the recursion depth here to track the number of times
-                    // this goal has resulted in inference progress. This doesn't precisely
-                    // model the way that we track recursion depth in the old solver due
-                    // to the fact that we only process root obligations, but it is a good
-                    // approximation and should only result in fulfillment overflow in
-                    // pathological cases.
-                    obligation.recursion_depth += 1;
-
                     if !infcx.tcx.recursion_limit().value_within_limit(obligation.recursion_depth) {
-                        // At this point we want to stop evaluating goals. We can't break out of
-                        // `retain_mut`, so instead we set this flag which causes all other
-                        // elements to be skipped.
-                        overflowed = true;
-                        return false;
+                        // We limit the total count of inference progress to avoid hang so we don't
+                        // try to recover from this.
+                        // It's more complicated to collect all overflows thus we stopped doing that.
+                        // Eager aborting is also what the old solver does.
+                        //
+                        // Note: it's incredibly rare to actually encounter fulfillment overflow
+                        // as a single obligation would have to result in different inference progress
+                        // a `recursion_depth` number of times. This mostly happens in bugs or with
+                        // `Subtype` obligations because we no longer use the `sub_unification_table`
+                        // in generalization.
+                        infcx.err_ctxt().report_overflow_obligation(obligation, true);
                     } else {
+                        // We increment the recursion depth here to track the number of times
+                        // this goal has resulted in inference progress. This doesn't precisely
+                        // model the way that we track recursion depth in the old solver due
+                        // to the fact that we only process root obligations, but it is a good
+                        // approximation and should only result in fulfillment overflow in
+                        // pathological cases.
+                        obligation.recursion_depth += 1;
                         any_changed = true;
                     }
                 }
@@ -288,11 +254,6 @@ where
                     }
                 }
             });
-            if overflowed {
-                self.obligations.on_fulfillment_overflow(infcx);
-                // Only return true errors that we have accumulated while processing.
-                return errors;
-            }
 
             if !any_changed {
                 break;
@@ -409,12 +370,6 @@ where
         .filter_map(|(obligation, _)| {
             try_ambiguity_error_for_stalled(infcx, obligation).map(NextSolverError::Ambiguity)
         })
-        .chain(
-            cx.obligations
-                .overflowed
-                .drain(..)
-                .map(|obligation| NextSolverError::Overflow(obligation)),
-        )
         .map(|e| E::from_solver_error(infcx, e))
         .collect()
 }
@@ -432,7 +387,6 @@ pub struct NextSolverAmbiguityError<'tcx> {
 pub enum NextSolverError<'tcx> {
     TrueError(PredicateObligation<'tcx>),
     Ambiguity(NextSolverAmbiguityError<'tcx>),
-    Overflow(PredicateObligation<'tcx>),
 }
 
 impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for FulfillmentError<'tcx> {
@@ -444,9 +398,6 @@ impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for FulfillmentError<'tc
             NextSolverError::Ambiguity(ambiguity) => {
                 fulfillment_error_for_stalled(infcx, ambiguity)
             }
-            NextSolverError::Overflow(obligation) => {
-                fulfillment_error_for_overflow(infcx, obligation)
-            }
         }
     }
 }
@@ -455,9 +406,7 @@ impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for ScrubbedTraitError<'
     fn from_solver_error(_infcx: &InferCtxt<'tcx>, error: NextSolverError<'tcx>) -> Self {
         match error {
             NextSolverError::TrueError(_) => ScrubbedTraitError::TrueError,
-            NextSolverError::Ambiguity(_) | NextSolverError::Overflow(_) => {
-                ScrubbedTraitError::Ambiguity
-            }
+            NextSolverError::Ambiguity(_) => ScrubbedTraitError::Ambiguity,
         }
     }
 }

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

@@ -161,17 +161,6 @@ pub(super) fn try_ambiguity_error_for_stalled<'tcx>(
     Some(NextSolverAmbiguityError { root_obligation, code, refine_obligation })
 }
 
-pub(super) fn fulfillment_error_for_overflow<'tcx>(
-    infcx: &InferCtxt<'tcx>,
-    root_obligation: PredicateObligation<'tcx>,
-) -> FulfillmentError<'tcx> {
-    FulfillmentError {
-        obligation: find_best_leaf_obligation(infcx, &root_obligation, true),
-        code: FulfillmentErrorCode::Ambiguity { overflow: Some(true) },
-        root_obligation,
-    }
-}
-
 #[instrument(level = "debug", skip(infcx), ret)]
 fn find_best_leaf_obligation<'tcx>(
     infcx: &InferCtxt<'tcx>,

tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr

@@ -5,6 +5,9 @@ LL | impl<T: TwoW> Trait for W<T> {}
    | ---------------------------- first implementation here
 LL | impl<T: TwoW> Trait for T {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>>>`
+   |
+   = note: overflow evaluating the requirement `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>>>: TwoW`
+   = help: consider increasing the recursion limit by adding a `#![recursion_limit = "20"]` attribute to your crate (`coherence_fulfill_overflow`)
 
 error: aborting due to 1 previous error
 

tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.rs

@@ -0,0 +1,53 @@
+//@ compile-flags: -Znext-solver
+
+// Regression test for trait-system-refactor-initiative#294
+// We used to drop all subsequent obligations when one obligation overflows
+// in fulfillment. It means we don't really prove all obligations even if
+// fulfillment returns no error.
+//
+// We now eagerly abort on the first overflowed obligation.
+
+#![feature(impl_trait_in_assoc_type)]
+#![forbid(unsafe_code)]
+
+trait Amb<'z> {}
+trait Sub<'c, 'd>: Amb<'c> + Amb<'d> {}
+impl<'z> Amb<'z> for i32 {}
+impl<'c, 'd> Sub<'c, 'd> for i32 {}
+
+trait Call<'a> {
+    type Output;
+    fn call() -> Self::Output;
+}
+
+trait Leak<G> {
+    fn leak(self) -> &'static u8;
+}
+impl<'z, G: Call<'static, Output = R>, R: Amb<'z>> Leak<G> for &'static u8 {
+    fn leak(self) -> &'static u8 {
+        self
+    }
+}
+
+#[expect(dead_code)]
+struct Foo<'c, 'd>(&'c (), &'d ());
+
+impl<'a, 'c, 'd> Call<'a> for Foo<'c, 'd>
+where
+    i32: Sub<'c, 'd>,
+{
+    type Output = impl Sized + use<>;
+    fn call() -> Self::Output {
+        let r = {
+            let local = 42_u8;
+            <&u8 as Leak<Foo<'c, 'd>>>::leak(&local)
+            //~^ ERROR: overflow evaluating the requirement `&u8: Leak<Foo<'c, 'd>>`
+        };
+        println!("{r}"); // use-after-free
+        1_i32
+    }
+}
+
+fn main() {
+    Foo::call();
+}

tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.stderr

@@ -0,0 +1,11 @@
+error[E0275]: overflow evaluating the requirement `&u8: Leak<Foo<'c, 'd>>`
+  --> $DIR/dont-drop-obligations-on-overflow-1.rs:43:13
+   |
+LL |             <&u8 as Leak<Foo<'c, 'd>>>::leak(&local)
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`dont_drop_obligations_on_overflow_1`)
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0275`.

tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.rs

@@ -0,0 +1,27 @@
+//@ compile-flags: -Znext-solver
+
+// Regression test for trait-system-refactor-initiative#242
+// We used to drop all subsequent obligations when one obligation overflows
+// in fulfillment. It means we don't really prove all obligations even if
+// fulfillment returns no error.
+//
+// We now eagerly abort on the first overflowed obligation.
+//
+// FIXME: this probably should compile and we shall fix duplicate
+// uses of opaques.
+
+#![feature(type_alias_impl_trait)]
+type Tait<'a> = impl Sized;
+
+fn prove()
+where
+    for<'a> Tait<'a>: Sized,
+{}
+
+#[define_opaque(Tait)]
+fn foo<'a>() -> &'a Tait<'a> {
+    prove();
+    //~^ ERROR: overflow evaluating the requirement `for<'a> Tait<'a>: Sized`
+    &()
+}
+fn main() {}

tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.stderr

@@ -0,0 +1,19 @@
+error[E0275]: overflow evaluating the requirement `for<'a> Tait<'a>: Sized`
+  --> $DIR/dont-drop-obligations-on-overflow-2.rs:23:5
+   |
+LL |     prove();
+   |     ^^^^^^^
+   |
+   = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`dont_drop_obligations_on_overflow_2`)
+note: required by a bound in `prove`
+  --> $DIR/dont-drop-obligations-on-overflow-2.rs:18:23
+   |
+LL | fn prove()
+   |    ----- required by a bound in this function
+LL | where
+LL |     for<'a> Tait<'a>: Sized,
+   |                       ^^^^^ required by this bound in `prove`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0275`.