Auto merge of #144699 - compiler-errors:extract-if-fulfill, r=<try>

Use `ExtractIf` in fulfillment loop

Author: rust-bors[bot]

compiler/rustc_next_trait_solver/src/delegate.rs

@@ -1,8 +1,11 @@
 use std::ops::Deref;
 
+use rustc_type_ir::inherent::*;
 use rustc_type_ir::solve::{Certainty, Goal, NoSolution};
 use rustc_type_ir::{self as ty, InferCtxtLike, Interner, TypeFoldable};
 
+use crate::solve::GoalStalledOn;
+
 pub trait SolverDelegate: Deref<Target = Self::Infcx> + Sized {
     type Infcx: InferCtxtLike<Interner = Self::Interner>;
     type Interner: Interner;
@@ -23,6 +26,11 @@ pub trait SolverDelegate: Deref<Target = Self::Infcx> + Sized {
         span: <Self::Interner as Interner>::Span,
     ) -> Option<Certainty>;
 
+    fn is_still_stalled(&self, stalled_on: &GoalStalledOn<Self::Interner>) -> bool {
+        !stalled_on.stalled_vars.iter().any(|value| self.is_changed_arg(*value))
+            && !self.opaque_types_storage_num_entries().needs_reevaluation(stalled_on.num_opaques)
+    }
+
     fn fresh_var_for_kind_with_span(
         &self,
         arg: <Self::Interner as Interner>::GenericArg,

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

@@ -432,11 +432,7 @@ where
         // args have changed. Otherwise, we don't need to re-run the goal because it'll remain
         // stalled, since it'll canonicalize the same way and evaluation is pure.
         if let Some(stalled_on) = stalled_on
-            && !stalled_on.stalled_vars.iter().any(|value| self.delegate.is_changed_arg(*value))
-            && !self
-                .delegate
-                .opaque_types_storage_num_entries()
-                .needs_reevaluation(stalled_on.num_opaques)
+            && self.delegate.is_still_stalled(&stalled_on)
         {
             return Ok((
                 NestedNormalizationGoals::empty(),
@@ -639,15 +635,24 @@ where
     ///
     /// Goals for the next step get directly added to the nested goals of the `EvalCtxt`.
     fn evaluate_added_goals_step(&mut self) -> Result<Option<Certainty>, NoSolution> {
+        if self.nested_goals.is_empty() {
+            return Ok(Some(Certainty::Yes));
+        }
+
         let cx = self.cx();
         // If this loop did not result in any progress, what's our final certainty.
         let mut unchanged_certainty = Some(Certainty::Yes);
-        for (source, goal, stalled_on) in mem::take(&mut self.nested_goals) {
+
+        let mut nested_goals = mem::take(&mut self.nested_goals);
+        let mut pending_goals = vec![];
+        for (source, goal, stalled_on) in nested_goals.extract_if(.., |(_, _, stalled_on)| {
+            stalled_on.as_ref().is_none_or(|s| !self.delegate.is_still_stalled(s))
+        }) {
             if let Some(certainty) = self.delegate.compute_goal_fast_path(goal, self.origin_span) {
                 match certainty {
                     Certainty::Yes => {}
                     Certainty::Maybe(_) => {
-                        self.nested_goals.push((source, goal, None));
+                        pending_goals.push((source, goal, None));
                         unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
                     }
                 }
@@ -684,7 +689,7 @@ where
                 )?;
                 // Add the nested goals from normalization to our own nested goals.
                 trace!(?nested_goals);
-                self.nested_goals.extend(nested_goals.into_iter().map(|(s, g)| (s, g, None)));
+                pending_goals.extend(nested_goals.into_iter().map(|(s, g)| (s, g, None)));
 
                 // Finally, equate the goal's RHS with the unconstrained var.
                 //
@@ -728,7 +733,7 @@ where
                 match certainty {
                     Certainty::Yes => {}
                     Certainty::Maybe(_) => {
-                        self.nested_goals.push((source, with_resolved_vars, stalled_on));
+                        pending_goals.push((source, with_resolved_vars, stalled_on));
                         unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
                     }
                 }
@@ -742,13 +747,25 @@ where
                 match certainty {
                     Certainty::Yes => {}
                     Certainty::Maybe(_) => {
-                        self.nested_goals.push((source, goal, stalled_on));
+                        pending_goals.push((source, goal, stalled_on));
                         unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
                     }
                 }
             }
         }
 
+        // Nested goals still need to be accounted for in the `unchanged_certainty`.
+        for (_, _, stalled_on) in &nested_goals {
+            if let Some(GoalStalledOn { stalled_cause, .. }) = stalled_on {
+                unchanged_certainty =
+                    unchanged_certainty.map(|c| c.and(Certainty::Maybe(*stalled_cause)));
+            }
+        }
+
+        debug_assert!(self.nested_goals.is_empty());
+        nested_goals.extend(pending_goals);
+        self.nested_goals = nested_goals;
+
         Ok(unchanged_certainty)
     }
 

compiler/rustc_trait_selection/src/solve/fulfill.rs

@@ -1,5 +1,4 @@
 use std::marker::PhantomData;
-use std::mem;
 use std::ops::ControlFlow;
 
 use rustc_data_structures::thinvec::ExtractIf;
@@ -75,6 +74,13 @@ impl<'tcx> ObligationStorage<'tcx> {
         self.pending.push((obligation, stalled_on));
     }
 
+    fn register_overflowed(
+        &mut self,
+        overflowed: impl IntoIterator<Item = PredicateObligation<'tcx>>,
+    ) {
+        self.overflowed.extend(overflowed);
+    }
+
     fn has_pending_obligations(&self) -> bool {
         !self.pending.is_empty() || !self.overflowed.is_empty()
     }
@@ -88,35 +94,14 @@ impl<'tcx> ObligationStorage<'tcx> {
 
     fn drain_pending(
         &mut self,
-        cond: impl Fn(&PredicateObligation<'tcx>) -> bool,
-    ) -> PendingObligations<'tcx> {
-        let (unstalled, pending) =
-            mem::take(&mut self.pending).into_iter().partition(|(o, _)| cond(o));
-        self.pending = pending;
-        unstalled
+        cond: impl Fn(&PredicateObligation<'tcx>, Option<&GoalStalledOn<TyCtxt<'tcx>>>) -> bool,
+    ) -> impl Iterator<Item = (PredicateObligation<'tcx>, Option<GoalStalledOn<TyCtxt<'tcx>>>)>
+    {
+        ExtractIf::new(&mut self.pending, move |(o, stalled)| cond(o, stalled.as_ref()))
     }
 
-    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 `select_where_possible`, which goals would
-            // change.
-            // FIXME: <https://github.com/Gankra/thin-vec/pull/66> is merged, this can be removed.
-            self.overflowed.extend(
-                ExtractIf::new(&mut self.pending, |(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),
-            );
-        })
+    fn num_pending(&self) -> usize {
+        self.pending.len()
     }
 }
 
@@ -133,21 +118,6 @@ impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> {
             _errors: PhantomData,
         }
     }
-
-    fn inspect_evaluated_obligation(
-        &self,
-        infcx: &InferCtxt<'tcx>,
-        obligation: &PredicateObligation<'tcx>,
-        result: &Result<GoalEvaluation<TyCtxt<'tcx>>, NoSolution>,
-    ) {
-        if let Some(inspector) = infcx.obligation_inspector.get() {
-            let result = match result {
-                Ok(GoalEvaluation { certainty, .. }) => Ok(*certainty),
-                Err(NoSolution) => Err(NoSolution),
-            };
-            (inspector)(infcx, &obligation, result);
-        }
-    }
 }
 
 impl<'tcx, E> TraitEngine<'tcx, E> for FulfillmentCtxt<'tcx, E>
@@ -180,19 +150,27 @@ where
     }
 
     fn select_where_possible(&mut self, infcx: &InferCtxt<'tcx>) -> Vec<E> {
+        if self.obligations.num_pending() == 0 {
+            return vec![];
+        }
+
         assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots());
+        let delegate = <&SolverDelegate<'tcx>>::from(infcx);
         let mut errors = Vec::new();
         loop {
             let mut any_changed = false;
-            for (mut obligation, stalled_on) in self.obligations.drain_pending(|_| true) {
+            let mut overflowed = vec![];
+            let mut pending = vec![];
+
+            for (mut obligation, stalled_on) in self.obligations.drain_pending(|_, stalled_on| {
+                stalled_on.is_none_or(|s| !delegate.is_still_stalled(s))
+            }) {
                 if !infcx.tcx.recursion_limit().value_within_limit(obligation.recursion_depth) {
-                    self.obligations.on_fulfillment_overflow(infcx);
-                    // Only return true errors that we have accumulated while processing.
-                    return errors;
+                    overflowed.push(obligation);
+                    continue;
                 }
 
                 let goal = obligation.as_goal();
-                let delegate = <&SolverDelegate<'tcx>>::from(infcx);
                 if let Some(certainty) =
                     delegate.compute_goal_fast_path(goal, obligation.cause.span)
                 {
@@ -204,15 +182,21 @@ where
                         //
                         // Only goals proven via the trait solver should be region dependent.
                         Certainty::Yes => {}
-                        Certainty::Maybe(_) => {
-                            self.obligations.register(obligation, None);
-                        }
+                        Certainty::Maybe(_) => pending.push((obligation, None)),
                     }
                     continue;
                 }
 
                 let result = delegate.evaluate_root_goal(goal, obligation.cause.span, stalled_on);
-                self.inspect_evaluated_obligation(infcx, &obligation, &result);
+
+                if let Some(inspector) = infcx.obligation_inspector.get() {
+                    let result = match result {
+                        Ok(GoalEvaluation { certainty, .. }) => Ok(certainty),
+                        Err(NoSolution) => Err(NoSolution),
+                    };
+                    (inspector)(infcx, &obligation, result);
+                }
+
                 let GoalEvaluation { goal, certainty, has_changed, stalled_on } = match result {
                     Ok(result) => result,
                     Err(NoSolution) => {
@@ -256,10 +240,19 @@ where
                             infcx.push_hir_typeck_potentially_region_dependent_goal(obligation);
                         }
                     }
-                    Certainty::Maybe(_) => self.obligations.register(obligation, stalled_on),
+                    Certainty::Maybe(_) => pending.push((obligation, stalled_on)),
                 }
             }
 
+            if !overflowed.is_empty() {
+                self.obligations.register_overflowed(overflowed);
+                return errors;
+            }
+
+            for (obligation, stalled_on) in pending {
+                self.obligations.register(obligation, stalled_on);
+            }
+
             if !any_changed {
                 break;
             }
@@ -295,7 +288,7 @@ where
         }
 
         self.obligations
