about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-11-06 21:22:14 +0000
committerbors <bors@rust-lang.org>2024-11-06 21:22:14 +0000
commitc07aa1e17199e27f6f7ac8f3307bca7bda0084b6 (patch)
treeb05c5cf7e7d316ac5796300302791cf4099e0a69
parent8549802939cd01111c46e34a7b67cb1933977af9 (diff)
parent49153739fd01d82ed999c763fd2771cb837d7dd2 (diff)
downloadrust-c07aa1e17199e27f6f7ac8f3307bca7bda0084b6.tar.gz
rust-c07aa1e17199e27f6f7ac8f3307bca7bda0084b6.zip
Auto merge of #132625 - compiler-errors:cache-only-if-opaque, r=lcnr
Only disable cache if predicate has opaques within it

This is an alternative to https://github.com/rust-lang/rust/pull/132075.

This refines the check implemented in https://github.com/rust-lang/rust/pull/126024 to only disable the global cache if the predicate being considered has opaques in it. This is still theoretically unsound, since goals can indirectly rely on opaques in the defining scope, but we're much less likely to hit it.

It doesn't totally fix https://github.com/rust-lang/rust/issues/132064: for example, `lemmy` goes from 1:29 (on rust 1.81) to 9:53 (on nightly) to 4:07 (after this PR). But I think it's at least *more* sound than a total revert :/

r? lcnr
-rw-r--r--compiler/rustc_trait_selection/src/traits/select/mod.rs24
1 files changed, 18 insertions, 6 deletions
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index 711480252ec..cffb62ab559 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -1310,7 +1310,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         trait_pred: ty::PolyTraitPredicate<'tcx>,
     ) -> Option<EvaluationResult> {
         let tcx = self.tcx();
-        if self.can_use_global_caches(param_env) {
+        if self.can_use_global_caches(param_env, trait_pred) {
             if let Some(res) = tcx.evaluation_cache.get(&(param_env, trait_pred), tcx) {
                 return Some(res);
             }
@@ -1331,7 +1331,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return;
         }
 
-        if self.can_use_global_caches(param_env) && !trait_pred.has_infer() {
+        if self.can_use_global_caches(param_env, trait_pred) && !trait_pred.has_infer() {
             debug!(?trait_pred, ?result, "insert_evaluation_cache global");
             // This may overwrite the cache with the same value
             // FIXME: Due to #50507 this overwrites the different values
@@ -1476,7 +1476,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     }
 
     /// Returns `true` if the global caches can be used.
-    fn can_use_global_caches(&self, param_env: ty::ParamEnv<'tcx>) -> bool {
+    fn can_use_global_caches(
+        &self,
+        param_env: ty::ParamEnv<'tcx>,
+        pred: ty::PolyTraitPredicate<'tcx>,
+    ) -> bool {
         // If there are any inference variables in the `ParamEnv`, then we
         // always use a cache local to this particular scope. Otherwise, we
         // switch to a global cache.
@@ -1494,7 +1498,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             TypingMode::Coherence => false,
             // Avoid using the global cache when we're defining opaque types
             // as their hidden type may impact the result of candidate selection.
-            TypingMode::Analysis { defining_opaque_types } => defining_opaque_types.is_empty(),
+            //
+            // HACK: This is still theoretically unsound. Goals can indirectly rely
+            // on opaques in the defining scope, and it's easier to do so with TAIT.
+            // However, if we disqualify *all* goals from being cached, perf suffers.
+            // This is likely fixed by better caching in general in the new solver.
+            // See: <https://github.com/rust-lang/rust/issues/132064>.
+            TypingMode::Analysis { defining_opaque_types } => {
+                defining_opaque_types.is_empty() || !pred.has_opaque_types()
+            }
             // The global cache is only used if there are no opaque types in
             // the defining scope or we're outside of analysis.
             //
@@ -1512,7 +1524,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         let tcx = self.tcx();
         let pred = cache_fresh_trait_pred.skip_binder();
 
-        if self.can_use_global_caches(param_env) {
+        if self.can_use_global_caches(param_env, cache_fresh_trait_pred) {
             if let Some(res) = tcx.selection_cache.get(&(param_env, pred), tcx) {
                 return Some(res);
             }
@@ -1562,7 +1574,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return;
         }
 
-        if self.can_use_global_caches(param_env) {
+        if self.can_use_global_caches(param_env, cache_fresh_trait_pred) {
             if let Err(Overflow(OverflowError::Canonical)) = candidate {
                 // Don't cache overflow globally; we only produce this in certain modes.
             } else if !pred.has_infer() && !candidate.has_infer() {