Introduce ProcessResult.

A tri-valued enum is nicer than Result<Option<T>>, and it's slightly
faster.

Author: nnethercote

src/librustc/traits/fulfill.rs

@@ -12,8 +12,8 @@ use infer::{RegionObligation, InferCtxt};
 use mir::interpret::GlobalId;
 use ty::{self, Ty, TypeFoldable, ToPolyTraitRef, ToPredicate};
 use ty::error::ExpectedFound;
-use rustc_data_structures::obligation_forest::{ObligationForest, Error};
-use rustc_data_structures::obligation_forest::{ForestObligation, ObligationProcessor};
+use rustc_data_structures::obligation_forest::{Error, ForestObligation, ObligationForest};
+use rustc_data_structures::obligation_forest::{ObligationProcessor, ProcessResult};
 use std::marker::PhantomData;
 use hir::def_id::DefId;
 use middle::const_val::{ConstEvalErr, ErrKind};
@@ -263,16 +263,16 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
     type Error = FulfillmentErrorCode<'tcx>;
 
     /// Processes a predicate obligation and returns either:
-    /// - `Ok(Some(v))` if the predicate is true, presuming that `v` are also true
-    /// - `Ok(None)` if we don't have enough info to be sure
-    /// - `Err` if the predicate does not hold
+    /// - `Changed(v)` if the predicate is true, presuming that `v` are also true
+    /// - `Unchanged` if we don't have enough info to be sure
+    /// - `Error(e)` if the predicate does not hold
     ///
     /// This is always inlined, despite its size, because it has a single
     /// callsite and it is called *very* frequently.
     #[inline(always)]
     fn process_obligation(&mut self,
                           pending_obligation: &mut Self::Obligation)
-                          -> Result<Option<Vec<Self::Obligation>>, Self::Error>
+                          -> ProcessResult<Self::Obligation, Self::Error>
     {
         // if we were stalled on some unresolved variables, first check
         // whether any of them have been resolved; if not, don't bother
@@ -286,7 +286,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                        self.selcx.infcx()
                            .resolve_type_vars_if_possible(&pending_obligation.obligation),
                        pending_obligation.stalled_on);
-                return Ok(None);
+                return ProcessResult::Unchanged;
             }
             pending_obligation.stalled_on = vec![];
         }
@@ -308,15 +308,15 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                     if self.selcx.infcx().predicate_must_hold(&obligation) {
                         debug!("selecting trait `{:?}` at depth {} evaluated to holds",
                                data, obligation.recursion_depth);
-                        return Ok(Some(vec![]))
+                        return ProcessResult::Changed(vec![])
                     }
                 }
 
                 match self.selcx.select(&trait_obligation) {
                     Ok(Some(vtable)) => {
                         debug!("selecting trait `{:?}` at depth {} yielded Ok(Some)",
                                data, obligation.recursion_depth);
-                        Ok(Some(mk_pending(vtable.nested_obligations())))
+                        ProcessResult::Changed(mk_pending(vtable.nested_obligations()))
                     }
                     Ok(None) => {
                         debug!("selecting trait `{:?}` at depth {} yielded Ok(None)",
@@ -342,21 +342,21 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                                self.selcx.infcx().resolve_type_vars_if_possible(obligation),
                                pending_obligation.stalled_on);
 
-                        Ok(None)
+                        ProcessResult::Unchanged
                     }
                     Err(selection_err) => {
                         info!("selecting trait `{:?}` at depth {} yielded Err",
                               data, obligation.recursion_depth);
 
-                        Err(CodeSelectionError(selection_err))
+                        ProcessResult::Error(CodeSelectionError(selection_err))
                     }
                 }
             }
 
             ty::Predicate::RegionOutlives(ref binder) => {
                 match self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder) {
-                    Ok(()) => Ok(Some(Vec::new())),
-                    Err(_) => Err(CodeSelectionError(Unimplemented)),
+                    Ok(()) => ProcessResult::Changed(vec![]),
+                    Err(_) => ProcessResult::Error(CodeSelectionError(Unimplemented)),
                 }
             }
 
@@ -373,7 +373,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                             // If so, this obligation is an error (for now). Eventually we should be
                             // able to support additional cases here, like `for<'a> &'a str: 'a`.
                             None => {
-                                Err(CodeSelectionError(Unimplemented))
+                                ProcessResult::Error(CodeSelectionError(Unimplemented))
                             }
                             // Otherwise, we have something of the form
                             // `for<'a> T: 'a where 'a not in T`, which we can treat as
@@ -389,7 +389,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                                             cause: obligation.cause.clone(),
                                         });
                                 }
-                                Ok(Some(vec![]))
+                                ProcessResult::Changed(vec![])
                             }
                         }
                     }
@@ -404,7 +404,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                                     cause: obligation.cause.clone()
                                 });
                         }
-                        Ok(Some(vec![]))
+                        ProcessResult::Changed(vec![])
                     }
                 }
             }
@@ -416,32 +416,32 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                         let tcx = self.selcx.tcx();
                         pending_obligation.stalled_on =
                             trait_ref_type_vars(self.selcx, data.to_poly_trait_ref(tcx));
-                        Ok(None)
+                        ProcessResult::Unchanged
                     }
