From 4ce959e0c62c75c730a24f1162260c3ced48dcdc Mon Sep 17 00:00:00 2001
From: lcnr <rust@lcnr.de>
Date: Wed, 2 Oct 2024 14:39:43 +0200
Subject: [PATCH] review

---
 .../src/infer/relate/type_relating.rs           |  6 ++++--
 compiler/rustc_middle/src/ty/fold.rs            | 17 +++++++++--------
 .../src/canonicalizer.rs                        | 12 +++++++-----
 compiler/rustc_next_trait_solver/src/resolve.rs |  2 ++
 .../src/solve/eval_ctxt/mod.rs                  | 15 ++++++++-------
 5 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/compiler/rustc_infer/src/infer/relate/type_relating.rs b/compiler/rustc_infer/src/infer/relate/type_relating.rs
index c8ced4a2cda85..0605a8d7dcc27 100644
--- a/compiler/rustc_infer/src/infer/relate/type_relating.rs
+++ b/compiler/rustc_infer/src/infer/relate/type_relating.rs
@@ -14,11 +14,13 @@ use crate::infer::{DefineOpaqueTypes, InferCtxt, SubregionOrigin};
 
 /// Enforce that `a` is equal to or a subtype of `b`.
 pub struct TypeRelating<'combine, 'a, 'tcx> {
-    // Partially mutable.
+    // Immutable except for the `InferCtxt` and the
+    // resulting nested `goals`.
     fields: &'combine mut CombineFields<'a, 'tcx>,
 
-    // Immutable fields.
+    // Immutable field.
     structurally_relate_aliases: StructurallyRelateAliases,
+    // Mutable field.
     ambient_variance: ty::Variance,
 
     /// The cache has only tracks the `ambient_variance` as its the
diff --git a/compiler/rustc_middle/src/ty/fold.rs b/compiler/rustc_middle/src/ty/fold.rs
index e152d3f5fbe9f..b5a77b3c9423e 100644
--- a/compiler/rustc_middle/src/ty/fold.rs
+++ b/compiler/rustc_middle/src/ty/fold.rs
@@ -210,16 +210,17 @@ where
                 debug_assert!(!ty.has_vars_bound_above(ty::INNERMOST));
                 ty::fold::shift_vars(self.tcx, ty, self.current_index.as_u32())
             }
-            _ if t.has_vars_bound_at_or_above(self.current_index) => {
-                if let Some(&ty) = self.cache.get(&(self.current_index, t)) {
-                    return ty;
+            _ => {
+                if !t.has_vars_bound_at_or_above(self.current_index) {
+                    t
+                } else if let Some(&t) = self.cache.get(&(self.current_index, t)) {
+                    t
+                } else {
+                    let res = t.super_fold_with(self);
+                    assert!(self.cache.insert((self.current_index, t), res));
+                    res
                 }
-
-                let res = t.super_fold_with(self);
-                assert!(self.cache.insert((self.current_index, t), res));
-                res
             }
-            _ => t,
         }
     }
 
diff --git a/compiler/rustc_next_trait_solver/src/canonicalizer.rs b/compiler/rustc_next_trait_solver/src/canonicalizer.rs
index 5c3c35b7b3e46..0bf9d7b924952 100644
--- a/compiler/rustc_next_trait_solver/src/canonicalizer.rs
+++ b/compiler/rustc_next_trait_solver/src/canonicalizer.rs
@@ -42,17 +42,19 @@ pub enum CanonicalizeMode {
 
 pub struct Canonicalizer<'a, D: SolverDelegate<Interner = I>, I: Interner> {
     delegate: &'a D,
+
+    // Immutable field.
     canonicalize_mode: CanonicalizeMode,
 
+    // Mutable fields.
     variables: &'a mut Vec<I::GenericArg>,
-    variable_lookup_table: HashMap<I::GenericArg, usize>,
-
     primitive_var_infos: Vec<CanonicalVarInfo<I>>,
+    variable_lookup_table: HashMap<I::GenericArg, usize>,
     binder_index: ty::DebruijnIndex,
 
-    /// We only use the debruijn index during lookup as all other fields
-    /// should not be impacted by whether a type is folded once or multiple
-    /// times.
+    /// We only use the debruijn index during lookup. We don't need to
+    /// track the `variables` as each generic arg only results in a single
+    /// bound variable regardless of how many times it is encountered.
     cache: HashMap<(ty::DebruijnIndex, I::Ty), I::Ty>,
 }
 
diff --git a/compiler/rustc_next_trait_solver/src/resolve.rs b/compiler/rustc_next_trait_solver/src/resolve.rs
index a37066cec669f..f2654f7534e1d 100644
--- a/compiler/rustc_next_trait_solver/src/resolve.rs
+++ b/compiler/rustc_next_trait_solver/src/resolve.rs
@@ -16,6 +16,8 @@ where
     I: Interner,
 {
     delegate: &'a D,
+    /// We're able to use a cache here as the folder does not have any
+    /// mutable state.
     cache: DelayedMap<I::Ty, I::Ty>,
 }
 
diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
index 12b4b3cb3a9d8..baa00465c0cce 100644
--- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
+++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
@@ -1057,16 +1057,17 @@ where
                 );
                 infer_ty
             }
-            _ if ty.has_aliases() => {
-                if let Some(&entry) = self.cache.get(&ty) {
+            _ => {
+                if !ty.has_aliases() {
+                    ty
+                } else if let Some(&entry) = self.cache.get(&ty) {
                     return entry;
+                } else {
+                    let res = ty.super_fold_with(self);
+                    assert!(self.cache.insert(ty, res).is_none());
+                    res
                 }
-
-                let res = ty.super_fold_with(self);
-                assert!(self.cache.insert(ty, res).is_none());
-                res
             }
-            _ => ty,
         }
     }