Auto merge of #85868 - Aaron1011:projection-cache, r=jackh726

Preserve most sub-obligations in the projection cache

Fixes https://github.com/rust-lang/rust/issues/85360

When we evaluate a projection predicate, we may produce sub-obligations. During trait evaluation, evaluating these sub-obligations might cause us to produce `EvaluatedToOkModuloRegions`.

When we cache the result of projection in our projection cache, we try to throw away some of the sub-obligations, so that we don't need to re-evaluate/process them the next time we need to perform this particular projection. However, we may end up throwing away predicates that will (recursively) evaluate to `EvaluatedToOkModuloRegions`. If we do, then the result of evaluating a predicate will depend on the state of the predicate cache - this is global untracked state, which interacts badly with incremental compilation.

To fix this, we now only discard global predicates that evaluate to `EvaluatedToOk`. This ensures that any predicates that (may) evaluate to `EvaluatedToOkModuloRegions` are kept in the cache, and influence the results of any queries which perform this projection.

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -224,7 +224,9 @@ fn report_overflow_error_cycle(&self, cycle: &[PredicateObligation<'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(

compiler/rustc_trait_selection/src/traits/project.rs

@@ -10,6 +10,7 @@
 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 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<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

compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs

@@ -71,7 +71,7 @@ fn evaluate_obligation(
         // 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

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -682,7 +682,7 @@ fn evaluate_predicate_recursively<'o>(
             }
         });
 
-        debug!(?result);
+        debug!("finished: {:?} from {:?}", result, obligation);
 
         result
     }

src/test/incremental/const-generics/hash-tyvid-regression-1.rs

@@ -9,7 +9,8 @@ fn c<T, const N: std::num::NonZeroUsize>()
     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() {}

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

src/test/ui/traits/cycle-cache-err-60010.rs

@@ -25,6 +25,7 @@ struct Runtime<DB: Database> {
 }
 struct SalsaStorage {
     _parse: <ParseQuery as Query<RootDatabase>>::Data,
+    //~^ ERROR overflow
 }
 
 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
 }
 

src/test/ui/traits/cycle-cache-err-60010.stderr

 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>`
@@ -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
 

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: 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>() {}
@@ -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
 }

src/test/ui/traits/inductive-overflow/lifetime.stderr

-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

src/test/ui/traits/inductive-overflow/simultaneous.stderr

-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);

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`

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`