From f2d9ee9c342104880dd978e85260243d2dcedc9a Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 31 May 2021 13:22:40 -0500 Subject: [PATCH] Preserve most sub-obligations in the projection cache --- .../src/traits/project.rs | 71 +++++++------------ .../src/traits/query/evaluate_obligation.rs | 2 +- .../src/traits/select/mod.rs | 2 +- src/test/ui/impl-trait/auto-trait-leak.stderr | 12 ++-- src/test/ui/traits/cycle-cache-err-60010.rs | 2 +- .../ui/traits/cycle-cache-err-60010.stderr | 34 +++------ 6 files changed, 41 insertions(+), 82 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 91b9ad0af356c..7038f16a2c9c4 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -10,6 +10,7 @@ use super::PredicateObligation; use super::Selection; use super::SelectionContext; use super::SelectionError; +use super::TraitQueryMode; use super::{ ImplSourceClosureData, ImplSourceDiscriminantKindData, ImplSourceFnPointerData, ImplSourceGeneratorData, ImplSourcePointeeData, ImplSourceUserDefinedData, @@ -18,7 +19,7 @@ use super::{Normalized, NormalizedTy, ProjectionCacheEntry, ProjectionCacheKey}; use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use crate::infer::{InferCtxt, InferOk, LateBoundRegionConversionTime}; -use crate::traits::error_reporting::InferCtxtExt; +use crate::traits::error_reporting::InferCtxtExt as _; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::ErrorReported; use rustc_hir::def_id::DefId; @@ -912,6 +913,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( } let obligation = Obligation::with_depth(cause.clone(), depth, param_env, projection_ty); + match project_type(selcx, &obligation) { Ok(ProjectedTy::Progress(Progress { ty: projected_ty, @@ -925,7 +927,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( let projected_ty = selcx.infcx().resolve_vars_if_possible(projected_ty); debug!(?projected_ty, ?depth, ?projected_obligations); - let result = if projected_ty.has_projections() { + let mut result = if projected_ty.has_projections() { let mut normalizer = AssocTypeNormalizer::new( selcx, param_env, @@ -942,8 +944,26 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( Normalized { value: projected_ty, obligations: projected_obligations } }; - let cache_value = prune_cache_value_obligations(infcx, &result); - infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, cache_value); + let mut canonical = + SelectionContext::with_query_mode(selcx.infcx(), TraitQueryMode::Canonical); + result.obligations.drain_filter(|projected_obligation| { + // If any global obligations always apply, considering regions, then we don't + // need to include them. The `is_global` check rules out inference variables, + // so there's no need for the caller of `opt_normalize_projection_type` + // to evaluate them. + // Note that we do *not* discard obligations that evaluate to + // `EvaluatedtoOkModuloRegions`. Evaluating these obligations + // inside of a query (e.g. `evaluate_obligation`) can change + // the result to `EvaluatedToOkModuloRegions`, while an + // `EvaluatedToOk` obligation will never change the result. + // See #85360 for more details + projected_obligation.is_global(canonical.tcx()) + && canonical + .evaluate_root_obligation(projected_obligation) + .map_or(false, |res| res.must_apply_considering_regions()) + }); + + infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, result.clone()); obligations.extend(result.obligations); Ok(Some(result.value)) } @@ -974,49 +994,6 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( } } -/// If there are unresolved type variables, then we need to include -/// any subobligations that bind them, at least until those type -/// variables are fully resolved. -fn prune_cache_value_obligations<'a, 'tcx>( - infcx: &'a InferCtxt<'a, 'tcx>, - result: &NormalizedTy<'tcx>, -) -> NormalizedTy<'tcx> { - if infcx.unresolved_type_vars(&result.value).is_none() { - return NormalizedTy { value: result.value, obligations: vec![] }; - } - - let mut obligations: Vec<_> = result - .obligations - .iter() - .filter(|obligation| { - let bound_predicate = obligation.predicate.kind(); - match bound_predicate.skip_binder() { - // We found a `T: Foo` predicate, let's check - // if `U` references any unresolved type - // variables. In principle, we only care if this - // projection can help resolve any of the type - // variables found in `result.value` -- but we just - // check for any type variables here, for fear of - // indirect obligations (e.g., we project to `?0`, - // but we have `T: Foo` and `?1: Bar`). - ty::PredicateKind::Projection(data) => { - infcx.unresolved_type_vars(&bound_predicate.rebind(data.ty)).is_some() - } - - // We are only interested in `T: Foo` predicates, whre - // `U` references one of `unresolved_type_vars`. =) - _ => false, - } - }) - .cloned() - .collect(); - - obligations.shrink_to_fit(); - - NormalizedTy { value: result.value, obligations } -} - /// If we are projecting `::Item`, but `T: Trait` does not /// hold. In various error cases, we cannot generate a valid /// normalized projection. Therefore, we create an inference variable diff --git a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs index 2dc48e47efccd..032d402fec045 100644 --- a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs +++ b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs @@ -71,7 +71,7 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { // Run canonical query. If overflow occurs, rerun from scratch but this time // in standard trait query mode so that overflow is handled appropriately // within `SelectionContext`. - self.tcx.evaluate_obligation(c_pred) + self.tcx.at(obligation.cause.span(self.tcx)).evaluate_obligation(c_pred) } // Helper function that canonicalizes and runs the query. If an diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 5214277a37d53..22013fb79cf79 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -682,7 +682,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } }); - debug!(?result); + debug!("finished: {:?} from {:?}", result, obligation); result } diff --git a/src/test/ui/impl-trait/auto-trait-leak.stderr b/src/test/ui/impl-trait/auto-trait-leak.stderr index 3eb141cc2bb55..a31c104d8f58b 100644 --- a/src/test/ui/impl-trait/auto-trait-leak.stderr +++ b/src/test/ui/impl-trait/auto-trait-leak.stderr @@ -30,10 +30,10 @@ note: ...which requires building MIR for `cycle1`... LL | fn cycle1() -> impl Clone { | ^^^^^^^^^^^^^^^^^^^^^^^^^ note: ...which requires type-checking `cycle1`... - --> $DIR/auto-trait-leak.rs:12:1 + --> $DIR/auto-trait-leak.rs:14:5 | -LL | fn cycle1() -> impl Clone { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | send(cycle2().clone()); + | ^^^^ = note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`... note: ...which requires computing type of `cycle2::{opaque#0}`... --> $DIR/auto-trait-leak.rs:19:16 @@ -66,10 +66,10 @@ note: ...which requires building MIR for `cycle2`... LL | fn cycle2() -> impl Clone { | ^^^^^^^^^^^^^^^^^^^^^^^^^ note: ...which requires type-checking `cycle2`... - --> $DIR/auto-trait-leak.rs:19:1 + --> $DIR/auto-trait-leak.rs:20:5 | -LL | fn cycle2() -> impl Clone { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | send(cycle1().clone()); + | ^^^^ = note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`... = note: ...which again requires computing type of `cycle1::{opaque#0}`, completing the cycle note: cycle used when checking item types in top-level module diff --git a/src/test/ui/traits/cycle-cache-err-60010.rs b/src/test/ui/traits/cycle-cache-err-60010.rs index 98bfcb8d67b51..88f0bd872535f 100644 --- a/src/test/ui/traits/cycle-cache-err-60010.rs +++ b/src/test/ui/traits/cycle-cache-err-60010.rs @@ -25,6 +25,7 @@ struct Runtime { } struct SalsaStorage { _parse: >::Data, + //~^ ERROR overflow evaluating the requirement `RootDatabase: SourceDatabase` } impl Database for RootDatabase { @@ -67,7 +68,6 @@ pub(crate) fn goto_implementation(db: &RootDatabase) -> u32 { // we used to fail to report an error here because we got the // caching wrong. SourceDatabase::parse(db); - //~^ ERROR overflow 22 } diff --git a/src/test/ui/traits/cycle-cache-err-60010.stderr b/src/test/ui/traits/cycle-cache-err-60010.stderr index 565899677bf1a..91c2bd6c3b225 100644 --- a/src/test/ui/traits/cycle-cache-err-60010.stderr +++ b/src/test/ui/traits/cycle-cache-err-60010.stderr @@ -1,32 +1,14 @@ -error[E0275]: overflow evaluating the requirement `SalsaStorage: RefUnwindSafe` - --> $DIR/cycle-cache-err-60010.rs:69:5 +error[E0275]: overflow evaluating the requirement `RootDatabase: SourceDatabase` + --> $DIR/cycle-cache-err-60010.rs:27:13 | -LL | SourceDatabase::parse(db); - | ^^^^^^^^^^^^^^^^^^^^^ +LL | _parse: >::Data, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: required because it appears within the type `*const SalsaStorage` - = note: required because it appears within the type `Unique` - = note: required because it appears within the type `Box` -note: required because it appears within the type `Runtime` - --> $DIR/cycle-cache-err-60010.rs:23:8 +note: required because of the requirements on the impl of `Query` for `ParseQuery` + --> $DIR/cycle-cache-err-60010.rs:37:10 | -LL | struct Runtime { - | ^^^^^^^ -note: required because it appears within the type `RootDatabase` - --> $DIR/cycle-cache-err-60010.rs:20:8 - | -LL | struct RootDatabase { - | ^^^^^^^^^^^^ -note: required because of the requirements on the impl of `SourceDatabase` for `RootDatabase` - --> $DIR/cycle-cache-err-60010.rs:43:9 - | -LL | impl SourceDatabase for T - | ^^^^^^^^^^^^^^ ^ -note: required by `SourceDatabase::parse` - --> $DIR/cycle-cache-err-60010.rs:14:5 - | -LL | fn parse(&self) { - | ^^^^^^^^^^^^^^^ +LL | impl Query for ParseQuery + | ^^^^^^^^^ ^^^^^^^^^^ error: aborting due to previous error