-                    Ok(Some(os)) => Ok(Some(mk_pending(os))),
-                    Err(e) => Err(CodeProjectionError(e))
+                    Ok(Some(os)) => ProcessResult::Changed(mk_pending(os)),
+                    Err(e) => ProcessResult::Error(CodeProjectionError(e))
                 }
             }
 
             ty::Predicate::ObjectSafe(trait_def_id) => {
                 if !self.selcx.tcx().is_object_safe(trait_def_id) {
-                    Err(CodeSelectionError(Unimplemented))
+                    ProcessResult::Error(CodeSelectionError(Unimplemented))
                 } else {
-                    Ok(Some(Vec::new()))
+                    ProcessResult::Changed(vec![])
                 }
             }
 
             ty::Predicate::ClosureKind(closure_def_id, closure_substs, kind) => {
                 match self.selcx.infcx().closure_kind(closure_def_id, closure_substs) {
                     Some(closure_kind) => {
                         if closure_kind.extends(kind) {
-                            Ok(Some(vec![]))
+                            ProcessResult::Changed(vec![])
                         } else {
-                            Err(CodeSelectionError(Unimplemented))
+                            ProcessResult::Error(CodeSelectionError(Unimplemented))
                         }
                     }
                     None => {
-                        Ok(None)
+                        ProcessResult::Unchanged
                     }
                 }
             }
@@ -453,9 +453,9 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                                           ty, obligation.cause.span) {
                     None => {
                         pending_obligation.stalled_on = vec![ty];
-                        Ok(None)
+                        ProcessResult::Unchanged
                     }
-                    Some(os) => Ok(Some(mk_pending(os)))
+                    Some(os) => ProcessResult::Changed(mk_pending(os))
                 }
             }
 
@@ -467,24 +467,25 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                         // None means that both are unresolved.
                         pending_obligation.stalled_on = vec![subtype.skip_binder().a,
                                                              subtype.skip_binder().b];
-                        Ok(None)
+                        ProcessResult::Unchanged
                     }
                     Some(Ok(ok)) => {
-                        Ok(Some(mk_pending(ok.obligations)))
+                        ProcessResult::Changed(mk_pending(ok.obligations))
                     }
                     Some(Err(err)) => {
                         let expected_found = ExpectedFound::new(subtype.skip_binder().a_is_expected,
                                                                 subtype.skip_binder().a,
                                                                 subtype.skip_binder().b);
-                        Err(FulfillmentErrorCode::CodeSubtypeError(expected_found, err))
+                        ProcessResult::Error(
+                            FulfillmentErrorCode::CodeSubtypeError(expected_found, err))
                     }
                 }
             }
 
             ty::Predicate::ConstEvaluatable(def_id, substs) => {
                 match self.selcx.tcx().lift_to_global(&obligation.param_env) {
                     None => {
-                        Ok(None)
+                        ProcessResult::Unchanged
                     }
                     Some(param_env) => {
                         match self.selcx.tcx().lift_to_global(&substs) {
@@ -502,19 +503,22 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                                     };
                                     match self.selcx.tcx().at(obligation.cause.span)
                                                           .const_eval(param_env.and(cid)) {
-                                        Ok(_) => Ok(Some(vec![])),
-                                        Err(err) => Err(CodeSelectionError(ConstEvalFailure(err)))
+                                        Ok(_) => ProcessResult::Changed(vec![]),
+                                        Err(err) => ProcessResult::Error(
+                                            CodeSelectionError(ConstEvalFailure(err)))
                                     }
                                 } else {
-                                    Err(CodeSelectionError(ConstEvalFailure(ConstEvalErr {
-                                        span: obligation.cause.span,
-                                        kind: ErrKind::CouldNotResolve.into(),
-                                    })))
+                                    ProcessResult::Error(
+                                        CodeSelectionError(ConstEvalFailure(ConstEvalErr {
+                                            span: obligation.cause.span,
+                                            kind: ErrKind::CouldNotResolve.into(),
+                                        }))
+                                    )
                                 }
                             },
                             None => {
                                 pending_obligation.stalled_on = substs.types().collect();
-                                Ok(None)
+                                ProcessResult::Unchanged
                             }
                         }
                     }

src/librustc_data_structures/obligation_forest/mod.rs

@@ -41,7 +41,7 @@ pub trait ObligationProcessor {
 
     fn process_obligation(&mut self,
                           obligation: &mut Self::Obligation)
-                          -> Result<Option<Vec<Self::Obligation>>, Self::Error>;
+                          -> ProcessResult<Self::Obligation, Self::Error>;
 
     /// As we do the cycle check, we invoke this callback when we
     /// encounter an actual cycle. `cycle` is an iterator that starts
@@ -57,6 +57,14 @@ pub trait ObligationProcessor {
         where I: Clone + Iterator<Item=&'c Self::Obligation>;
 }
 
+/// The result type used by `process_obligation`.
+#[derive(Debug)]
+pub enum ProcessResult<O, E> {
+    Unchanged,
+    Changed(Vec<O>),
+    Error(E),
+}
+
 pub struct ObligationForest<O: ForestObligation> {
     /// The list of obligations. In between calls to
     /// `process_obligations`, this list only contains nodes in the
@@ -136,8 +144,8 @@ pub struct Outcome<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 `Err` or
-    /// `Ok(None)`, those results do not affect environmental
+    /// 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,
@@ -270,11 +278,11 @@ impl<O: ForestObligation> ObligationForest<O> {
                    result);
 
             match result {
-                Ok(None) => {
-                    // no change in state
+                ProcessResult::Unchanged => {
+                    // No change in state.
                 }
-                Ok(Some(children)) => {
-                    // if we saw a Some(_) result, we are not (yet) stalled
+                ProcessResult::Changed(children) => {
+                    // We are not (yet) stalled.
                     stalled = false;
                     self.nodes[index].state.set(NodeState::Success);
 
@@ -290,7 +298,7 @@ impl<O: ForestObligation> ObligationForest<O> {
                         }
                     }
                 }
-                Err(err) => {
+                ProcessResult::Error(err) => {
                     stalled = false;
                     let backtrace = self.error_at(index);
                     errors.push(Error {