simplify and reduce the size of EvaluationResult

src/librustc/middle/traits/mod.rs

@@ -44,6 +44,7 @@ pub use self::object_safety::object_safety_violations;
 pub use self::object_safety::ObjectSafetyViolation;
 pub use self::object_safety::MethodViolationCode;
 pub use self::object_safety::is_vtable_safe_method;
+pub use self::select::EvaluationCache;
 pub use self::select::SelectionContext;
 pub use self::select::SelectionCache;
 pub use self::select::{MethodMatchResult, MethodMatched, MethodAmbiguous, MethodDidNotMatch};

src/librustc/middle/traits/select.rs

@@ -236,11 +236,24 @@ enum BuiltinBoundConditions<'tcx> {
     AmbiguousBuiltin
 }
 
-#[derive(Debug)]
+#[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
-enum EvaluationResult<'tcx> {
+/// The result of trait evaluation. The order is important
+/// here as the evaluation of a list is the maximum of the
+/// evaluations.
+enum EvaluationResult {
+    /// Evaluation successful
     EvaluatedToOk,
+    /// Evaluation failed because of recursion - treated as ambiguous
+    EvaluatedToUnknown,
+    /// Evaluation is known to be ambiguous
     EvaluatedToAmbig,
-    EvaluatedToErr(SelectionError<'tcx>),
+    /// Evaluation failed
+    EvaluatedToErr,
+}
+
+#[derive(Clone)]
+pub struct EvaluationCache<'tcx> {
+    hashmap: RefCell<FnvHashMap<ty::PolyTraitRef<'tcx>, EvaluationResult>>
 }
 
 impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
@@ -381,6 +394,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     // The result is "true" if the obligation *may* hold and "false" if
     // we can be sure it does not.
 
+
     /// Evaluates whether the obligation `obligation` can be satisfied (by any means).
     pub fn evaluate_obligation(&mut self,
                                obligation: &PredicateObligation<'tcx>)
@@ -393,41 +407,22 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             .may_apply()
     }
 
-    fn evaluate_builtin_bound_recursively<'o>(&mut self,
-                                              bound: ty::BuiltinBound,
-                                              previous_stack: &TraitObligationStack<'o, 'tcx>,
-                                              ty: Ty<'tcx>)
-                                              -> EvaluationResult<'tcx>
-    {
-        let obligation =
-            util::predicate_for_builtin_bound(
-                self.tcx(),
-                previous_stack.obligation.cause.clone(),
-                bound,
-                previous_stack.obligation.recursion_depth + 1,
-                ty);
-
-        match obligation {
-            Ok(obligation) => {
-                self.evaluate_predicate_recursively(previous_stack.list(), &obligation)
-            }
-            Err(ErrorReported) => {
-                EvaluatedToOk
-            }
-        }
-    }
-
     fn evaluate_predicates_recursively<'a,'o,I>(&mut self,
                                                 stack: TraitObligationStackList<'o, 'tcx>,
                                                 predicates: I)
-                                                -> EvaluationResult<'tcx>
+                                                -> EvaluationResult
         where I : Iterator<Item=&'a PredicateObligation<'tcx>>, 'tcx:'a
     {
         let mut result = EvaluatedToOk;
         for obligation in predicates {
             match self.evaluate_predicate_recursively(stack, obligation) {
-                EvaluatedToErr(e) => { return EvaluatedToErr(e); }
+                EvaluatedToErr => { return EvaluatedToErr; }
                 EvaluatedToAmbig => { result = EvaluatedToAmbig; }
+                EvaluatedToUnknown => {
+                    if result < EvaluatedToUnknown {
+                        result = EvaluatedToUnknown;
+                    }
+                }
                 EvaluatedToOk => { }
             }
         }
@@ -437,7 +432,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn evaluate_predicate_recursively<'o>(&mut self,
                                           previous_stack: TraitObligationStackList<'o, 'tcx>,
                                           obligation: &PredicateObligation<'tcx>)
-                                           -> EvaluationResult<'tcx>
+                                           -> EvaluationResult
     {
         debug!("evaluate_predicate_recursively({:?})",
                obligation);
@@ -464,7 +459,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 });
                 match result {
                     Ok(()) => EvaluatedToOk,
-                    Err(_) => EvaluatedToErr(Unimplemented),
+                    Err(_) => EvaluatedToErr
                 }
             }
 
