Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve most sub-obligations in the projection cache #85868

Merged
merged 2 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
71 changes: 24 additions & 47 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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))
}
Expand Down Expand Up @@ -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<X = U>` 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<X = ?1>` and `?1: Bar<X =
// ?0>`).
ty::PredicateKind::Projection(data) => {
infcx.unresolved_type_vars(&bound_predicate.rebind(data.ty)).is_some()
}

// We are only interested in `T: Foo<X = U>` 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 `<T as Trait>::Item`, but `T: Trait` does not
/// hold. In various error cases, we cannot generate a valid
/// normalized projection. Therefore, we create an inference variable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
});

debug!(?result);
debug!("finished: {:?} from {:?}", result, obligation);

result
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
12 changes: 6 additions & 6 deletions src/test/ui/impl-trait/auto-trait-leak.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/traits/cycle-cache-err-60010.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct Runtime<DB: Database> {
}
struct SalsaStorage {
_parse: <ParseQuery as Query<RootDatabase>>::Data,
//~^ ERROR overflow
}

impl Database for RootDatabase {
Expand Down Expand Up @@ -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
}

Expand Down
16 changes: 8 additions & 8 deletions src/test/ui/traits/cycle-cache-err-60010.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0275]: overflow evaluating the requirement `SalsaStorage: RefUnwindSafe`
--> $DIR/cycle-cache-err-60010.rs:69:5
--> $DIR/cycle-cache-err-60010.rs:27:13
|
LL | SourceDatabase::parse(db);
| ^^^^^^^^^^^^^^^^^^^^^
LL | _parse: <ParseQuery as Query<RootDatabase>>::Data,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: required because it appears within the type `*const SalsaStorage`
= note: required because it appears within the type `Unique<SalsaStorage>`
Expand All @@ -18,15 +18,15 @@ note: required because it appears within the type `RootDatabase`
LL | struct RootDatabase {
| ^^^^^^^^^^^^
note: required because of the requirements on the impl of `SourceDatabase` for `RootDatabase`
--> $DIR/cycle-cache-err-60010.rs:43:9
--> $DIR/cycle-cache-err-60010.rs:44:9
|
LL | impl<T> SourceDatabase for T
| ^^^^^^^^^^^^^^ ^
note: required by `SourceDatabase::parse`
--> $DIR/cycle-cache-err-60010.rs:14:5
note: required because of the requirements on the impl of `Query<RootDatabase>` for `ParseQuery`
--> $DIR/cycle-cache-err-60010.rs:37:10
|
LL | fn parse(&self) {
| ^^^^^^^^^^^^^^^
LL | impl<DB> Query<DB> for ParseQuery
| ^^^^^^^^^ ^^^^^^^^^^

error: aborting due to previous error

Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/traits/inductive-overflow/lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ impl<'a> Y for C<'a> {
struct C<'a>(&'a ());
struct X<T: Y>(T::P);

impl<T: NotAuto> NotAuto for Box<T> {}
impl<T: Y> NotAuto for X<T> where T::P: NotAuto {} //~ NOTE: required
impl<T: NotAuto> NotAuto for Box<T> {} //~ NOTE: required
impl<T: Y> NotAuto for X<T> where T::P: NotAuto {}
impl<'a> NotAuto for C<'a> {}

fn is_send<S: NotAuto>() {}
Expand All @@ -26,6 +26,6 @@ fn main() {
// Should only be a few notes.
is_send::<X<C<'static>>>();
//~^ ERROR overflow evaluating
//~| 2 redundant requirements hidden
//~| 3 redundant requirements hidden
//~| required because of
}
12 changes: 6 additions & 6 deletions src/test/ui/traits/inductive-overflow/lifetime.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
error[E0275]: overflow evaluating the requirement `Box<X<C<'_>>>: NotAuto`
error[E0275]: overflow evaluating the requirement `X<C<'_>>: NotAuto`
--> $DIR/lifetime.rs:27:5
|
LL | is_send::<X<C<'static>>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
note: required because of the requirements on the impl of `NotAuto` for `X<C<'_>>`
--> $DIR/lifetime.rs:19:12
note: required because of the requirements on the impl of `NotAuto` for `Box<X<C<'_>>>`
--> $DIR/lifetime.rs:18:18
|
LL | impl<T: Y> NotAuto for X<T> where T::P: NotAuto {}
| ^^^^^^^ ^^^^
= note: 2 redundant requirements hidden
LL | impl<T: NotAuto> NotAuto for Box<T> {}
| ^^^^^^^ ^^^^^^
= note: 3 redundant requirements hidden
= note: required because of the requirements on the impl of `NotAuto` for `X<C<'static>>`
note: required by a bound in `is_send`
--> $DIR/lifetime.rs:22:15
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/traits/inductive-overflow/simultaneous.stderr
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/type-alias-impl-trait/auto-trait-leakage3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/type-alias-impl-trait/inference-cycle.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down