From f2d9ee9c342104880dd978e85260243d2dcedc9a Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 31 May 2021 13:22:40 -0500 Subject: [PATCH 1/2] 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 From 611191f54c563587a9130b8cb4afba1856aebebc Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 27 Aug 2021 17:34:48 -0500 Subject: [PATCH 2/2] Report cycle error using 'deepest' obligation in the cycle --- .../src/traits/error_reporting/mod.rs | 4 +++- .../const-generics/hash-tyvid-regression-1.rs | 3 ++- src/test/ui/traits/cycle-cache-err-60010.rs | 2 +- .../ui/traits/cycle-cache-err-60010.stderr | 20 ++++++++++++++++++- .../ui/traits/inductive-overflow/lifetime.rs | 6 +++--- .../traits/inductive-overflow/lifetime.stderr | 12 +++++------ .../inductive-overflow/simultaneous.stderr | 2 +- .../auto-trait-leakage3.stderr | 6 +++--- .../inference-cycle.stderr | 6 +++--- 9 files changed, 41 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 9fd5cb2a0b33c..254ada7cf1ffb 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -224,7 +224,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { debug!("report_overflow_error_cycle: cycle={:?}", cycle); - self.report_overflow_error(&cycle[0], false); + // The 'deepest' obligation is most likely to have a useful + // cause 'backtrace' + self.report_overflow_error(cycle.iter().max_by_key(|p| p.recursion_depth).unwrap(), false); } fn report_selection_error( diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-1.rs b/src/test/incremental/const-generics/hash-tyvid-regression-1.rs index b5a0108a0a397..5ff7b19d8945e 100644 --- a/src/test/incremental/const-generics/hash-tyvid-regression-1.rs +++ b/src/test/incremental/const-generics/hash-tyvid-regression-1.rs @@ -9,7 +9,8 @@ where use std::convert::TryFrom; <[T; N.get()]>::try_from(()) //~^ error: the trait bound - //~^^ error: mismatched types + //~| error: the trait bound + //~| error: mismatched types } fn main() {} diff --git a/src/test/ui/traits/cycle-cache-err-60010.rs b/src/test/ui/traits/cycle-cache-err-60010.rs index 88f0bd872535f..94e718317e7c8 100644 --- a/src/test/ui/traits/cycle-cache-err-60010.rs +++ b/src/test/ui/traits/cycle-cache-err-60010.rs @@ -25,7 +25,7 @@ struct Runtime { } struct SalsaStorage { _parse: >::Data, - //~^ ERROR overflow evaluating the requirement `RootDatabase: SourceDatabase` + //~^ ERROR overflow } impl Database for RootDatabase { diff --git a/src/test/ui/traits/cycle-cache-err-60010.stderr b/src/test/ui/traits/cycle-cache-err-60010.stderr index 91c2bd6c3b225..9452e11e302e3 100644 --- a/src/test/ui/traits/cycle-cache-err-60010.stderr +++ b/src/test/ui/traits/cycle-cache-err-60010.stderr @@ -1,9 +1,27 @@ -error[E0275]: overflow evaluating the requirement `RootDatabase: SourceDatabase` +error[E0275]: overflow evaluating the requirement `SalsaStorage: RefUnwindSafe` --> $DIR/cycle-cache-err-60010.rs:27:13 | 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 + | +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:44:9 + | +LL | impl SourceDatabase for T + | ^^^^^^^^^^^^^^ ^ note: required because of the requirements on the impl of `Query` for `ParseQuery` --> $DIR/cycle-cache-err-60010.rs:37:10 | diff --git a/src/test/ui/traits/inductive-overflow/lifetime.rs b/src/test/ui/traits/inductive-overflow/lifetime.rs index 072c46bc21e9a..8be42fc4ad07f 100644 --- a/src/test/ui/traits/inductive-overflow/lifetime.rs +++ b/src/test/ui/traits/inductive-overflow/lifetime.rs @@ -15,8 +15,8 @@ impl<'a> Y for C<'a> { struct C<'a>(&'a ()); struct X(T::P); -impl NotAuto for Box {} -impl NotAuto for X where T::P: NotAuto {} //~ NOTE: required +impl NotAuto for Box {} //~ NOTE: required +impl NotAuto for X where T::P: NotAuto {} impl<'a> NotAuto for C<'a> {} fn is_send() {} @@ -26,6 +26,6 @@ fn main() { // Should only be a few notes. is_send::>>(); //~^ ERROR overflow evaluating - //~| 2 redundant requirements hidden + //~| 3 redundant requirements hidden //~| required because of } diff --git a/src/test/ui/traits/inductive-overflow/lifetime.stderr b/src/test/ui/traits/inductive-overflow/lifetime.stderr index 2905deb940f3a..2ffcdb0e1c6de 100644 --- a/src/test/ui/traits/inductive-overflow/lifetime.stderr +++ b/src/test/ui/traits/inductive-overflow/lifetime.stderr @@ -1,15 +1,15 @@ -error[E0275]: overflow evaluating the requirement `Box>>: NotAuto` +error[E0275]: overflow evaluating the requirement `X>: NotAuto` --> $DIR/lifetime.rs:27:5 | LL | is_send::>>(); | ^^^^^^^^^^^^^^^^^^^^^^^^ | -note: required because of the requirements on the impl of `NotAuto` for `X>` - --> $DIR/lifetime.rs:19:12 +note: required because of the requirements on the impl of `NotAuto` for `Box>>` + --> $DIR/lifetime.rs:18:18 | -LL | impl NotAuto for X where T::P: NotAuto {} - | ^^^^^^^ ^^^^ - = note: 2 redundant requirements hidden +LL | impl NotAuto for Box {} + | ^^^^^^^ ^^^^^^ + = note: 3 redundant requirements hidden = note: required because of the requirements on the impl of `NotAuto` for `X>` note: required by a bound in `is_send` --> $DIR/lifetime.rs:22:15 diff --git a/src/test/ui/traits/inductive-overflow/simultaneous.stderr b/src/test/ui/traits/inductive-overflow/simultaneous.stderr index 7eb1c9ffb3b49..230c2638c5003 100644 --- a/src/test/ui/traits/inductive-overflow/simultaneous.stderr +++ b/src/test/ui/traits/inductive-overflow/simultaneous.stderr @@ -1,4 +1,4 @@ -error[E0275]: overflow evaluating the requirement `{integer}: Tweedledee` +error[E0275]: overflow evaluating the requirement `{integer}: Tweedledum` --> $DIR/simultaneous.rs:18:5 | LL | is_ee(4); diff --git a/src/test/ui/type-alias-impl-trait/auto-trait-leakage3.stderr b/src/test/ui/type-alias-impl-trait/auto-trait-leakage3.stderr index ac7bbd272c771..86b3f87d34d03 100644 --- a/src/test/ui/type-alias-impl-trait/auto-trait-leakage3.stderr +++ b/src/test/ui/type-alias-impl-trait/auto-trait-leakage3.stderr @@ -5,10 +5,10 @@ LL | type Foo = impl std::fmt::Debug; | ^^^^^^^^^^^^^^^^^^^^ | note: ...which requires type-checking `m::bar`... - --> $DIR/auto-trait-leakage3.rs:14:5 + --> $DIR/auto-trait-leakage3.rs:15:9 | -LL | pub fn bar() { - | ^^^^^^^^^^^^ +LL | is_send(foo()); + | ^^^^^^^ = note: ...which requires evaluating trait selection obligation `impl std::fmt::Debug: std::marker::Send`... = note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle note: cycle used when checking item types in module `m` diff --git a/src/test/ui/type-alias-impl-trait/inference-cycle.stderr b/src/test/ui/type-alias-impl-trait/inference-cycle.stderr index ac0ca8e048cb0..4c5921c7f660b 100644 --- a/src/test/ui/type-alias-impl-trait/inference-cycle.stderr +++ b/src/test/ui/type-alias-impl-trait/inference-cycle.stderr @@ -5,10 +5,10 @@ LL | type Foo = impl std::fmt::Debug; | ^^^^^^^^^^^^^^^^^^^^ | note: ...which requires type-checking `m::bar`... - --> $DIR/inference-cycle.rs:14:5 + --> $DIR/inference-cycle.rs:15:9 | -LL | pub fn bar() { - | ^^^^^^^^^^^^ +LL | is_send(foo()); // Today: error + | ^^^^^^^ = note: ...which requires evaluating trait selection obligation `impl std::fmt::Debug: std::marker::Send`... = note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle note: cycle used when checking item types in module `m`