Rollup merge of #131348 - nnethercote:rustc_infer-more-cleanups, r=lcnr

More `rustc_infer` cleanups

A sequel to #131226.

r? `@lcnr`

compiler/rustc_infer/src/infer/mod.rs

@@ -18,7 +18,7 @@
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_data_structures::sync::Lrc;
-use rustc_data_structures::undo_log::Rollback;
+use rustc_data_structures::undo_log::{Rollback, UndoLogs};
 use rustc_data_structures::unify as ut;
 use rustc_errors::{DiagCtxtHandle, ErrorGuaranteed};
 use rustc_hir as hir;
@@ -50,6 +50,7 @@
 use tracing::{debug, instrument};
 use type_variable::TypeVariableOrigin;
 
+use crate::infer::region_constraints::UndoLog;
 use crate::traits::{self, ObligationCause, ObligationInspector, PredicateObligation, TraitEngine};
 
 pub mod at;
@@ -67,6 +68,13 @@
 pub(crate) mod snapshot;
 mod type_variable;
 
+/// `InferOk<'tcx, ()>` is used a lot. It may seem like a useless wrapper
+/// around `Vec<PredicateObligation<'tcx>>`, but it has one important property:
+/// because `InferOk` is marked with `#[must_use]`, if you have a method
+/// `InferCtxt::f` that returns `InferResult<'tcx, ()>` and you call it with
+/// `infcx.f()?;` you'll get a warning about the obligations being discarded
+/// without use, which is probably unintentional and has been a source of bugs
+/// in the past.
 #[must_use]
 #[derive(Debug)]
 pub struct InferOk<'tcx, T> {
@@ -163,12 +171,12 @@ fn new() -> InferCtxtInner<'tcx> {
             undo_log: InferCtxtUndoLogs::default(),
 
             projection_cache: Default::default(),
-            type_variable_storage: type_variable::TypeVariableStorage::new(),
-            const_unification_storage: ut::UnificationTableStorage::new(),
-            int_unification_storage: ut::UnificationTableStorage::new(),
-            float_unification_storage: ut::UnificationTableStorage::new(),
-            effect_unification_storage: ut::UnificationTableStorage::new(),
-            region_constraint_storage: Some(RegionConstraintStorage::new()),
+            type_variable_storage: Default::default(),
+            const_unification_storage: Default::default(),
+            int_unification_storage: Default::default(),
+            float_unification_storage: Default::default(),
+            effect_unification_storage: Default::default(),
+            region_constraint_storage: Some(Default::default()),
             region_obligations: vec![],
             opaque_type_storage: Default::default(),
         }
@@ -1004,8 +1012,8 @@ pub fn var_for_effect(&self, param: &ty::GenericParamDef) -> GenericArg<'tcx> {
         ty::Const::new_infer(self.tcx, ty::InferConst::EffectVar(effect_vid)).into()
     }
 
-    /// Given a set of generics defined on a type or impl, returns the generic parameters mapping each
-    /// type/region parameter to a fresh inference variable.
+    /// Given a set of generics defined on a type or impl, returns the generic parameters mapping
+    /// each type/region parameter to a fresh inference variable.
     pub fn fresh_args_for_item(&self, span: Span, def_id: DefId) -> GenericArgsRef<'tcx> {
         GenericArgs::for_item(self.tcx, def_id, |param, _| self.var_for_def(span, param))
     }