@@ -489,7 +484,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 if object_safety::is_object_safe(self.tcx(), trait_def_id) {
                     EvaluatedToOk
                 } else {
-                    EvaluatedToErr(Unimplemented)
+                    EvaluatedToErr
                 }
             }
 
@@ -505,7 +500,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                             EvaluatedToAmbig
                         }
                         Err(_) => {
-                            EvaluatedToErr(Unimplemented)
+                            EvaluatedToErr
                         }
                     }
                 })
@@ -516,22 +511,33 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn evaluate_obligation_recursively<'o>(&mut self,
                                            previous_stack: TraitObligationStackList<'o, 'tcx>,
                                            obligation: &TraitObligation<'tcx>)
-                                           -> EvaluationResult<'tcx>
+                                           -> EvaluationResult
     {
         debug!("evaluate_obligation_recursively({:?})",
                obligation);
 
         let stack = self.push_stack(previous_stack, obligation);
+        let fresh_trait_ref = stack.fresh_trait_ref;
+        if let Some(result) = self.check_evaluation_cache(fresh_trait_ref) {
+            debug!("CACHE HIT: EVAL({:?})={:?}",
+                   fresh_trait_ref,
+                   result);
+            return result;
+        }
 
         let result = self.evaluate_stack(&stack);
 
-        debug!("result: {:?}", result);
+        debug!("CACHE MISS: EVAL({:?})={:?}",
+               fresh_trait_ref,
+               result);
+        self.insert_evaluation_cache(fresh_trait_ref, result);
+
         result
     }
 
     fn evaluate_stack<'o>(&mut self,
                           stack: &TraitObligationStack<'o, 'tcx>)
-                          -> EvaluationResult<'tcx>
+                          -> EvaluationResult
     {
         // In intercrate mode, whenever any of the types are unbound,
         // there can always be an impl. Even if there are no impls in
@@ -559,17 +565,20 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // precise still.
         let input_types = stack.fresh_trait_ref.0.input_types();
         let unbound_input_types = input_types.iter().any(|ty| ty.is_fresh());
-        if
+        if unbound_input_types && self.intercrate {
-            unbound_input_types &&
+            debug!("evaluate_stack({:?}) --> unbound argument, intercrate -->  ambiguous",
-             (self.intercrate ||
-              stack.iter().skip(1).any(
-                  |prev| self.match_fresh_trait_refs(&stack.fresh_trait_ref,
-                                                     &prev.fresh_trait_ref)))
-        {
-            debug!("evaluate_stack({:?}) --> unbound argument, recursion -->  ambiguous",
                    stack.fresh_trait_ref);
             return EvaluatedToAmbig;
         }
+        if unbound_input_types &&
+              stack.iter().skip(1).any(
+                  |prev| self.match_fresh_trait_refs(&stack.fresh_trait_ref,
+                                                     &prev.fresh_trait_ref))
+        {
+            debug!("evaluate_stack({:?}) --> unbound argument, recursive --> giving up",
+                   stack.fresh_trait_ref);
+            return EvaluatedToUnknown;
+        }
 
         // If there is any previous entry on the stack that precisely
         // matches this obligation, then we can assume that the
@@ -603,7 +612,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         match self.candidate_from_obligation(stack) {
             Ok(Some(c)) => self.winnow_candidate(stack, &c),
             Ok(None) => EvaluatedToAmbig,
-            Err(e) => EvaluatedToErr(e),
+            Err(..) => EvaluatedToErr
         }
     }
 
@@ -637,6 +646,34 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         })
     }
 