-            .drain_pending(|obl| {
+            .drain_pending(|obl, _| {
                 infcx.probe(|_| {
                     infcx
                         .visit_proof_tree(

tests/ui/impl-trait/recursive-in-exhaustiveness.next.stderr

@@ -5,16 +5,16 @@ LL |     build(x)
    |     ^^^^^^^^ cannot normalize `build<_>::{opaque#0}`
 
 error[E0271]: type mismatch resolving `build2<(_,)>::{opaque#0} normalizes-to _`
-  --> $DIR/recursive-in-exhaustiveness.rs:30:6
+  --> $DIR/recursive-in-exhaustiveness.rs:30:5
    |
 LL |     (build2(x),)
-   |      ^^^^^^^^^ types differ
+   |     ^^^^^^^^^^^^ types differ
 
 error[E0271]: type mismatch resolving `build2<(_,)>::{opaque#0} normalizes-to _`
-  --> $DIR/recursive-in-exhaustiveness.rs:30:5
+  --> $DIR/recursive-in-exhaustiveness.rs:30:6
    |
 LL |     (build2(x),)
-   |     ^^^^^^^^^^^^ types differ
+   |      ^^^^^^^^^ types differ
 
 error[E0277]: the size for values of type `(impl Sized,)` cannot be known at compilation time
   --> $DIR/recursive-in-exhaustiveness.rs:30:5

tests/ui/impl-trait/two_tait_defining_each_other2.next.stderr

@@ -2,7 +2,7 @@ error[E0282]: type annotations needed
   --> $DIR/two_tait_defining_each_other2.rs:12:11
    |
 LL | fn muh(x: A) -> B {
-   |           ^ cannot infer type
+   |           ^ cannot infer type for type alias `A`
 
 error: aborting due to 1 previous error
 

tests/ui/traits/next-solver/alias-bound-unsound.rs

@@ -25,6 +25,7 @@ fn main() {
     //~^ ERROR overflow evaluating the requirement `String <: <() as Foo>::Item`
     //~| ERROR overflow evaluating the requirement `<() as Foo>::Item well-formed`
     //~| ERROR overflow evaluating the requirement `&<() as Foo>::Item well-formed`
+    //~| ERROR overflow evaluating the requirement `<() as Foo>::Item: Sized`
     //~| ERROR overflow evaluating the requirement `<() as Foo>::Item == _`
     //~| ERROR overflow evaluating the requirement `<() as Foo>::Item == _`
     //~| ERROR overflow evaluating the requirement `<() as Foo>::Item == _`

tests/ui/traits/next-solver/alias-bound-unsound.stderr

@@ -32,6 +32,14 @@ LL |     drop(<() as Foo>::copy_me(&x));
    |
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
+error[E0275]: overflow evaluating the requirement `<() as Foo>::Item: Sized`
+  --> $DIR/alias-bound-unsound.rs:24:10
+   |
+LL |     drop(<() as Foo>::copy_me(&x));
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the return type of a function must have a statically known size
+
 error[E0275]: overflow evaluating the requirement `&<() as Foo>::Item well-formed`
   --> $DIR/alias-bound-unsound.rs:24:31
    |
@@ -58,6 +66,6 @@ error[E0275]: overflow evaluating the requirement `<() as Foo>::Item == _`
 LL |     drop(<() as Foo>::copy_me(&x));
    |                               ^^
 
-error: aborting due to 8 previous errors
+error: aborting due to 9 previous errors
 
 For more information about this error, try `rustc --explain E0275`.

tests/ui/traits/next-solver/assembly/runaway-impl-candidate-selection.stderr

@@ -1,16 +1,9 @@
-error[E0283]: type annotations needed
+error[E0284]: type annotations needed: cannot normalize `<_ as Iterator>::Item`
   --> $DIR/runaway-impl-candidate-selection.rs:13:22
    |
 LL |     println!("{:?}", iter::<_>());
-   |                      ^^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `iter`
-   |
-   = note: cannot satisfy `_: Iterator`
-note: required by a bound in `iter`
-  --> $DIR/runaway-impl-candidate-selection.rs:8:12
-   |
-LL | fn iter<T: Iterator>() -> <T as Iterator>::Item {
-   |            ^^^^^^^^ required by this bound in `iter`
+   |                      ^^^^^^^^^^^ cannot normalize `<_ as Iterator>::Item`
 
 error: aborting due to 1 previous error
 
-For more information about this error, try `rustc --explain E0283`.
+For more information about this error, try `rustc --explain E0284`.

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/coroutine.fail.stderr

@@ -11,10 +11,10 @@ LL | |         },
    | |_________^ the trait `Coroutine<A>` is not implemented for `{coroutine@$DIR/coroutine.rs:20:9: 20:11}`
    |
 note: required by a bound in `needs_coroutine`
-  --> $DIR/coroutine.rs:14:28
+  --> $DIR/coroutine.rs:14:41
    |
 LL | fn needs_coroutine(_: impl Coroutine<A, Yield = B, Return = C>) {}
-   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `needs_coroutine`
+   |                                         ^^^^^^^^^ required by this bound in `needs_coroutine`
 
 error: aborting due to 1 previous error