@@ -1036,18 +1044,14 @@ pub fn region_var_origin(&self, vid: ty::RegionVid) -> RegionVariableOrigin {
     /// Clone the list of variable regions. This is used only during NLL processing
     /// to put the set of region variables into the NLL region context.
     pub fn get_region_var_origins(&self) -> VarInfos {
-        let mut inner = self.inner.borrow_mut();
-        let (var_infos, data) = inner
-            .region_constraint_storage
-            // We clone instead of taking because borrowck still wants to use
-            // the inference context after calling this for diagnostics
-            // and the new trait solver.
-            .clone()
-            .expect("regions already resolved")
-            .with_log(&mut inner.undo_log)
-            .into_infos_and_data();
-        assert!(data.is_empty());
-        var_infos
+        let inner = self.inner.borrow();
+        assert!(!UndoLogs::<UndoLog<'_>>::in_snapshot(&inner.undo_log));
+        let storage = inner.region_constraint_storage.as_ref().expect("regions already resolved");
+        assert!(storage.data.is_empty());
+        // We clone instead of taking because borrowck still wants to use the
+        // inference context after calling this for diagnostics and the new
+        // trait solver.
+        storage.var_infos.clone()
     }
 
     #[instrument(level = "debug", skip(self), ret)]
@@ -1383,10 +1387,10 @@ pub fn try_const_eval_resolve(
     ///
     /// The constant can be located on a trait like `<A as B>::C`, in which case the given
     /// generic parameters and environment are used to resolve the constant. Alternatively if the
-    /// constant has generic parameters in scope the instantiations are used to evaluate the value of
-    /// the constant. For example in `fn foo<T>() { let _ = [0; bar::<T>()]; }` the repeat count
-    /// constant `bar::<T>()` requires a instantiation for `T`, if the instantiation for `T` is still
-    /// too generic for the constant to be evaluated then `Err(ErrorHandled::TooGeneric)` is
+    /// constant has generic parameters in scope the instantiations are used to evaluate the value
+    /// of the constant. For example in `fn foo<T>() { let _ = [0; bar::<T>()]; }` the repeat count
+    /// constant `bar::<T>()` requires a instantiation for `T`, if the instantiation for `T` is
+    /// still too generic for the constant to be evaluated then `Err(ErrorHandled::TooGeneric)` is
     /// returned.
     ///
     /// This handles inferences variables within both `param_env` and `args` by

compiler/rustc_infer/src/infer/opaque_types/mod.rs

@@ -148,11 +148,11 @@ pub fn handle_opaque_type(
                 }
 
                 if let ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, .. }) = *b.kind() {
-                    // We could accept this, but there are various ways to handle this situation, and we don't
-                    // want to make a decision on it right now. Likely this case is so super rare anyway, that
-                    // no one encounters it in practice.
-                    // It does occur however in `fn fut() -> impl Future<Output = i32> { async { 42 } }`,
-                    // where it is of no concern, so we only check for TAITs.
+                    // We could accept this, but there are various ways to handle this situation,
+                    // and we don't want to make a decision on it right now. Likely this case is so
+                    // super rare anyway, that no one encounters it in practice. It does occur
+                    // however in `fn fut() -> impl Future<Output = i32> { async { 42 } }`, where
+                    // it is of no concern, so we only check for TAITs.
                     if self.can_define_opaque_ty(b_def_id)
                         && self.tcx.is_type_alias_impl_trait(b_def_id)
                     {

compiler/rustc_infer/src/infer/outlives/env.rs

@@ -1,8 +1,7 @@
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_data_structures::transitive_relation::TransitiveRelationBuilder;
-use rustc_middle::bug;
-use rustc_middle::ty::{self, Region};
-use tracing::{debug, instrument};
+use rustc_middle::{bug, ty};
+use tracing::debug;
 
 use super::explicit_outlives_bounds;
 use crate::infer::GenericKind;
@@ -54,37 +53,16 @@ pub struct OutlivesEnvironment<'tcx> {
     region_bound_pairs: RegionBoundPairs<'tcx>,
 }
 
-/// Builder of OutlivesEnvironment.
-#[derive(Debug)]
-struct OutlivesEnvironmentBuilder<'tcx> {
-    param_env: ty::ParamEnv<'tcx>,
-    region_relation: TransitiveRelationBuilder<Region<'tcx>>,
-    region_bound_pairs: RegionBoundPairs<'tcx>,
-}
-
 /// "Region-bound pairs" tracks outlives relations that are known to
 /// be true, either because of explicit where-clauses like `T: 'a` or
 /// because of implied bounds.
 pub type RegionBoundPairs<'tcx> = FxIndexSet<ty::OutlivesPredicate<'tcx, GenericKind<'tcx>>>;
 
 impl<'tcx> OutlivesEnvironment<'tcx> {
-    /// Create a builder using `ParamEnv` and add explicit outlives bounds into it.
-    fn builder(param_env: ty::ParamEnv<'tcx>) -> OutlivesEnvironmentBuilder<'tcx> {
-        let mut builder = OutlivesEnvironmentBuilder {
-            param_env,
-            region_relation: Default::default(),
-            region_bound_pairs: Default::default(),
-        };
-
-        builder.add_outlives_bounds(explicit_outlives_bounds(param_env));
-
-        builder
-    }
-
-    #[inline]
     /// Create a new `OutlivesEnvironment` without extra outlives bounds.
+    #[inline]
     pub fn new(param_env: ty::ParamEnv<'tcx>) -> Self {
-        Self::builder(param_env).build()
+        Self::with_bounds(param_env, vec![])
     }
 
     /// Create a new `OutlivesEnvironment` with extra outlives bounds.
@@ -92,9 +70,41 @@ pub fn with_bounds(
         param_env: ty::ParamEnv<'tcx>,
         extra_bounds: impl IntoIterator<Item = OutlivesBound<'tcx>>,
     ) -> Self {
-        let mut builder = Self::builder(param_env);
-        builder.add_outlives_bounds(extra_bounds);
-        builder.build()
+        let mut region_relation = TransitiveRelationBuilder::default();
+        let mut region_bound_pairs = RegionBoundPairs::default();
+
+        // Record relationships such as `T:'x` that don't go into the
+        // free-region-map but which we use here.
+        for outlives_bound in explicit_outlives_bounds(param_env).chain(extra_bounds) {
+            debug!("add_outlives_bounds: outlives_bound={:?}", outlives_bound);
+            match outlives_bound {
+                OutlivesBound::RegionSubParam(r_a, param_b) => {
+                    region_bound_pairs
+                        .insert(ty::OutlivesPredicate(GenericKind::Param(param_b), r_a));
+                }
+                OutlivesBound::RegionSubAlias(r_a, alias_b) => {
+                    region_bound_pairs
+                        .insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a));
+                }
+                OutlivesBound::RegionSubRegion(r_a, r_b) => match (*r_a, *r_b) {
+                    (
+                        ty::ReStatic | ty::ReEarlyParam(_) | ty::ReLateParam(_),
+                        ty::ReStatic | ty::ReEarlyParam(_) | ty::ReLateParam(_),
+                    ) => region_relation.add(r_a, r_b),
+                    (ty::ReError(_), _) | (_, ty::ReError(_)) => {}
+                    // FIXME(#109628): We shouldn't have existential variables in implied bounds.
+                    // Panic here once the linked issue is resolved!
+                    (ty::ReVar(_), _) | (_, ty::ReVar(_)) => {}
+                    _ => bug!("add_outlives_bounds: unexpected regions: ({r_a:?}, {r_b:?})"),
+                },
+            }
+        }
+
+        OutlivesEnvironment {
+            param_env,
+            free_region_map: FreeRegionMap { relation: region_relation.freeze() },
+            region_bound_pairs,
+        }
     }
 
     /// Borrows current value of the `free_region_map`.
@@ -107,48 +117,3 @@ pub fn region_bound_pairs(&self) -> &RegionBoundPairs<'tcx> {
         &self.region_bound_pairs
     }
 }
