Auto merge of #157910 - jdonszelmann:stalled-goal-rerun, r=lcnr

Dont rerun stalled goals if erased runs succeeded*



\* Unless the succesful erased run was conditional on the state of opaque type storage, and the opaque type storage changed in a significant way 

r? @lcnr

Author: bors

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

@@ -35,8 +35,8 @@ use crate::solve::ty::may_use_unstable_feature;
 use crate::solve::{
     CanonicalInput, CanonicalResponse, Certainty, ExternalConstraintsData, FIXPOINT_STEP_LIMIT,
     Goal, GoalEvaluation, GoalSource, GoalStalledOn, HasChanged, MaybeCause,
-    NestedNormalizationGoals, NoSolution, QueryInput, QueryResult, Response, VisibleForLeakCheck,
-    inspect,
+    NestedNormalizationGoals, NoSolution, QueryInput, QueryResult, Response, SucceededInErased,
+    VisibleForLeakCheck, inspect,
 };
 
 mod probe;
@@ -284,6 +284,12 @@ where
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+enum RerunStalled {
+    WontMakeProgress(Certainty),
+    MayMakeProgress,
+}
+
 impl<'a, D, I> EvalCtxt<'a, D>
 where
     D: SolverDelegate<Interner = I>,
@@ -482,6 +488,81 @@ where
         Ok(goal_evaluation)
     }
 
+    /// This is a fast path optimization:
+    /// If we have run this goal before, and it was stalled, check that any of the goal's
+    /// args have changed. This is a cheap way to determine that if we were to rerun this goal now,
+    /// it will remain stalled since it'll canonicalize the same way and evaluation is pure.
+    /// Therefore, we can skip this rerun
+    fn rerunning_stalled_goal_may_make_progress(
+        &self,
+        stalled_on: Option<&GoalStalledOn<I>>,
+    ) -> RerunStalled {
+        use RerunStalled::*;
+
+        // If fast paths are turned off, then we assume all goals can always make progress
+        if self.delegate.disable_trait_solver_fast_paths() {
+            return MayMakeProgress;
+        }
+
+        // If the goal isn't stalled, we should definitely run it.
+        let Some(&GoalStalledOn {
+            num_opaques,
+            ref stalled_vars,
+            ref sub_roots,
+            stalled_certainty,
+            ref previously_succeeded_in_erased,
+        }) = stalled_on
+        else {
+            return MayMakeProgress;
+        };
+
+        // If any of the stalled goal's generic arguments changed,
+        // rerunning might make progress so we should rerun.
+        if stalled_vars.iter().any(|value| self.delegate.is_changed_arg(*value)) {
+            return MayMakeProgress;
+        }
+
+        // If some inference took place in any of the sub roots,
+        // rerunning might make progress so we should rerun.
+        if sub_roots.iter().any(|&vid| self.delegate.sub_unification_table_root_var(vid) != vid) {
+            return MayMakeProgress;
+        }
+
+        // If any opaques changed in the opaque type storage,
+        // rerunning might make progress so we should rerun.
+        if self.delegate.opaque_types_storage_num_entries().needs_reevaluation(num_opaques) {
+            // Unless this goal previously succeeded in erased mode.
+            // If the stalled goal successfully evaluated while erasing opaque types,
+            // and the current state of the opaque type storage is not different in a way that is
+            // relevant, this stalled goal cannot make any progress and we set this variable to true.
+            let mut previous_erased_run_is_still_valid = false;
+
+            if let &SucceededInErased::Yes { accessed_opaques } = previously_succeeded_in_erased {
+                match self.should_rerun_after_erased_canonicalization(
+                    accessed_opaques,
+                    self.typing_mode(),
+                    &self.delegate.clone_opaque_types_lookup_table(),
+                ) {
+                    RerunDecision::Yes => {}
+                    RerunDecision::EagerlyPropagateToParent => {
+                        unreachable!("we never retry stalled queries if the parent was erased")
+                    }
+                    RerunDecision::No => {
+                        previous_erased_run_is_still_valid = true;
+                    }
+                }
+            }
+
+            if !previous_erased_run_is_still_valid {
+                return MayMakeProgress;
+            }
+        }
+
+        // Otherwise, we can be sure that this stalled goal cannot make any progress
+        // and we can exit early.
+        WontMakeProgress(stalled_certainty)
+    }
+
     /// Recursively evaluates `goal`, returning the nested goals in case
     /// the nested goal is a `NormalizesTo` goal.
     ///
