Handle stalling within ObligationForest.

It is simpler if `ObligationForest` does this itself, rather than the
caller having to manage it.

compiler/rustc_data_structures/src/obligation_forest/mod.rs

@@ -42,7 +42,7 @@
 //!   now considered to be in error.
 //!
 //! When the call to `process_obligations` completes, you get back an `Outcome`,
-//! which includes three bits of information:
+//! which includes two bits of information:
 //!
 //! - `completed`: a list of obligations where processing was fully
 //!   completed without error (meaning that all transitive subobligations
@@ -53,13 +53,10 @@
 //!   all the obligations in `C` have been found completed.
 //! - `errors`: a list of errors that occurred and associated backtraces
 //!   at the time of error, which can be used to give context to the user.
-//! - `stalled`: if true, then none of the existing obligations were
+//!
-//!   *shallowly successful* (that is, no callback returned `Changed(_)`).
+//! Upon completion, none of the existing obligations were *shallowly
-//!   This implies that all obligations were either errors or returned an
+//! successful* (that is, no callback returned `Changed(_)`). This implies that
-//!   ambiguous result, which means that any further calls to
+//! all obligations were either errors or returned an ambiguous result.
-//!   `process_obligations` would simply yield back further ambiguous
-//!   results. This is used by the `FulfillmentContext` to decide when it
-//!   has reached a steady state.
 //!
 //! ### Implementation details
 //!
@@ -260,8 +257,6 @@ pub trait OutcomeTrait {
     type Obligation;
 
     fn new() -> Self;
-    fn mark_not_stalled(&mut self);
-    fn is_stalled(&self) -> bool;
     fn record_completed(&mut self, outcome: &Self::Obligation);
     fn record_error(&mut self, error: Self::Error);
 }
@@ -270,14 +265,6 @@ pub trait OutcomeTrait {
 pub struct Outcome<O, E> {
     /// Backtrace of obligations that were found to be in error.
     pub errors: Vec<Error<O, E>>,
-
-    /// If true, then we saw no successful obligations, which means
-    /// there is no point in further iteration. This is based on the
-    /// assumption that when trait matching returns `Error` or
-    /// `Unchanged`, those results do not affect environmental
-    /// inference state. (Note that if we invoke `process_obligations`
-    /// with no pending obligations, stalled will be true.)
-    pub stalled: bool,
 }
 
 impl<O, E> OutcomeTrait for Outcome<O, E> {
@@ -285,15 +272,7 @@ impl<O, E> OutcomeTrait for Outcome<O, E> {
     type Obligation = O;
 
     fn new() -> Self {
-        Self { stalled: true, errors: vec![] }
+        Self { errors: vec![] }
-    }
-
-    fn mark_not_stalled(&mut self) {
-        self.stalled = false;
-    }
-
-    fn is_stalled(&self) -> bool {
-        self.stalled
     }
 
     fn record_completed(&mut self, _outcome: &Self::Obligation) {
@@ -415,10 +394,7 @@ fn insert_into_error_cache(&mut self, index: usize) {
             .insert(node.obligation.as_cache_key());
     }
 
-    /// Performs a pass through the obligation list. This must
+    /// Performs a fixpoint computation over the obligation list.
-    /// be called in a loop until `outcome.stalled` is false.
-    ///
-    /// This _cannot_ be unrolled (presently, at least).
     #[inline(never)]
     pub fn process_obligations<P, OUT>(&mut self, processor: &mut P) -> OUT
     where
@@ -427,6 +403,10 @@ pub fn process_obligations<P, OUT>(&mut self, processor: &mut P) -> OUT
     {
         let mut outcome = OUT::new();
 
+        // Fixpoint computation: we repeat until the inner loop stalls.
+        loop {
+            let mut has_changed = false;
+
             // Note that the loop body can append new nodes, and those new nodes
             // will then be processed by subsequent iterations of the loop.
             //
@@ -453,7 +433,7 @@ pub fn process_obligations<P, OUT>(&mut self, processor: &mut P) -> OUT
                     }
                     ProcessResult::Changed(children) => {
                         // We are not (yet) stalled.
-                    outcome.mark_not_stalled();
+                        has_changed = true;
                         node.state.set(NodeState::Success);
 
                         for child in children {
@@ -466,16 +446,23 @@ pub fn process_obligations<P, OUT>(&mut self, processor: &mut P) -> OUT
                         }
                     }
                     ProcessResult::Error(err) => {
-                    outcome.mark_not_stalled();
+                        has_changed = true;
                         outcome.record_error(Error { error: err, backtrace: self.error_at(index) });
                     }
                 }
                 index += 1;
             }
 
-        // There's no need to perform marking, cycle processing and compression when nothing
+            // If unchanged, then we saw no successful obligations, which means
-        // changed.
+            // there is no point in further iteration. This is based on the
-        if !outcome.is_stalled() {
+            // assumption that when trait matching returns `Error` or
+            // `Unchanged`, those results do not affect environmental inference
+            // state. (Note that this will occur if we invoke
+            // `process_obligations` with no pending obligations.)
+            if !has_changed {
+                break;
+            }
+
             self.mark_successes();
             self.process_cycles(processor);
             self.compress(|obl| outcome.record_completed(obl));

compiler/rustc_data_structures/src/obligation_forest/tests.rs

@@ -20,7 +20,6 @@ struct ClosureObligationProcessor<OF, BF, O, E> {
 struct TestOutcome<O, E> {
     pub completed: Vec<O>,
     pub errors: Vec<Error<O, E>>,
-    pub stalled: bool,
 }
 
 impl<O, E> OutcomeTrait for TestOutcome<O, E>
@@ -31,15 +30,7 @@ impl<O, E> OutcomeTrait for TestOutcome<O, E>
     type Obligation = O;
 
     fn new() -> Self {
-        Self { errors: vec![], stalled: false, completed: vec![] }
+        Self { errors: vec![], completed: vec![] }
-    }
-
-    fn mark_not_stalled(&mut self) {
-        self.stalled = false;
-    }
-
-    fn is_stalled(&self) -> bool {
-        self.stalled
     }
 
     fn record_completed(&mut self, outcome: &Self::Obligation) {

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -133,28 +133,17 @@ fn select(&mut self, selcx: &mut SelectionContext<'a, 'tcx>) -> Vec<FulfillmentE
 
         let mut errors = Vec::new();
 
-        loop {
-            debug!("select: starting another iteration");
-
         // Process pending obligations.
-            let outcome: Outcome<_, _> =
+        let outcome: Outcome<_, _> = self.predicates.process_obligations(&mut FulfillProcessor {
-                self.predicates.process_obligations(&mut FulfillProcessor {
             selcx,
             register_region_obligations: self.register_region_obligations,
         });
-            debug!("select: outcome={:#?}", outcome);
 
         // FIXME: if we kept the original cache key, we could mark projection
         // obligations as complete for the projection cache here.
 
         errors.extend(outcome.errors.into_iter().map(to_fulfillment_error));
 
-            // If nothing new was added, no need to keep looping.
-            if outcome.stalled {
-                break;
-            }
-        }
-
         debug!(
             "select({} predicates remaining, {} errors) done",
             self.predicates.len(),