-
-impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
-    #[inline]
-    #[instrument(level = "debug")]
-    fn build(self) -> OutlivesEnvironment<'tcx> {
-        OutlivesEnvironment {
-            param_env: self.param_env,
-            free_region_map: FreeRegionMap { relation: self.region_relation.freeze() },
-            region_bound_pairs: self.region_bound_pairs,
-        }
-    }
-
-    /// Processes outlives bounds that are known to hold, whether from implied or other sources.
-    fn add_outlives_bounds<I>(&mut self, outlives_bounds: I)
-    where
-        I: IntoIterator<Item = OutlivesBound<'tcx>>,
-    {
-        // Record relationships such as `T:'x` that don't go into the
-        // free-region-map but which we use here.
-        for outlives_bound in outlives_bounds {
-            debug!("add_outlives_bounds: outlives_bound={:?}", outlives_bound);
-            match outlives_bound {
-                OutlivesBound::RegionSubParam(r_a, param_b) => {
-                    self.region_bound_pairs
-                        .insert(ty::OutlivesPredicate(GenericKind::Param(param_b), r_a));
-                }
-                OutlivesBound::RegionSubAlias(r_a, alias_b) => {
-                    self.region_bound_pairs
-                        .insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a));
-                }
-                OutlivesBound::RegionSubRegion(r_a, r_b) => match (*r_a, *r_b) {
-                    (
-                        ty::ReStatic | ty::ReEarlyParam(_) | ty::ReLateParam(_),
-                        ty::ReStatic | ty::ReEarlyParam(_) | ty::ReLateParam(_),
-                    ) => self.region_relation.add(r_a, r_b),
-                    (ty::ReError(_), _) | (_, ty::ReError(_)) => {}
-                    // FIXME(#109628): We shouldn't have existential variables in implied bounds.
-                    // Panic here once the linked issue is resolved!
-                    (ty::ReVar(_), _) | (_, ty::ReVar(_)) => {}
-                    _ => bug!("add_outlives_bounds: unexpected regions: ({r_a:?}, {r_b:?})"),
-                },
-            }
-        }
-    }
-}

compiler/rustc_infer/src/infer/outlives/mod.rs

@@ -1,11 +1,12 @@
 //! Various code related to computing outlives relations.
 
+use rustc_data_structures::undo_log::UndoLogs;
 use rustc_middle::traits::query::{NoSolution, OutlivesBound};
 use rustc_middle::ty;
 use tracing::instrument;
 
 use self::env::OutlivesEnvironment;
-use super::region_constraints::RegionConstraintData;
+use super::region_constraints::{RegionConstraintData, UndoLog};
 use super::{InferCtxt, RegionResolutionError, SubregionOrigin};
 use crate::infer::free_regions::RegionRelations;
 use crate::infer::lexical_region_resolve;
@@ -63,7 +64,7 @@ pub fn resolve_regions_with_normalize(
             }
         };
 