@@ -495,21 +576,8 @@ where
         goal: Goal<I, I::Predicate>,
         stalled_on: Option<GoalStalledOn<I>>,
     ) -> Result<(NestedNormalizationGoals<I>, GoalEvaluation<I>), NoSolutionOrRerunNonErased> {
-        // If we have run this goal before, and it was stalled, check that any of the goal's
-        // 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(GoalStalledOn {
-            num_opaques,
-            ref stalled_vars,
-            ref sub_roots,
-            stalled_certainty,
-        }) = stalled_on
-            && !self.delegate.disable_trait_solver_fast_paths()
-            && !stalled_vars.iter().any(|value| self.delegate.is_changed_arg(*value))
-            && !sub_roots
-                .iter()
-                .any(|&vid| self.delegate.sub_unification_table_root_var(vid) != vid)
-            && !self.delegate.opaque_types_storage_num_entries().needs_reevaluation(num_opaques)
+        if let RerunStalled::WontMakeProgress(stalled_certainty) =
+            self.rerunning_stalled_goal_may_make_progress(stalled_on.as_ref())
         {
             return Ok((
                 NestedNormalizationGoals::empty(),
@@ -539,7 +607,7 @@ where
         )
         .entered();
 
-        let (result, orig_values, canonical_goal) = 'retry_canonicalize: {
+        let (result, orig_values, canonical_goal, succeeded_in_erased) = 'retry_canonicalize: {
             let skip_erased_attempt = if typing_mode.is_coherence() {
                 true
             } else {
@@ -595,11 +663,23 @@ where
                 match should_rerun {
                     RerunDecision::Yes => debug!("rerunning in original typing mode"),
                     RerunDecision::No => {
-                        break 'retry_canonicalize (canonical_result, orig_values, canonical_goal);
+                        break 'retry_canonicalize (
+                            canonical_result,
+                            orig_values,
+                            canonical_goal,
+                            SucceededInErased::Yes { accessed_opaques },
+                        );
                     }
                     RerunDecision::EagerlyPropagateToParent => {
                         self.opaque_accesses.update(accessed_opaques)?;
-                        break 'retry_canonicalize (canonical_result, orig_values, canonical_goal);
+                        break 'retry_canonicalize (
+                            canonical_result,
+                            orig_values,
+                            canonical_goal,
+                            // If we're propagating up, we should never retry the goal.
+                            // That means `No` is fine to return, it doesn't really matter.
+                            SucceededInErased::No,
+                        );
                     }
                 }
             }
@@ -618,7 +698,7 @@ where
                 "we run without TypingMode::ErasedNotCoherence, so opaques are available, and we don't retry if the outer typing mode is ErasedNotCoherence: {accessed_opaques:?} after {goal:?}"
             );
 
-            (canonical_result, orig_values, canonical_goal)
+            (canonical_result, orig_values, canonical_goal, SucceededInErased::No)
         };
 
         debug!(?result);
@@ -714,6 +794,7 @@ where
                         stalled_vars,
                         sub_roots,
                         stalled_certainty: certainty,
+                        previously_succeeded_in_erased: succeeded_in_erased,
                     })
                 }
             },

compiler/rustc_next_trait_solver/src/solve/mod.rs

@@ -438,4 +438,11 @@ pub struct GoalStalledOn<I: Interner> {
     /// The certainty that will be returned on subsequent evaluations if this
     /// goal remains stalled.
     pub stalled_certainty: Certainty,
+    pub previously_succeeded_in_erased: SucceededInErased<I>,
+}
+
+#[derive_where(Clone, Debug; I: Interner)]
+pub enum SucceededInErased<I: Interner> {
+    Yes { accessed_opaques: AccessedOpaques<I> },
+    No,
 }