+    fn pick_evaluation_cache(&self) -> &EvaluationCache<'tcx> {
+        &self.param_env().evaluation_cache
+    }
+
+    fn check_evaluation_cache(&self, trait_ref: ty::PolyTraitRef<'tcx>)
+                              -> Option<EvaluationResult>
+    {
+        let cache = self.pick_evaluation_cache();
+        cache.hashmap.borrow().get(&trait_ref).cloned()
+    }
+
+    fn insert_evaluation_cache(&mut self,
+                               trait_ref: ty::PolyTraitRef<'tcx>,
+                               result: EvaluationResult)
+    {
+        // Avoid caching results that depend on more than just the trait-ref:
+        // The stack can create EvaluatedToUnknown, and closure signatures
+        // being yet uninferred can create "spurious" EvaluatedToAmbig.
+        if result == EvaluatedToUnknown ||
+            (result == EvaluatedToAmbig && trait_ref.has_closure_types())
+        {
+            return;
+        }
+
+        let cache = self.pick_evaluation_cache();
+        cache.hashmap.borrow_mut().insert(trait_ref, result);
+    }
+
     ///////////////////////////////////////////////////////////////////////////
     // CANDIDATE ASSEMBLY
     //
@@ -669,7 +706,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         match self.check_candidate_cache(&cache_fresh_trait_pred) {
             Some(c) => {
-                debug!("CACHE HIT: cache_fresh_trait_pred={:?}, candidate={:?}",
+                debug!("CACHE HIT: SELECT({:?})={:?}",
                        cache_fresh_trait_pred,
                        c);
                 return c;
@@ -681,7 +718,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         let candidate = self.candidate_from_obligation_no_cache(stack);
 
         if self.should_update_candidate_cache(&cache_fresh_trait_pred, &candidate) {
-            debug!("CACHE MISS: cache_fresh_trait_pred={:?}, candidate={:?}",
+            debug!("CACHE MISS: SELECT({:?})={:?}",
                    cache_fresh_trait_pred, candidate);
             self.insert_candidate_cache(cache_fresh_trait_pred, candidate.clone());
         }
@@ -1138,7 +1175,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn evaluate_where_clause<'o>(&mut self,
                                  stack: &TraitObligationStack<'o, 'tcx>,
                                  where_clause_trait_ref: ty::PolyTraitRef<'tcx>)
-                                 -> EvaluationResult<'tcx>
+                                 -> EvaluationResult
     {
         self.infcx().probe(move |_| {
             match self.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) {
@@ -1146,7 +1183,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     self.evaluate_predicates_recursively(stack.list(), obligations.iter())
                 }
                 Err(()) => {
-                    EvaluatedToErr(Unimplemented)
+                    EvaluatedToErr
                 }
             }
         })
@@ -1492,7 +1529,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn winnow_candidate<'o>(&mut self,
                             stack: &TraitObligationStack<'o, 'tcx>,
                             candidate: &SelectionCandidate<'tcx>)
-                            -> EvaluationResult<'tcx>
+                            -> EvaluationResult
     {
         debug!("winnow_candidate: candidate={:?}", candidate);
         let result = self.infcx.probe(|_| {
@@ -1500,7 +1537,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             match self.confirm_candidate(stack.obligation, candidate) {
                 Ok(selection) => self.winnow_selection(stack.list(),
                                                        selection),
-                Err(error) => EvaluatedToErr(error),
+                Err(..) => EvaluatedToErr
             }
         });
         debug!("winnow_candidate depth={} result={:?}",
@@ -1511,7 +1548,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn winnow_selection<'o>(&mut self,
                             stack: TraitObligationStackList<'o,'tcx>,
                             selection: Selection<'tcx>)
-                            -> EvaluationResult<'tcx>
+                            -> EvaluationResult
     {
         self.evaluate_predicates_recursively(stack,
                                              selection.nested_obligations().iter())
@@ -2956,6 +2993,14 @@ impl<'tcx> SelectionCache<'tcx> {
     }
 }
 
+impl<'tcx> EvaluationCache<'tcx> {
+    pub fn new() -> EvaluationCache<'tcx> {
+        EvaluationCache {
+            hashmap: RefCell::new(FnvHashMap())
+        }
+    }
+}
+
 impl<'o,'tcx> TraitObligationStack<'o,'tcx> {
     fn list(&'o self) -> TraitObligationStackList<'o,'tcx> {
         TraitObligationStackList::with(self)
@@ -3001,17 +3046,14 @@ impl<'o,'tcx> fmt::Debug for TraitObligationStack<'o,'tcx> {
     }
 }
 
-impl<'tcx> EvaluationResult<'tcx> {
+impl EvaluationResult {
     fn may_apply(&self) -> bool {
         match *self {
             EvaluatedToOk |
             EvaluatedToAmbig |
-            EvaluatedToErr(OutputTypeParameterMismatch(..)) |
+            EvaluatedToUnknown => true,
-            EvaluatedToErr(TraitNotObjectSafe(_)) =>
-                true,
 
-            EvaluatedToErr(Unimplemented) =>
+            EvaluatedToErr => false
-                false,
         }
     }
 }

src/librustc/middle/ty/mod.rs

@@ -1091,6 +1091,9 @@ pub struct ParameterEnvironment<'a, 'tcx:'a> {
     /// for things that have to do with the parameters in scope.
     pub selection_cache: traits::SelectionCache<'tcx>,
 
+    /// Caches the results of trait evaluation.
+    pub evaluation_cache: traits::EvaluationCache<'tcx>,
+
     /// Scope that is attached to free regions for this scope. This
     /// is usually the id of the fn body, but for more abstract scopes
     /// like structs we often use the node-id of the struct.
@@ -1112,6 +1115,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> {
             implicit_region_bound: self.implicit_region_bound,
             caller_bounds: caller_bounds,
             selection_cache: traits::SelectionCache::new(),
+            evaluation_cache: traits::EvaluationCache::new(),
             free_id: self.free_id,
         }
     }
@@ -2584,6 +2588,7 @@ impl<'tcx> ctxt<'tcx> {
                                    caller_bounds: Vec::new(),
                                    implicit_region_bound: ty::ReEmpty,
                                    selection_cache: traits::SelectionCache::new(),
+                                   evaluation_cache: traits::EvaluationCache::new(),
 
                                    // for an empty parameter
                                    // environment, there ARE no free
@@ -2673,6 +2678,7 @@ impl<'tcx> ctxt<'tcx> {
             implicit_region_bound: ty::ReScope(free_id_outlive),
             caller_bounds: predicates,
             selection_cache: traits::SelectionCache::new(),
+            evaluation_cache: traits::EvaluationCache::new(),
             free_id: free_id,
         };
 

src/librustc/middle/ty/structural_impls.rs

@@ -822,6 +822,7 @@ impl<'a, 'tcx> TypeFoldable<'tcx> for ty::ParameterEnvironment<'a, 'tcx> where '
             implicit_region_bound: self.implicit_region_bound.fold_with(folder),
             caller_bounds: self.caller_bounds.fold_with(folder),
             selection_cache: traits::SelectionCache::new(),
+            evaluation_cache: traits::EvaluationCache::new(),
             free_id: self.free_id,
         }
     }

src/test/compile-fail/issue-29147.rs

@@ -0,0 +1,32 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![recursion_limit="1024"]
+
+pub struct S0<T>(T,T);
+pub struct S1<T>(Option<Box<S0<S0<T>>>>,Option<Box<S0<S0<T>>>>);
+pub struct S2<T>(Option<Box<S1<S1<T>>>>,Option<Box<S1<S1<T>>>>);
+pub struct S3<T>(Option<Box<S2<S2<T>>>>,Option<Box<S2<S2<T>>>>);
+pub struct S4<T>(Option<Box<S3<S3<T>>>>,Option<Box<S3<S3<T>>>>);
+pub struct S5<T>(Option<Box<S4<S4<T>>>>,Option<Box<S4<S4<T>>>>,Option<T>);
+
+trait Foo { fn xxx(&self); }
+trait Bar {} // anything local or #[fundamental]
+
+impl<T> Foo for T where T: Bar, T: Sync {
+    fn xxx(&self) {}
+}
+
+impl Foo for S5<u32> { fn xxx(&self) {} }
+impl Foo for S5<u64> { fn xxx(&self) {} }
+
+fn main() {
+    let _ = <S5<_>>::xxx; //~ ERROR cannot resolve `S5<_> : Foo`
+}

src/test/run-pass/issue-29147.rs

@@ -0,0 +1,35 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![recursion_limit="1024"]
+
+use std::mem;
+
+pub struct S0<T>(T,T);
+pub struct S1<T>(Option<Box<S0<S0<T>>>>,Option<Box<S0<S0<T>>>>);
+pub struct S2<T>(Option<Box<S1<S1<T>>>>,Option<Box<S1<S1<T>>>>);
+pub struct S3<T>(Option<Box<S2<S2<T>>>>,Option<Box<S2<S2<T>>>>);
+pub struct S4<T>(Option<Box<S3<S3<T>>>>,Option<Box<S3<S3<T>>>>);
+pub struct S5<T>(Option<Box<S4<S4<T>>>>,Option<Box<S4<S4<T>>>>,Option<T>);
+
+trait Foo { fn xxx(&self); }
+trait Bar {} // anything local or #[fundamental]
+
+impl<T> Foo for T where T: Bar, T: Sync {
+    fn xxx(&self) {}
+}
+
+impl Foo for S5<u8> { fn xxx(&self) {} }
+
+fn main() {
+    let s = S5(None,None,None);
+    s.xxx();
+    assert_eq!(mem::size_of_val(&s.2), mem::size_of::<Option<u8>>());
+}