Fix review comments

Author: Markus Westerlind

src/librustc_infer/infer/mod.rs

@@ -13,7 +13,7 @@ use crate::traits::{self, ObligationCause, PredicateObligations, TraitEngine};
 use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::sync::Lrc;
-use rustc_data_structures::undo_log::{Rollback, Snapshots};
+use rustc_data_structures::undo_log::Rollback;
 use rustc_data_structures::unify as ut;
 use rustc_errors::DiagnosticBuilder;
 use rustc_hir as hir;
@@ -151,16 +151,16 @@ pub struct InferCtxtInner<'tcx> {
     /// We instantiate `UnificationTable` with `bounds<Ty>` because the types
     /// that might instantiate a general type variable have an order,
     /// represented by its upper and lower bounds.
-    type_variables: type_variable::TypeVariableStorage<'tcx>,
+    type_variable_storage: type_variable::TypeVariableStorage<'tcx>,
 
     /// Map from const parameter variable to the kind of const it represents.
-    const_unification_table: ut::UnificationTableStorage<ty::ConstVid<'tcx>>,
+    const_unification_storage: ut::UnificationTableStorage<ty::ConstVid<'tcx>>,
 
     /// Map from integral variable to the kind of integer it represents.
-    int_unification_table: ut::UnificationTableStorage<ty::IntVid>,
+    int_unification_storage: ut::UnificationTableStorage<ty::IntVid>,
 
     /// Map from floating variable to the kind of float it represents.
-    float_unification_table: ut::UnificationTableStorage<ty::FloatVid>,
+    float_unification_storage: ut::UnificationTableStorage<ty::FloatVid>,
 
     /// Tracks the set of region variables and the constraints between them.
     /// This is initially `Some(_)` but when
