review

lcnr · lcnr · commit 1a04a317c447 · 2024-10-02T14:49:36.000+02:00
diff --git a/compiler/rustc_infer/src/infer/relate/type_relating.rs b/compiler/rustc_infer/src/infer/relate/type_relating.rs
@@ -14,14 +14,16 @@ 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
+    /// The cache only tracks the `ambient_variance` as it's the
     /// only field which is mutable and which meaningfully changes
     /// the result when relating types.
     ///
diff --git 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
@@ -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
@@ -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
@@ -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,
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -210,16 +210,17 @@ where`
`210`	`210`	`debug_assert!(!ty.has_vars_bound_above(ty::INNERMOST));`
`211`	`211`	`ty::fold::shift_vars(self.tcx, ty, self.current_index.as_u32())`
`212`	`212`	`}`
`213`		`- _ if t.has_vars_bound_at_or_above(self.current_index) => {`
`214`		`- if let Some(&ty) = self.cache.get(&(self.current_index, t)) {`
`215`		`- return ty;`
	`213`	`+ _ => {`
	`214`	`+ if !t.has_vars_bound_at_or_above(self.current_index) {`
	`215`	`+ t`
	`216`	`+ } else if let Some(&t) = self.cache.get(&(self.current_index, t)) {`
	`217`	`+ t`
	`218`	`+ } else {`
	`219`	`+ let res = t.super_fold_with(self);`
	`220`	`+ assert!(self.cache.insert((self.current_index, t), res));`
	`221`	`+ res`
`216`	`222`	`}`
`217`		`-`
`218`		`- let res = t.super_fold_with(self);`
`219`		`- assert!(self.cache.insert((self.current_index, t), res));`
`220`		`- res`
`221`	`223`	`}`
`222`		`- _ => t,`
`223`	`224`	`}`
`224`	`225`	`}`
`225`	`226`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,8 @@ where`
`16`	`16`	`I: Interner,`
`17`	`17`	`{`
`18`	`18`	`delegate: &'a D,`
	`19`	`+ /// We're able to use a cache here as the folder does not have any`
	`20`	`+ /// mutable state.`
`19`	`21`	`cache: DelayedMap<I::Ty, I::Ty>,`
`20`	`22`	`}`
`21`	`23`
Original file line number	Diff line number	Diff line change
`@@ -1057,16 +1057,17 @@ where`
`1057`	`1057`	`);`
`1058`	`1058`	`infer_ty`
`1059`	`1059`	`}`
`1060`		`- _ if ty.has_aliases() => {`
`1061`		`- if let Some(&entry) = self.cache.get(&ty) {`
	`1060`	`+ _ => {`
	`1061`	`+ if !ty.has_aliases() {`
	`1062`	`+ ty`
	`1063`	`+ } else if let Some(&entry) = self.cache.get(&ty) {`
`1062`	`1064`	`return entry;`
	`1065`	`+ } else {`
	`1066`	`+ let res = ty.super_fold_with(self);`
	`1067`	`+ assert!(self.cache.insert(ty, res).is_none());`
	`1068`	`+ res`
`1063`	`1069`	`}`
`1064`		`-`
`1065`		`- let res = ty.super_fold_with(self);`
`1066`		`- assert!(self.cache.insert(ty, res).is_none());`
`1067`		`- res`
`1068`	`1070`	`}`
`1069`		`- _ => ty,`
`1070`	`1071`	`}`
`1071`	`1072`	`}`
`1072`	`1073`