Skip to content

Commit

Permalink
simplify and reduce the size of EvaluationResult
Browse files Browse the repository at this point in the history
  • Loading branch information
Ariel Ben-Yehuda committed Nov 15, 2015
1 parent 768630b commit a43533a
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 56 deletions.
1 change: 1 addition & 0 deletions src/librustc/middle/traits/mod.rs
Expand Up @@ -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};
Expand Down
154 changes: 98 additions & 56 deletions src/librustc/middle/traits/select.rs
Expand Up @@ -236,11 +236,24 @@ enum BuiltinBoundConditions<'tcx> {
AmbiguousBuiltin
}

#[derive(Debug)]
enum EvaluationResult<'tcx> {
#[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
/// 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> {
Expand Down Expand Up @@ -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>)
Expand All @@ -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 => { }
}
}
Expand All @@ -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);
Expand All @@ -464,7 +459,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
});
match result {
Ok(()) => EvaluatedToOk,
Err(_) => EvaluatedToErr(Unimplemented),
Err(_) => EvaluatedToErr
}
}

Expand All @@ -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
}
}

Expand All @@ -505,7 +500,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
EvaluatedToAmbig
}
Err(_) => {
EvaluatedToErr(Unimplemented)
EvaluatedToErr
}
}
})
Expand All @@ -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
Expand Down Expand Up @@ -559,16 +565,19 @@ 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
unbound_input_types &&
(self.intercrate ||
if unbound_input_types && self.intercrate {
debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> 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)))
&prev.fresh_trait_ref))
{
debug!("evaluate_stack({:?}) --> unbound argument, recursion --> ambiguous",
debug!("evaluate_stack({:?}) --> unbound argument, recursive --> giving up",
stack.fresh_trait_ref);
return EvaluatedToAmbig;
return EvaluatedToUnknown;
}

// If there is any previous entry on the stack that precisely
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
//
Expand Down Expand Up @@ -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;
Expand All @@ -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());
}
Expand Down Expand Up @@ -1138,15 +1175,15 @@ 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) {
Ok(obligations) => {
self.evaluate_predicates_recursively(stack.list(), obligations.iter())
}
Err(()) => {
EvaluatedToErr(Unimplemented)
EvaluatedToErr
}
}
})
Expand Down Expand Up @@ -1492,15 +1529,15 @@ 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(|_| {
let candidate = (*candidate).clone();
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={:?}",
Expand All @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(..)) |
EvaluatedToErr(TraitNotObjectSafe(_)) =>
true,
EvaluatedToUnknown => true,

EvaluatedToErr(Unimplemented) =>
false,
EvaluatedToErr => false
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/middle/ty/mod.rs
Expand Up @@ -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.
Expand All @@ -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,
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
};

Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/ty/structural_impls.rs
Expand Up @@ -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,
}
}
Expand Down

0 comments on commit a43533a

Please sign in to comment.