-        let (var_infos, data) = {
+        let storage = {
             let mut inner = self.inner.borrow_mut();
             let inner = &mut *inner;
             assert!(
@@ -71,18 +72,14 @@ pub fn resolve_regions_with_normalize(
                 "region_obligations not empty: {:#?}",
                 inner.region_obligations
             );
-            inner
-                .region_constraint_storage
-                .take()
-                .expect("regions already resolved")
-                .with_log(&mut inner.undo_log)
-                .into_infos_and_data()
+            assert!(!UndoLogs::<UndoLog<'_>>::in_snapshot(&inner.undo_log));
+            inner.region_constraint_storage.take().expect("regions already resolved")
         };
 
         let region_rels = &RegionRelations::new(self.tcx, outlives_env.free_region_map());
 
         let (lexical_region_resolutions, errors) =
-            lexical_region_resolve::resolve(region_rels, var_infos, data);
+            lexical_region_resolve::resolve(region_rels, storage.var_infos, storage.data);
 
         let old_value = self.lexical_region_resolutions.replace(Some(lexical_region_resolutions));
         assert!(old_value.is_none());

compiler/rustc_infer/src/infer/outlives/obligations.rs

@@ -396,11 +396,12 @@ fn alias_ty_must_outlive(
         // 'a` in the environment but `trait Foo<'b> { type Item: 'b
         // }` in the trait definition.
         approx_env_bounds.retain(|bound_outlives| {
-            // OK to skip binder because we only manipulate and compare against other
-            // values from the same binder. e.g. if we have (e.g.) `for<'a> <T as Trait<'a>>::Item: 'a`
-            // in `bound`, the `'a` will be a `^1` (bound, debruijn index == innermost) region.
-            // If the declaration is `trait Trait<'b> { type Item: 'b; }`, then `projection_declared_bounds_from_trait`
-            // will be invoked with `['b => ^1]` and so we will get `^1` returned.
+            // OK to skip binder because we only manipulate and compare against other values from
+            // the same binder. e.g. if we have (e.g.) `for<'a> <T as Trait<'a>>::Item: 'a` in
+            // `bound`, the `'a` will be a `^1` (bound, debruijn index == innermost) region. If the
+            // declaration is `trait Trait<'b> { type Item: 'b; }`, then
+            // `projection_declared_bounds_from_trait` will be invoked with `['b => ^1]` and so we
+            // will get `^1` returned.
             let bound = bound_outlives.skip_binder();
             let ty::Alias(_, alias_ty) = bound.0.kind() else { bug!("expected AliasTy") };
             self.verify_bound.declared_bounds_from_definition(*alias_ty).all(|r| r != bound.1)

compiler/rustc_infer/src/infer/region_constraints/leak_check.rs

@@ -55,8 +55,8 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
     /// * what placeholder they must outlive transitively
     ///   * if they must also be equal to a placeholder, report an error because `P1: P2`
     /// * minimum universe U of all SCCs they must outlive
-    ///   * if they must also be equal to a placeholder P, and U cannot name P, report an error, as that
-    ///     indicates `P: R` and `R` is in an incompatible universe
+    ///   * if they must also be equal to a placeholder P, and U cannot name P, report an error, as
+    ///     that indicates `P: R` and `R` is in an incompatible universe
     ///
     /// To improve performance and for the old trait solver caching to be sound, this takes
     /// an optional snapshot in which case we only look at region constraints added in that
@@ -73,7 +73,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
     /// * R: P1, R: P2, as above
     #[instrument(level = "debug", skip(self, tcx, only_consider_snapshot), ret)]
     pub fn leak_check(
-        &mut self,
+        self,
         tcx: TyCtxt<'tcx>,
         outer_universe: ty::UniverseIndex,
         max_universe: ty::UniverseIndex,
@@ -83,7 +83,7 @@ pub fn leak_check(
             return Ok(());
         }
 
-        let mini_graph = &MiniGraph::new(tcx, self, only_consider_snapshot);
+        let mini_graph = MiniGraph::new(tcx, &self, only_consider_snapshot);
 
         let mut leak_check = LeakCheck::new(tcx, outer_universe, max_universe, mini_graph, self);
         leak_check.assign_placeholder_values()?;
@@ -92,11 +92,11 @@ pub fn leak_check(
     }
 }
 
-struct LeakCheck<'a, 'b, 'tcx> {
+struct LeakCheck<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
     outer_universe: ty::UniverseIndex,
-    mini_graph: &'a MiniGraph<'tcx>,
-    rcc: &'a mut RegionConstraintCollector<'b, 'tcx>,
+    mini_graph: MiniGraph<'tcx>,
+    rcc: RegionConstraintCollector<'a, 'tcx>,
 
     // Initially, for each SCC S, stores a placeholder `P` such that `S = P`
     // must hold.
@@ -115,26 +115,27 @@ struct LeakCheck<'a, 'b, 'tcx> {
     // either the placeholder `P1` or the empty region in that same universe.
     //
     // To detect errors, we look for an SCC S where the values in
-    // `scc_values[S]` (if any) cannot be stored into `scc_universes[S]`.
+    // `scc_placeholders[S]` (if any) cannot be stored into `scc_universes[S]`.
     scc_universes: IndexVec<LeakCheckScc, SccUniverse<'tcx>>,
 }
 
-impl<'a, 'b, 'tcx> LeakCheck<'a, 'b, 'tcx> {
+impl<'a, 'tcx> LeakCheck<'a, 'tcx> {
     fn new(
         tcx: TyCtxt<'tcx>,
         outer_universe: ty::UniverseIndex,
         max_universe: ty::UniverseIndex,
-        mini_graph: &'a MiniGraph<'tcx>,
-        rcc: &'a mut RegionConstraintCollector<'b, 'tcx>,
+        mini_graph: MiniGraph<'tcx>,
+        rcc: RegionConstraintCollector<'a, 'tcx>,
     ) -> Self {
         let dummy_scc_universe = SccUniverse { universe: max_universe, region: None };
+        let num_sccs = mini_graph.sccs.num_sccs();
         Self {
             tcx,
             outer_universe,
             mini_graph,
             rcc,
-            scc_placeholders: IndexVec::from_elem_n(None, mini_graph.sccs.num_sccs()),
-            scc_universes: IndexVec::from_elem_n(dummy_scc_universe, mini_graph.sccs.num_sccs()),
+            scc_placeholders: IndexVec::from_elem_n(None, num_sccs),
+            scc_universes: IndexVec::from_elem_n(dummy_scc_universe, num_sccs),
         }
     }
 
@@ -156,7 +157,16 @@ fn assign_placeholder_values(&mut self) -> RelateResult<'tcx, ()> {
             // Detect those SCCs that directly contain a placeholder
             if let ty::RePlaceholder(placeholder) = **region {
                 if self.outer_universe.cannot_name(placeholder.universe) {
-                    self.assign_scc_value(scc, placeholder)?;
+                    // Update `scc_placeholders` to account for the fact that `P: S` must hold.
+                    match self.scc_placeholders[scc] {
+                        Some(p) => {
+                            assert_ne!(p, placeholder);
+                            return Err(self.placeholder_error(p, placeholder));
+                        }
+                        None => {
+                            self.scc_placeholders[scc] = Some(placeholder);
+                        }
+                    }
                 }
             }
         }
@@ -164,26 +174,6 @@ fn assign_placeholder_values(&mut self) -> RelateResult<'tcx, ()> {
         Ok(())
     }
 
-    // assign_scc_value(S, P): Update `scc_values` to account for the fact that `P: S` must hold.
-    // This may create an error.
-    fn assign_scc_value(
-        &mut self,
-        scc: LeakCheckScc,
-        placeholder: ty::PlaceholderRegion,
-    ) -> RelateResult<'tcx, ()> {
-        match self.scc_placeholders[scc] {
-            Some(p) => {
-                assert_ne!(p, placeholder);
-                return Err(self.placeholder_error(p, placeholder));
-            }
-            None => {
-                self.scc_placeholders[scc] = Some(placeholder);
-            }
-        };
-
-        Ok(())
-    }
-
     /// For each SCC S, iterate over each successor S1 where `S: S1`:
     ///
     /// * Compute
@@ -216,8 +206,8 @@ fn propagate_scc_value(&mut self) -> RelateResult<'tcx, ()> {
             // Walk over each `scc2` such that `scc1: scc2` and compute:
             //
             // * `scc1_universe`: the minimum universe of `scc2` and the constituents of `scc1`
-            // * `succ_bound`: placeholder `P` that the successors must outlive, if any (if there are multiple,
-            //   we pick one arbitrarily)
+            // * `succ_bound`: placeholder `P` that the successors must outlive, if any (if there
+            //   are multiple, we pick one arbitrarily)
             let mut scc1_universe = self.scc_universes[scc1];
             let mut succ_bound = None;
             for &scc2 in self.mini_graph.sccs.successors(scc1) {
@@ -260,7 +250,8 @@ fn propagate_scc_value(&mut self) -> RelateResult<'tcx, ()> {
                 self.scc_placeholders[scc1] = succ_bound;
             }
 
-            // At this point, `scc_placeholder[scc1]` stores some placeholder that `scc1` must outlive (if any).
+            // At this point, `scc_placeholder[scc1]` stores some placeholder that `scc1` must
+            // outlive (if any).
         }
         Ok(())
     }

compiler/rustc_infer/src/infer/region_constraints/mod.rs

@@ -27,9 +27,9 @@
 #[derive(Clone, Default)]
 pub struct RegionConstraintStorage<'tcx> {
     /// For each `RegionVid`, the corresponding `RegionVariableOrigin`.
-    var_infos: IndexVec<RegionVid, RegionVariableInfo>,
+    pub(super) var_infos: IndexVec<RegionVid, RegionVariableInfo>,
 
-    data: RegionConstraintData<'tcx>,
+    pub(super) data: RegionConstraintData<'tcx>,
 
     /// For a given pair of regions (R1, R2), maps to a region R3 that
     /// is designated as their LUB (edges R1 <= R3 and R2 <= R3
@@ -61,21 +61,6 @@ pub struct RegionConstraintCollector<'a, 'tcx> {
     undo_log: &'a mut InferCtxtUndoLogs<'tcx>,
 }
 
-impl<'tcx> std::ops::Deref for RegionConstraintCollector<'_, 'tcx> {
-    type Target = RegionConstraintStorage<'tcx>;
-    #[inline]
-    fn deref(&self) -> &RegionConstraintStorage<'tcx> {
-        self.storage
-    }
-}
-
-impl<'tcx> std::ops::DerefMut for RegionConstraintCollector<'_, 'tcx> {
-    #[inline]
-    fn deref_mut(&mut self) -> &mut RegionConstraintStorage<'tcx> {
-        self.storage
-    }
-}
-
 pub type VarInfos = IndexVec<RegionVid, RegionVariableInfo>;
 
 /// The full set of region constraints gathered up by the collector.
