From 8588f798029758c4d412757ff5d31c79ce5c02bf Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Wed, 16 Mar 2022 23:05:52 -0700
Subject: [PATCH] Do not use ParamEnv::and to cache param-env with candidate

---
 compiler/rustc_middle/src/traits/select.rs       | 13 ++++++++++---
 .../src/traits/select/mod.rs                     | 16 ++++++++--------
 .../normalize-under-binder/issue-80706.rs        |  3 ++-
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/compiler/rustc_middle/src/traits/select.rs b/compiler/rustc_middle/src/traits/select.rs
index 5297825a92f..ffa70cddbd5 100644
--- a/compiler/rustc_middle/src/traits/select.rs
+++ b/compiler/rustc_middle/src/traits/select.rs
@@ -13,12 +13,19 @@
 use rustc_query_system::cache::Cache;
 
 pub type SelectionCache<'tcx> = Cache<
-    ty::ParamEnvAnd<'tcx, ty::TraitPredicate<'tcx>>,
+    // This cache does not use `ParamEnvAnd` in its keys because `ParamEnv::and` can replace
+    // caller bounds with an empty list if the `TraitPredicate` looks global, which may happen
+    // after erasing lifetimes from the predicate.
+    (ty::ParamEnv<'tcx>, ty::TraitPredicate<'tcx>),
     SelectionResult<'tcx, SelectionCandidate<'tcx>>,
 >;
 
-pub type EvaluationCache<'tcx> =
-    Cache<ty::ParamEnvAnd<'tcx, ty::PolyTraitPredicate<'tcx>>, EvaluationResult>;
+pub type EvaluationCache<'tcx> = Cache<
+    // See above: this cache does not use `ParamEnvAnd` in its keys due to sometimes incorrectly
+    // caching with the wrong `ParamEnv`.
+    (ty::ParamEnv<'tcx>, ty::PolyTraitPredicate<'tcx>),
+    EvaluationResult,
+>;
 
 /// The selection process begins by considering all impls, where
 /// clauses, and so forth that might resolve an obligation. Sometimes
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index 6d232d86d8a..ad4eb6e21a6 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -1046,11 +1046,11 @@ fn check_evaluation_cache(
 
         let tcx = self.tcx();
         if self.can_use_global_caches(param_env) {
-            if let Some(res) = tcx.evaluation_cache.get(&param_env.and(trait_pred), tcx) {
+            if let Some(res) = tcx.evaluation_cache.get(&(param_env, trait_pred), tcx) {
                 return Some(res);
             }
         }
-        self.infcx.evaluation_cache.get(&param_env.and(trait_pred), tcx)
+        self.infcx.evaluation_cache.get(&(param_env, trait_pred), tcx)
     }
 
     fn insert_evaluation_cache(
@@ -1081,13 +1081,13 @@ fn insert_evaluation_cache(
                 // FIXME: Due to #50507 this overwrites the different values
                 // This should be changed to use HashMapExt::insert_same
                 // when that is fixed
-                self.tcx().evaluation_cache.insert(param_env.and(trait_pred), dep_node, result);
+                self.tcx().evaluation_cache.insert((param_env, trait_pred), dep_node, result);
                 return;
             }
         }
 
         debug!(?trait_pred, ?result, "insert_evaluation_cache");
-        self.infcx.evaluation_cache.insert(param_env.and(trait_pred), dep_node, result);
+        self.infcx.evaluation_cache.insert((param_env, trait_pred), dep_node, result);
     }
 
     /// For various reasons, it's possible for a subobligation
@@ -1297,11 +1297,11 @@ fn check_candidate_cache(
         pred.remap_constness(tcx, &mut param_env);
 
         if self.can_use_global_caches(param_env) {
-            if let Some(res) = tcx.selection_cache.get(&param_env.and(pred), tcx) {
+            if let Some(res) = tcx.selection_cache.get(&(param_env, pred), tcx) {
                 return Some(res);
             }
         }
-        self.infcx.selection_cache.get(&param_env.and(pred), tcx)
+        self.infcx.selection_cache.get(&(param_env, pred), tcx)
     }
 
     /// Determines whether can we safely cache the result
@@ -1361,14 +1361,14 @@ fn insert_candidate_cache(
                 if !candidate.needs_infer() {
                     debug!(?pred, ?candidate, "insert_candidate_cache global");
                     // This may overwrite the cache with the same value.
-                    tcx.selection_cache.insert(param_env.and(pred), dep_node, candidate);
+                    tcx.selection_cache.insert((param_env, pred), dep_node, candidate);
                     return;
                 }
             }
         }
 
         debug!(?pred, ?candidate, "insert_candidate_cache local");
-        self.infcx.selection_cache.insert(param_env.and(pred), dep_node, candidate);
+        self.infcx.selection_cache.insert((param_env, pred), dep_node, candidate);
     }
 
     /// Matches a predicate against the bounds of its self type.
diff --git a/src/test/ui/higher-rank-trait-bounds/normalize-under-binder/issue-80706.rs b/src/test/ui/higher-rank-trait-bounds/normalize-under-binder/issue-80706.rs
index 112227c85c4..00a866f220b 100644
--- a/src/test/ui/higher-rank-trait-bounds/normalize-under-binder/issue-80706.rs
+++ b/src/test/ui/higher-rank-trait-bounds/normalize-under-binder/issue-80706.rs
@@ -1,4 +1,4 @@
-// check-pass
+// build-pass
 // edition:2018
 
 type BoxFuture<T> = std::pin::Pin<Box<dyn std::future::Future<Output=T>>>;
@@ -65,6 +65,7 @@ async fn run<S>(dep: &str)
 where
     S: Storage,
     for<'a> SaveUser<'a>: StorageRequest<S>,
+    for<'a> SaveUser<'a>: StorageRequestReturnType,
 {
     User { dep }.save().await;
 }