1
Fork 0

clear out projection subobligations after they are processed

After a projection was processed, its derived subobligations no longer
need any processing when encountered, and can be removed. This improves
the status of #43787.

This is actually complementary to #43938 - that PR fixes selection
caching (and @remram44's example, which "accidentally" worked because of
the buggy projection caching) while this PR fixes projection caching
This commit is contained in:
Ariel Ben-Yehuda 2017-08-20 19:16:36 +03:00
parent 78e95bb7ac
commit 7534f7375b
3 changed files with 112 additions and 19 deletions

View file

@ -251,6 +251,9 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
}); });
debug!("select: outcome={:?}", outcome); 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( errors.extend(
outcome.errors.into_iter() outcome.errors.into_iter()
.map(|e| to_fulfillment_error(e))); .map(|e| to_fulfillment_error(e)));

View file

@ -121,11 +121,13 @@ struct ProjectionTyCandidateSet<'tcx> {
/// ///
/// for<...> <T as Trait>::U == V /// for<...> <T as Trait>::U == V
/// ///
/// If successful, this may result in additional obligations. /// If successful, this may result in additional obligations. Also returns
/// the projection cache key used to track these additional obligations.
pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>( pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
obligation: &PolyProjectionObligation<'tcx>) obligation: &PolyProjectionObligation<'tcx>)
-> Result<Option<Vec<PredicateObligation<'tcx>>>, MismatchedProjectionTypes<'tcx>> -> Result<Option<Vec<PredicateObligation<'tcx>>>,
MismatchedProjectionTypes<'tcx>>
{ {
debug!("poly_project_and_unify_type(obligation={:?})", debug!("poly_project_and_unify_type(obligation={:?})",
obligation); obligation);
@ -161,7 +163,8 @@ pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(
fn project_and_unify_type<'cx, 'gcx, 'tcx>( fn project_and_unify_type<'cx, 'gcx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
obligation: &ProjectionObligation<'tcx>) obligation: &ProjectionObligation<'tcx>)
-> Result<Option<Vec<PredicateObligation<'tcx>>>, MismatchedProjectionTypes<'tcx>> -> Result<Option<Vec<PredicateObligation<'tcx>>>,
MismatchedProjectionTypes<'tcx>>
{ {
debug!("project_and_unify_type(obligation={:?})", debug!("project_and_unify_type(obligation={:?})",
obligation); obligation);
@ -396,6 +399,7 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
let infcx = selcx.infcx(); let infcx = selcx.infcx();
let projection_ty = infcx.resolve_type_vars_if_possible(&projection_ty); let projection_ty = infcx.resolve_type_vars_if_possible(&projection_ty);
let cache_key = ProjectionCacheKey { ty: projection_ty };
debug!("opt_normalize_projection_type(\ debug!("opt_normalize_projection_type(\
projection_ty={:?}, \ projection_ty={:?}, \
@ -411,7 +415,7 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
// bounds. It might be the case that we want two distinct caches, // bounds. It might be the case that we want two distinct caches,
// or else another kind of cache entry. // or else another kind of cache entry.
match infcx.projection_cache.borrow_mut().try_start(projection_ty) { match infcx.projection_cache.borrow_mut().try_start(cache_key) {
Ok(()) => { } Ok(()) => { }
Err(ProjectionCacheEntry::Ambiguous) => { Err(ProjectionCacheEntry::Ambiguous) => {
// If we found ambiguity the last time, that generally // If we found ambiguity the last time, that generally
@ -522,7 +526,7 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
obligations, obligations,
} }
}; };
infcx.projection_cache.borrow_mut().complete(projection_ty, &result); infcx.projection_cache.borrow_mut().insert_ty(cache_key, &result);
Some(result) Some(result)
} }
Ok(ProjectedTy::NoProgress(projected_ty)) => { Ok(ProjectedTy::NoProgress(projected_ty)) => {
@ -533,14 +537,14 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
value: projected_ty, value: projected_ty,
obligations: vec![] obligations: vec![]
}; };
infcx.projection_cache.borrow_mut().complete(projection_ty, &result); infcx.projection_cache.borrow_mut().insert_ty(cache_key, &result);
Some(result) Some(result)
} }
Err(ProjectionTyError::TooManyCandidates) => { Err(ProjectionTyError::TooManyCandidates) => {
debug!("opt_normalize_projection_type: \ debug!("opt_normalize_projection_type: \
too many candidates"); too many candidates");
infcx.projection_cache.borrow_mut() infcx.projection_cache.borrow_mut()
.ambiguous(projection_ty); .ambiguous(cache_key);
None None
} }
Err(ProjectionTyError::TraitSelectionError(_)) => { Err(ProjectionTyError::TraitSelectionError(_)) => {
@ -551,7 +555,7 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
// reported later // reported later
infcx.projection_cache.borrow_mut() infcx.projection_cache.borrow_mut()
.error(projection_ty); .error(cache_key);
Some(normalize_to_error(selcx, param_env, projection_ty, cause, depth)) Some(normalize_to_error(selcx, param_env, projection_ty, cause, depth))
} }
} }
@ -1323,8 +1327,62 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>(
// # Cache // # Cache
/// The projection cache. Unlike the standard caches, this can
/// include infcx-dependent type variables - therefore, we have to roll
/// the cache back each time we roll a snapshot back, to avoid assumptions
/// on yet-unresolved inference variables. Types with skolemized regions
/// also have to be removed when the respective snapshot ends.
///
/// Because of that, projection cache entries can be "stranded" and left
/// inaccessible when type variables inside the key are resolved. We make no
/// attempt to recover or remove "stranded" entries, but rather let them be
/// (for the lifetime of the infcx).
///
/// Entries in the projection cache might contain inference variables
/// that will be resolved by obligations on the projection cache entry - e.g.
/// when a type parameter in the associated type is constrained through
/// an "RFC 447" projection on the impl.
///
/// When working with a fulfillment context, the derived obligations of each
/// projection cache entry will be registered on the fulfillcx, so any users
/// that can wait for a fulfillcx fixed point need not care about this. However,
/// users that don't wait for a fixed point (e.g. trait evaluation) have to
/// resolve the obligations themselves to make sure the projected result is
/// ok and avoid issues like #43132.
///
/// If that is done, after evaluation the obligations, it is a good idea to
/// call `ProjectionCache::complete` to make sure the obligations won't be
/// re-evaluated and avoid an exponential worst-case.
///
/// FIXME: we probably also want some sort of cross-infcx cache here to
/// reduce the amount of duplication. Let's see what we get with the Chalk
/// reforms.
pub struct ProjectionCache<'tcx> { pub struct ProjectionCache<'tcx> {
map: SnapshotMap<ty::ProjectionTy<'tcx>, ProjectionCacheEntry<'tcx>>, map: SnapshotMap<ProjectionCacheKey<'tcx>, ProjectionCacheEntry<'tcx>>,
}
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub struct ProjectionCacheKey<'tcx> {
ty: ty::ProjectionTy<'tcx>
}
impl<'cx, 'gcx, 'tcx> ProjectionCacheKey<'tcx> {
pub fn from_poly_projection_predicate(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
predicate: &ty::PolyProjectionPredicate<'tcx>)
-> Option<Self>
{
let infcx = selcx.infcx();
// We don't do cross-snapshot caching of obligations with escaping regions,
// so there's no cache key to use
infcx.tcx.no_late_bound_regions(&predicate)
.map(|predicate| ProjectionCacheKey {
// We don't attempt to match up with a specific type-variable state
// from a specific call to `opt_normalize_projection_type` - if
// there's no precise match, the original cache entry is "stranded"
// anyway.
ty: infcx.resolve_type_vars_if_possible(&predicate.projection_ty)
})
}
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
@ -1337,7 +1395,7 @@ enum ProjectionCacheEntry<'tcx> {
// NB: intentionally not Clone // NB: intentionally not Clone
pub struct ProjectionCacheSnapshot { pub struct ProjectionCacheSnapshot {
snapshot: Snapshot snapshot: Snapshot,
} }
impl<'tcx> ProjectionCache<'tcx> { impl<'tcx> ProjectionCache<'tcx> {
@ -1356,7 +1414,7 @@ impl<'tcx> ProjectionCache<'tcx> {
} }
pub fn rollback_skolemized(&mut self, snapshot: &ProjectionCacheSnapshot) { pub fn rollback_skolemized(&mut self, snapshot: &ProjectionCacheSnapshot) {
self.map.partial_rollback(&snapshot.snapshot, &|k| k.has_re_skol()); self.map.partial_rollback(&snapshot.snapshot, &|k| k.ty.has_re_skol());
} }
pub fn commit(&mut self, snapshot: ProjectionCacheSnapshot) { pub fn commit(&mut self, snapshot: ProjectionCacheSnapshot) {
@ -1366,7 +1424,7 @@ impl<'tcx> ProjectionCache<'tcx> {
/// Try to start normalize `key`; returns an error if /// Try to start normalize `key`; returns an error if
/// normalization already occurred (this error corresponds to a /// normalization already occurred (this error corresponds to a
/// cache hit, so it's actually a good thing). /// cache hit, so it's actually a good thing).
fn try_start(&mut self, key: ty::ProjectionTy<'tcx>) fn try_start(&mut self, key: ProjectionCacheKey<'tcx>)
-> Result<(), ProjectionCacheEntry<'tcx>> { -> Result<(), ProjectionCacheEntry<'tcx>> {
if let Some(entry) = self.map.get(&key) { if let Some(entry) = self.map.get(&key) {
return Err(entry.clone()); return Err(entry.clone());
@ -1377,25 +1435,51 @@ impl<'tcx> ProjectionCache<'tcx> {
} }
/// Indicates that `key` was normalized to `value`. /// Indicates that `key` was normalized to `value`.
fn complete(&mut self, key: ty::ProjectionTy<'tcx>, value: &NormalizedTy<'tcx>) { fn insert_ty(&mut self, key: ProjectionCacheKey<'tcx>, value: &NormalizedTy<'tcx>) {
debug!("ProjectionCacheEntry::complete: adding cache entry: key={:?}, value={:?}", debug!("ProjectionCacheEntry::insert_ty: adding cache entry: key={:?}, value={:?}",
key, value); key, value);
let fresh_key = self.map.insert(key, ProjectionCacheEntry::NormalizedTy(value.clone())); let fresh_key = self.map.insert(key, ProjectionCacheEntry::NormalizedTy(value.clone()));
assert!(!fresh_key, "never started projecting `{:?}`", key); assert!(!fresh_key, "never started projecting `{:?}`", key);
} }
/// Mark the relevant projection cache key as having its derived obligations
/// complete, so they won't have to be re-computed (this is OK to do in a
/// snapshot - if the snapshot is rolled back, the obligations will be
/// marked as incomplete again).
pub fn complete(&mut self, key: ProjectionCacheKey<'tcx>) {
let ty = match self.map.get(&key) {
Some(&ProjectionCacheEntry::NormalizedTy(ref ty)) => {
debug!("ProjectionCacheEntry::complete({:?}) - completing {:?}",
key, ty);
ty.value
}
ref value => {
// Type inference could "strand behind" old cache entries. Leave
// them alone for now.
debug!("ProjectionCacheEntry::complete({:?}) - ignoring {:?}",
key, value);
return
}
};
self.map.insert(key, ProjectionCacheEntry::NormalizedTy(Normalized {
value: ty,
obligations: vec![]
}));
}
/// Indicates that trying to normalize `key` resulted in /// Indicates that trying to normalize `key` resulted in
/// ambiguity. No point in trying it again then until we gain more /// ambiguity. No point in trying it again then until we gain more
/// type information (in which case, the "fully resolved" key will /// type information (in which case, the "fully resolved" key will
/// be different). /// be different).
fn ambiguous(&mut self, key: ty::ProjectionTy<'tcx>) { fn ambiguous(&mut self, key: ProjectionCacheKey<'tcx>) {
let fresh = self.map.insert(key, ProjectionCacheEntry::Ambiguous); let fresh = self.map.insert(key, ProjectionCacheEntry::Ambiguous);
assert!(!fresh, "never started projecting `{:?}`", key); assert!(!fresh, "never started projecting `{:?}`", key);
} }
/// Indicates that trying to normalize `key` resulted in /// Indicates that trying to normalize `key` resulted in
/// error. /// error.
fn error(&mut self, key: ty::ProjectionTy<'tcx>) { fn error(&mut self, key: ProjectionCacheKey<'tcx>) {
let fresh = self.map.insert(key, ProjectionCacheEntry::Error); let fresh = self.map.insert(key, ProjectionCacheEntry::Error);
assert!(!fresh, "never started projecting `{:?}`", key); assert!(!fresh, "never started projecting `{:?}`", key);
} }

View file

@ -16,7 +16,7 @@ use self::EvaluationResult::*;
use super::coherence; use super::coherence;
use super::DerivedObligationCause; use super::DerivedObligationCause;
use super::project; use super::project;
use super::project::{normalize_with_depth, Normalized}; use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey};
use super::{PredicateObligation, TraitObligation, ObligationCause}; use super::{PredicateObligation, TraitObligation, ObligationCause};
use super::{ObligationCauseCode, BuiltinDerivedObligation, ImplDerivedObligation}; use super::{ObligationCauseCode, BuiltinDerivedObligation, ImplDerivedObligation};
use super::{SelectionError, Unimplemented, OutputTypeParameterMismatch}; use super::{SelectionError, Unimplemented, OutputTypeParameterMismatch};
@ -655,8 +655,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
let project_obligation = obligation.with(data.clone()); let project_obligation = obligation.with(data.clone());
match project::poly_project_and_unify_type(self, &project_obligation) { match project::poly_project_and_unify_type(self, &project_obligation) {
Ok(Some(subobligations)) => { Ok(Some(subobligations)) => {
self.evaluate_predicates_recursively(previous_stack, let result = self.evaluate_predicates_recursively(previous_stack,
subobligations.iter()) subobligations.iter());
if let Some(key) =
ProjectionCacheKey::from_poly_projection_predicate(self, data)
{
self.infcx.projection_cache.borrow_mut().complete(key);
}
result
} }
Ok(None) => { Ok(None) => {
EvaluatedToAmbig EvaluatedToAmbig