@@ -309,10 +294,6 @@ pub(crate) struct RegionSnapshot {
 }
 
 impl<'tcx> RegionConstraintStorage<'tcx> {
-    pub fn new() -> Self {
-        Self::default()
-    }
-
     #[inline]
     pub(crate) fn with_log<'a>(
         &'a mut self,
@@ -320,46 +301,15 @@ pub(crate) fn with_log<'a>(
     ) -> RegionConstraintCollector<'a, 'tcx> {
         RegionConstraintCollector { storage: self, undo_log }
     }
-
-    fn rollback_undo_entry(&mut self, undo_entry: UndoLog<'tcx>) {
-        match undo_entry {
-            AddVar(vid) => {
-                self.var_infos.pop().unwrap();
-                assert_eq!(self.var_infos.len(), vid.index());
-            }
-            AddConstraint(index) => {
-                self.data.constraints.pop().unwrap();
-                assert_eq!(self.data.constraints.len(), index);
-            }
-            AddVerify(index) => {
-                self.data.verifys.pop();
-                assert_eq!(self.data.verifys.len(), index);
-            }
-            AddCombination(Glb, ref regions) => {
-                self.glbs.remove(regions);
-            }
-            AddCombination(Lub, ref regions) => {
-                self.lubs.remove(regions);
-            }
-        }
-    }
 }
 
 impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
     pub fn num_region_vars(&self) -> usize {
-        self.var_infos.len()
+        self.storage.var_infos.len()
     }
 
     pub fn region_constraint_data(&self) -> &RegionConstraintData<'tcx> {
