More rustc_infer cleanups

compiler/rustc_infer/src/infer/mod.rs

@@ -18,7 +18,7 @@ pub use relate::combine::PredicateEmittingRelation;
 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 snapshot::undo_log::InferCtxtUndoLogs;
 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 mod resolve;
 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 @@ impl<'tcx> 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 @@ impl<'tcx> InferCtxt<'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 @@ impl<'tcx> InferCtxt<'tcx> {
     /// 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 @@ impl<'tcx> InferCtxt<'tcx> {
     ///
     /// 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 @@ impl<'tcx> InferCtxt<'tcx> {
                 }
 
                 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,94 +53,44 @@ 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.
     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();
 
-    /// Borrows current value of the `free_region_map`.
-    pub fn free_region_map(&self) -> &FreeRegionMap<'tcx> {
-        &self.free_region_map
-    }
-
-    /// Borrows current `region_bound_pairs`.
-    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 {
+        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) => {
-                    self.region_bound_pairs
+                    region_bound_pairs
                         .insert(ty::OutlivesPredicate(GenericKind::Param(param_b), r_a));
                 }
                 OutlivesBound::RegionSubAlias(r_a, alias_b) => {
-                    self.region_bound_pairs
+                    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),
+                    ) => 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!
@@ -150,5 +99,21 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
                 },
             }
         }
+
+        OutlivesEnvironment {
+            param_env,
+            free_region_map: FreeRegionMap { relation: region_relation.freeze() },
+            region_bound_pairs,
+        }
+    }
+
+    /// Borrows current value of the `free_region_map`.
+    pub fn free_region_map(&self) -> &FreeRegionMap<'tcx> {
+        &self.free_region_map
+    }
+
+    /// Borrows current `region_bound_pairs`.
+    pub fn region_bound_pairs(&self) -> &RegionBoundPairs<'tcx> {
+        &self.region_bound_pairs
     }
 }

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,26 +64,22 @@ impl<'tcx> InferCtxt<'tcx> {
             }
         };
 
-        let (var_infos, data) = {
+        let storage = {
             let mut inner = self.inner.borrow_mut();
             let inner = &mut *inner;
             assert!(
                 self.tainted_by_errors().is_some() || inner.region_obligations.is_empty(),
                 "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 @@ where
         // '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)