@@ -209,11 +209,11 @@ impl<'tcx> InferCtxtInner<'tcx> {
     fn new() -> InferCtxtInner<'tcx> {
         InferCtxtInner {
             projection_cache: Default::default(),
-            type_variables: type_variable::TypeVariableStorage::new(),
+            type_variable_storage: type_variable::TypeVariableStorage::new(),
             undo_log: InferCtxtUndoLogs::default(),
-            const_unification_table: ut::UnificationTableStorage::new(),
-            int_unification_table: ut::UnificationTableStorage::new(),
-            float_unification_table: ut::UnificationTableStorage::new(),
+            const_unification_storage: ut::UnificationTableStorage::new(),
+            int_unification_storage: ut::UnificationTableStorage::new(),
+            float_unification_storage: ut::UnificationTableStorage::new(),
             region_constraints: Some(RegionConstraintStorage::new()),
             region_obligations: vec![],
         }
@@ -223,12 +223,12 @@ impl<'tcx> InferCtxtInner<'tcx> {
         &self.region_obligations
     }
 
-    pub fn projection_cache(&mut self) -> traits::ProjectionCache<'tcx, '_> {
+    pub fn projection_cache(&mut self) -> traits::ProjectionCache<'_, 'tcx> {
         self.projection_cache.with_log(&mut self.undo_log)
     }
 
-    fn type_variables(&mut self) -> type_variable::TypeVariableTable<'tcx, '_> {
-        self.type_variables.with_log(&mut self.undo_log)
+    fn type_variables(&mut self) -> type_variable::TypeVariableTable<'_, 'tcx> {
+        self.type_variable_storage.with_log(&mut self.undo_log)
     }
 
     fn int_unification_table(
@@ -240,7 +240,7 @@ impl<'tcx> InferCtxtInner<'tcx> {
             &mut InferCtxtUndoLogs<'tcx>,
         >,
     > {
-        self.int_unification_table.with_log(&mut self.undo_log)
+        self.int_unification_storage.with_log(&mut self.undo_log)
     }
 
     fn float_unification_table(
@@ -252,7 +252,7 @@ impl<'tcx> InferCtxtInner<'tcx> {
             &mut InferCtxtUndoLogs<'tcx>,
         >,
     > {
-        self.float_unification_table.with_log(&mut self.undo_log)
+        self.float_unification_storage.with_log(&mut self.undo_log)
     }
 
     fn const_unification_table(
@@ -264,7 +264,7 @@ impl<'tcx> InferCtxtInner<'tcx> {
             &mut InferCtxtUndoLogs<'tcx>,
         >,
     > {
-        self.const_unification_table.with_log(&mut self.undo_log)
+        self.const_unification_storage.with_log(&mut self.undo_log)
     }
 
     pub fn unwrap_region_constraints(&mut self) -> RegionConstraintCollector<'tcx, '_> {
@@ -868,29 +868,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         self.in_snapshot.set(was_in_snapshot);
         self.universe.set(universe);
 
-        let InferCtxtInner {
-            type_variables,
-            const_unification_table,
-            int_unification_table,
-            float_unification_table,
-            region_constraints,
-            projection_cache,
-            region_obligations,
-            undo_log,
-            ..
-        } = &mut *self.inner.borrow_mut();
-        undo_log.rollback_to(
-            || undo_log::RollbackView {
-                type_variables,
-                const_unification_table,
-                int_unification_table,
-                float_unification_table,
-                region_constraints: region_constraints.as_mut().unwrap(),
-                projection_cache,
-                region_obligations,
-            },
-            undo_snapshot,
-        );
+        self.inner.borrow_mut().rollback_to(undo_snapshot);
     }
 
     fn commit_from(&self, snapshot: CombinedSnapshot<'a, 'tcx>) {
@@ -900,8 +878,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
 
         self.in_snapshot.set(was_in_snapshot);
 
-        let mut inner = self.inner.borrow_mut();
-        inner.undo_log.commit(undo_snapshot);
+        self.inner.borrow_mut().commit(undo_snapshot);
     }
 
     /// Executes `f` and commit the bindings.

src/librustc_infer/infer/type_variable.rs

@@ -11,7 +11,7 @@ use std::cmp;
 use std::marker::PhantomData;
 use std::ops::Range;
 
-use rustc_data_structures::undo_log::{Rollback, Snapshots, UndoLogs};
+use rustc_data_structures::undo_log::{Rollback, UndoLogs};
 
 pub(crate) enum UndoLog<'tcx> {
     EqRelation(sv::UndoLog<ut::Delegate<TyVidEqKey<'tcx>>>),
@@ -81,7 +81,7 @@ pub struct TypeVariableStorage<'tcx> {
     sub_relations: ut::UnificationTableStorage<ty::TyVid>,
 }
 
-pub struct TypeVariableTable<'tcx, 'a> {
+pub struct TypeVariableTable<'a, 'tcx> {
     values: &'a mut sv::SnapshotVecStorage<Delegate>,
 
     eq_relations: &'a mut ut::UnificationTableStorage<TyVidEqKey<'tcx>>,
@@ -168,13 +168,13 @@ impl<'tcx> TypeVariableStorage<'tcx> {
     pub(crate) fn with_log<'a>(
         &'a mut self,
         undo_log: &'a mut InferCtxtUndoLogs<'tcx>,
-    ) -> TypeVariableTable<'tcx, 'a> {
+    ) -> TypeVariableTable<'a, 'tcx> {
         let TypeVariableStorage { values, eq_relations, sub_relations } = self;
         TypeVariableTable { values, eq_relations, sub_relations, undo_log }
     }
 }
 
-impl<'tcx> TypeVariableTable<'tcx, '_> {
+impl<'tcx> TypeVariableTable<'_, 'tcx> {
     /// Returns the diverges flag given when `vid` was created.
     ///
     /// Note that this function does not return care whether

src/librustc_infer/infer/undo_log.rs

@@ -1,16 +1,12 @@
 use std::marker::PhantomData;
 
 use rustc_data_structures::snapshot_vec as sv;
-use rustc_data_structures::undo_log::{Rollback, Snapshots, UndoLogs};
+use rustc_data_structures::undo_log::{Rollback, UndoLogs};
 use rustc_data_structures::unify as ut;
-use rustc_hir as hir;
 use rustc_middle::ty;
 
 use crate::{
-    infer::{
-        region_constraints::{self, RegionConstraintStorage},
-        type_variable, RegionObligation,
-    },
+    infer::{region_constraints, type_variable, InferCtxtInner},
     traits,
 };
 
@@ -19,6 +15,7 @@ pub struct Snapshot<'tcx> {
     _marker: PhantomData<&'tcx ()>,
 }
 
+/// Records the 'undo' data fora single operation that affects some form of inference variable.
 pub(crate) enum UndoLog<'tcx> {
     TypeVariables(type_variable::UndoLog<'tcx>),
     ConstUnificationTable(sv::UndoLog<ut::Delegate<ty::ConstVid<'tcx>>>),
@@ -96,26 +93,18 @@ impl<'tcx> From<traits::UndoLog<'tcx>> for UndoLog<'tcx> {
     }
 }
 
-pub(super) struct RollbackView<'tcx, 'a> {
-    pub(super) type_variables: &'a mut type_variable::TypeVariableStorage<'tcx>,
-    pub(super) const_unification_table: &'a mut ut::UnificationTableStorage<ty::ConstVid<'tcx>>,
-    pub(super) int_unification_table: &'a mut ut::UnificationTableStorage<ty::IntVid>,
-    pub(super) float_unification_table: &'a mut ut::UnificationTableStorage<ty::FloatVid>,
-    pub(super) region_constraints: &'a mut RegionConstraintStorage<'tcx>,
-    pub(super) projection_cache: &'a mut traits::ProjectionCacheStorage<'tcx>,
-    pub(super) region_obligations: &'a mut Vec<(hir::HirId, RegionObligation<'tcx>)>,
-}
-
-impl<'tcx> Rollback<UndoLog<'tcx>> for RollbackView<'tcx, '_> {
+impl<'tcx> Rollback<UndoLog<'tcx>> for InferCtxtInner<'tcx> {
     fn reverse(&mut self, undo: UndoLog<'tcx>) {
         match undo {
-            UndoLog::TypeVariables(undo) => self.type_variables.reverse(undo),
-            UndoLog::ConstUnificationTable(undo) => self.const_unification_table.reverse(undo),
-            UndoLog::IntUnificationTable(undo) => self.int_unification_table.reverse(undo),
-            UndoLog::FloatUnificationTable(undo) => self.float_unification_table.reverse(undo),
-            UndoLog::RegionConstraintCollector(undo) => self.region_constraints.reverse(undo),
+            UndoLog::TypeVariables(undo) => self.type_variable_storage.reverse(undo),
+            UndoLog::ConstUnificationTable(undo) => self.const_unification_storage.reverse(undo),
+            UndoLog::IntUnificationTable(undo) => self.int_unification_storage.reverse(undo),
+            UndoLog::FloatUnificationTable(undo) => self.float_unification_storage.reverse(undo),
+            UndoLog::RegionConstraintCollector(undo) => {
+                self.region_constraints.as_mut().unwrap().reverse(undo)
+            }
             UndoLog::RegionUnificationTable(undo) => {
-                self.region_constraints.unification_table.reverse(undo)
+                self.region_constraints.as_mut().unwrap().unification_table.reverse(undo)
             }
             UndoLog::ProjectionCache(undo) => self.projection_cache.reverse(undo),
             UndoLog::PushRegionObligation => {
@@ -163,58 +152,52 @@ where
     }
 }
 
-impl<'tcx> Snapshots<UndoLog<'tcx>> for InferCtxtUndoLogs<'tcx> {
-    type Snapshot = Snapshot<'tcx>;
-    fn actions_since_snapshot(&self, snapshot: &Self::Snapshot) -> &[UndoLog<'tcx>] {
-        &self.logs[snapshot.undo_len..]
-    }
-
-    fn start_snapshot(&mut self) -> Self::Snapshot {
-        self.num_open_snapshots += 1;
-        Snapshot { undo_len: self.logs.len(), _marker: PhantomData }
-    }
-
-    fn rollback_to<R>(&mut self, values: impl FnOnce() -> R, snapshot: Self::Snapshot)
-    where
-        R: Rollback<UndoLog<'tcx>>,
-    {
+impl<'tcx> InferCtxtInner<'tcx> {
+    pub fn rollback_to(&mut self, snapshot: Snapshot<'tcx>) {
         debug!("rollback_to({})", snapshot.undo_len);
-        self.assert_open_snapshot(&snapshot);
+        self.undo_log.assert_open_snapshot(&snapshot);
 
-        if self.logs.len() > snapshot.undo_len {
-            let mut values = values();
-            while self.logs.len() > snapshot.undo_len {
-                values.reverse(self.logs.pop().unwrap());
-            }
+        while self.undo_log.logs.len() > snapshot.undo_len {
+            let undo = self.undo_log.logs.pop().unwrap();
+            self.reverse(undo);
         }
 
-        if self.num_open_snapshots == 1 {
+        if self.undo_log.num_open_snapshots == 1 {
             // The root snapshot. It's safe to clear the undo log because
             // there's no snapshot further out that we might need to roll back
             // to.
             assert!(snapshot.undo_len == 0);
-            self.logs.clear();
+            self.undo_log.logs.clear();
         }
 
-        self.num_open_snapshots -= 1;
+        self.undo_log.num_open_snapshots -= 1;
     }
 
-    fn commit(&mut self, snapshot: Self::Snapshot) {
+    pub fn commit(&mut self, snapshot: Snapshot<'tcx>) {
         debug!("commit({})", snapshot.undo_len);
 
-        if self.num_open_snapshots == 1 {
+        if self.undo_log.num_open_snapshots == 1 {
             // The root snapshot. It's safe to clear the undo log because
             // there's no snapshot further out that we might need to roll back
             // to.
             assert!(snapshot.undo_len == 0);
-            self.logs.clear();
+            self.undo_log.logs.clear();
         }
 
-        self.num_open_snapshots -= 1;
+        self.undo_log.num_open_snapshots -= 1;
     }
 }
 
 impl<'tcx> InferCtxtUndoLogs<'tcx> {
+    pub fn actions_since_snapshot(&self, snapshot: &Snapshot<'tcx>) -> &[UndoLog<'tcx>] {
+        &self.logs[snapshot.undo_len..]
+    }
+
+    pub fn start_snapshot(&mut self) -> Snapshot<'tcx> {
+        self.num_open_snapshots += 1;
+        Snapshot { undo_len: self.logs.len(), _marker: PhantomData }
+    }
+
     pub(crate) fn region_constraints_in_snapshot(
         &self,
         s: &Snapshot<'tcx>,

src/librustc_infer/traits/project.rs

@@ -65,7 +65,7 @@ impl<'tcx, T> Normalized<'tcx, T> {
 //
 // FIXME: we probably also want some sort of cross-infcx cache here to
 // reduce the amount of duplication. Let's see what we get with the Chalk reforms.
-pub struct ProjectionCache<'tcx, 'a> {
+pub struct ProjectionCache<'a, 'tcx> {
     map: &'a mut SnapshotMapStorage<ProjectionCacheKey<'tcx>, ProjectionCacheEntry<'tcx>>,
     undo_log: &'a mut InferCtxtUndoLogs<'tcx>,
 }
@@ -98,12 +98,12 @@ impl<'tcx> ProjectionCacheStorage<'tcx> {
     pub(crate) fn with_log<'a>(
         &'a mut self,
         undo_log: &'a mut InferCtxtUndoLogs<'tcx>,
-    ) -> ProjectionCache<'tcx, 'a> {
+    ) -> ProjectionCache<'a, 'tcx> {
         ProjectionCache { map: &mut self.map, undo_log }
     }
 }
 
-impl<'tcx> ProjectionCache<'tcx, '_> {
+impl<'tcx> ProjectionCache<'_, 'tcx> {
     fn map(
         &mut self,
     ) -> SnapshotMapRef<