-        &self.data
-    }
-
-    /// Once all the constraints have been gathered, extract out the final data.
-    ///
-    /// Not legal during a snapshot.
-    pub fn into_infos_and_data(self) -> (VarInfos, RegionConstraintData<'tcx>) {
-        assert!(!UndoLogs::<UndoLog<'_>>::in_snapshot(&self.undo_log));
-        (mem::take(&mut self.storage.var_infos), mem::take(&mut self.storage.data))
+        &self.storage.data
     }
 
     /// Takes (and clears) the current set of constraints. Note that
@@ -415,17 +365,17 @@ pub fn take_and_reset_data(&mut self) -> RegionConstraintData<'tcx> {
     }
 
     pub fn data(&self) -> &RegionConstraintData<'tcx> {
-        &self.data
+        &self.storage.data
     }
 
-    pub(super) fn start_snapshot(&mut self) -> RegionSnapshot {
+    pub(super) fn start_snapshot(&self) -> RegionSnapshot {
         debug!("RegionConstraintCollector: start_snapshot");
-        RegionSnapshot { any_unifications: self.any_unifications }
+        RegionSnapshot { any_unifications: self.storage.any_unifications }
     }
 
     pub(super) fn rollback_to(&mut self, snapshot: RegionSnapshot) {
         debug!("RegionConstraintCollector: rollback_to({:?})", snapshot);
-        self.any_unifications = snapshot.any_unifications;
+        self.storage.any_unifications = snapshot.any_unifications;
     }
 
     pub(super) fn new_region_var(
@@ -433,7 +383,7 @@ pub(super) fn new_region_var(
         universe: ty::UniverseIndex,
         origin: RegionVariableOrigin,
     ) -> RegionVid {
-        let vid = self.var_infos.push(RegionVariableInfo { origin, universe });
+        let vid = self.storage.var_infos.push(RegionVariableInfo { origin, universe });
 
         let u_vid = self.unification_table_mut().new_key(RegionVariableValue::Unknown { universe });
         assert_eq!(vid, u_vid.vid);
@@ -444,7 +394,7 @@ pub(super) fn new_region_var(
 
     /// Returns the origin for the given variable.
     pub(super) fn var_origin(&self, vid: RegionVid) -> RegionVariableOrigin {
-        self.var_infos[vid].origin
+        self.storage.var_infos[vid].origin
     }
 
     fn add_constraint(&mut self, constraint: Constraint<'tcx>, origin: SubregionOrigin<'tcx>) {
@@ -467,8 +417,8 @@ fn add_verify(&mut self, verify: Verify<'tcx>) {
             return;
         }
 
-        let index = self.data.verifys.len();
-        self.data.verifys.push(verify);
+        let index = self.storage.data.verifys.len();
+        self.storage.data.verifys.push(verify);
         self.undo_log.push(AddVerify(index));
     }
 
@@ -488,7 +438,7 @@ pub(super) fn make_eqregion(
                 (ty::ReVar(a), ty::ReVar(b)) => {
                     debug!("make_eqregion: unifying {:?} with {:?}", a, b);
                     if self.unification_table_mut().unify_var_var(a, b).is_ok() {
-                        self.any_unifications = true;
+                        self.storage.any_unifications = true;
                     }
                 }
                 (ty::ReVar(vid), _) => {
@@ -498,7 +448,7 @@ pub(super) fn make_eqregion(
                         .unify_var_value(vid, RegionVariableValue::Known { value: b })
                         .is_ok()
                     {
-                        self.any_unifications = true;
+                        self.storage.any_unifications = true;
                     };
                 }
                 (_, ty::ReVar(vid)) => {
@@ -508,7 +458,7 @@ pub(super) fn make_eqregion(
                         .unify_var_value(vid, RegionVariableValue::Known { value: a })
                         .is_ok()
                     {
-                        self.any_unifications = true;
+                        self.storage.any_unifications = true;
                     };
                 }
                 (_, _) => {}
@@ -530,7 +480,7 @@ pub(super) fn member_constraint(
             return;
         }
 
-        self.data.member_constraints.push(MemberConstraint {
+        self.storage.data.member_constraints.push(MemberConstraint {
             key,
             definition_span,
             hidden_ty,
@@ -646,8 +596,8 @@ pub fn probe_value(
 
     fn combine_map(&mut self, t: CombineMapType) -> &mut CombineMap<'tcx> {
         match t {
-            Glb => &mut self.glbs,
-            Lub => &mut self.lubs,
+            Glb => &mut self.storage.glbs,
+            Lub => &mut self.storage.lubs,
         }
     }
 
@@ -700,11 +650,12 @@ pub fn vars_since_snapshot(
         &self,
         value_count: usize,
     ) -> (Range<RegionVid>, Vec<RegionVariableOrigin>) {
-        let range = RegionVid::from(value_count)..RegionVid::from(self.unification_table.len());
+        let range =
+            RegionVid::from(value_count)..RegionVid::from(self.storage.unification_table.len());
         (
             range.clone(),
             (range.start.index()..range.end.index())
-                .map(|index| self.var_infos[ty::RegionVid::from(index)].origin)
+                .map(|index| self.storage.var_infos[ty::RegionVid::from(index)].origin)
                 .collect(),
         )
     }
@@ -801,6 +752,25 @@ pub fn is_empty(&self) -> bool {
 
 impl<'tcx> Rollback<UndoLog<'tcx>> for RegionConstraintStorage<'tcx> {
     fn reverse(&mut self, undo: UndoLog<'tcx>) {
-        self.rollback_undo_entry(undo)
+        match undo {
+            AddVar(vid) => {
+                self.var_infos.pop().unwrap();
+                assert_eq!(self.var_infos.len(), vid.index());
+            }
+            AddConstraint(index) => {
+                self.data.constraints.pop().unwrap();
+                assert_eq!(self.data.constraints.len(), index);
+            }
+            AddVerify(index) => {
+                self.data.verifys.pop();
+                assert_eq!(self.data.verifys.len(), index);
+            }
+            AddCombination(Glb, ref regions) => {
+                self.glbs.remove(regions);
+            }
+            AddCombination(Lub, ref regions) => {
+                self.lubs.remove(regions);
+            }
+        }
     }
 }

compiler/rustc_infer/src/infer/relate/generalize.rs

@@ -50,7 +50,8 @@ pub fn instantiate_ty_var<R: PredicateEmittingRelation<InferCtxt<'tcx>>>(
         // Then the `generalized_ty` would be `&'?2 ?3`, where `'?2` and `?3` are fresh
         // region/type inference variables.
         //
-        // We then relate `generalized_ty <: source_ty`,adding constraints like `'x: '?2` and `?1 <: ?3`.
+        // We then relate `generalized_ty <: source_ty`, adding constraints like `'x: '?2` and
+        // `?1 <: ?3`.
         let Generalization { value_may_be_infer: generalized_ty, has_unconstrained_ty_var } = self
             .generalize(
                 relation.span(),
@@ -104,7 +105,8 @@ pub fn instantiate_ty_var<R: PredicateEmittingRelation<InferCtxt<'tcx>>>(
                     &ty::Alias(ty::Projection, data) => {
                         // FIXME: This does not handle subtyping correctly, we could
                         // instead create a new inference variable `?normalized_source`, emitting
-                        // `Projection(normalized_source, ?ty_normalized)` and `?normalized_source <: generalized_ty`.
+                        // `Projection(normalized_source, ?ty_normalized)` and
+                        // `?normalized_source <: generalized_ty`.
                         relation.register_predicates([ty::ProjectionPredicate {
                             projection_term: data.into(),
                             term: generalized_ty.into(),

compiler/rustc_infer/src/infer/resolve.rs

@@ -38,7 +38,7 @@ fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
         if !t.has_non_region_infer() {
             t // micro-optimize -- if there is nothing in this type that this fold affects...
         } else if let Some(&ty) = self.cache.get(&t) {
-            return ty;
+            ty
         } else {
             let shallow = self.infcx.shallow_resolve(t);
             let res = shallow.super_fold_with(self);

compiler/rustc_infer/src/infer/type_variable.rs

@@ -19,7 +19,7 @@ fn reverse(&mut self, undo: sv::UndoLog<ut::Delegate<TyVidEqKey<'tcx>>>) {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Default)]
 pub(crate) struct TypeVariableStorage<'tcx> {
     /// The origins of each type variable.
     values: IndexVec<TyVid, TypeVariableData>,
@@ -74,13 +74,6 @@ pub(crate) fn is_unknown(&self) -> bool {
 }
 
 impl<'tcx> TypeVariableStorage<'tcx> {
-    pub(crate) fn new() -> TypeVariableStorage<'tcx> {
-        TypeVariableStorage {
-            values: Default::default(),
-            eq_relations: ut::UnificationTableStorage::new(),
-        }
-    }
-
     #[inline]
     pub(crate) fn with_log<'a>(
         &'a mut self,

compiler/rustc_infer/src/traits/project.rs

@@ -92,38 +92,31 @@ pub enum ProjectionCacheEntry<'tcx> {
     Error,
     NormalizedTerm {
         ty: NormalizedTerm<'tcx>,
-        /// If we were able to successfully evaluate the
-        /// corresponding cache entry key during predicate
-        /// evaluation, then this field stores the final
-        /// result obtained from evaluating all of the projection
-        /// sub-obligations. During evaluation, we will skip
-        /// evaluating the cached sub-obligations in `ty`
-        /// if this field is set. Evaluation only
-        /// cares about the final result, so we don't
-        /// care about any region constraint side-effects
-        /// produced by evaluating the sub-obligations.
-        ///
-        /// Additionally, we will clear out the sub-obligations
-        /// entirely if we ever evaluate the cache entry (along
-        /// with all its sub obligations) to `EvaluatedToOk`.
-        /// This affects all users of the cache, not just evaluation.
-        /// Since a result of `EvaluatedToOk` means that there were
-        /// no region obligations that need to be tracked, it's
-        /// fine to forget about the sub-obligations - they
-        /// don't provide any additional information. However,
-        /// we do *not* discard any obligations when we see
-        /// `EvaluatedToOkModuloRegions` - we don't know
-        /// which sub-obligations may introduce region constraints,
-        /// so we keep them all to be safe.
-        ///
-        /// When we are not performing evaluation
-        /// (e.g. in `FulfillmentContext`), we ignore this field,
-        /// and always re-process the cached sub-obligations
-        /// (which may have been cleared out - see the above
-        /// paragraph).
-        /// This ensures that we do not lose any regions
-        /// constraints that arise from processing the
+        /// If we were able to successfully evaluate the corresponding cache
+        /// entry key during predicate evaluation, then this field stores the
+        /// final result obtained from evaluating all of the projection
+        /// sub-obligations. During evaluation, we will skip evaluating the
+        /// cached sub-obligations in `ty` if this field is set. Evaluation
+        /// only cares about the final result, so we don't care about any
+        /// region constraint side-effects produced by evaluating the
         /// sub-obligations.
+        ///
+        /// Additionally, we will clear out the sub-obligations entirely if we
+        /// ever evaluate the cache entry (along with all its sub obligations)
+        /// to `EvaluatedToOk`. This affects all users of the cache, not just
+        /// evaluation. Since a result of `EvaluatedToOk` means that there were
+        /// no region obligations that need to be tracked, it's fine to forget
+        /// about the sub-obligations - they don't provide any additional
+        /// information. However, we do *not* discard any obligations when we
+        /// see `EvaluatedToOkModuloRegions` - we don't know which
+        /// sub-obligations may introduce region constraints, so we keep them
+        /// all to be safe.
+        ///
+        /// When we are not performing evaluation (e.g. in
+        /// `FulfillmentContext`), we ignore this field, and always re-process
+        /// the cached sub-obligations (which may have been cleared out - see
+        /// the above paragraph). This ensures that we do not lose any regions
+        /// constraints that arise from processing the sub-obligations.
         complete: Option<EvaluationResult>,
     },
 }