Auto merge of #108861 - b-naber:eager-nll-type-relating, r=lcnr

Make NLL Type Relating Eager

We previously instantiated bound regions in nll type relating lazily. Making this eager is more consistent with how we handle type relating in [`higher_ranked_sub`](0a3b557d52/compiler/rustc_infer/src/infer/higher_ranked/mod.rs (L28)) and should allow us to short circuit in case there's structural equality.

compiler/rustc_borrowck/src/lib.rs

@@ -510,16 +510,11 @@ impl<'cx, 'tcx> BorrowckInferCtxt<'cx, 'tcx> {
             .as_var()
             .unwrap_or_else(|| bug!("expected RegionKind::RegionVar on {:?}", next_region));
 
-        if cfg!(debug_assertions) {
+        if cfg!(debug_assertions) && !self.inside_canonicalization_ctxt() {
             debug!("inserting vid {:?} with origin {:?} into var_to_origin", vid, origin);
             let ctxt = get_ctxt_fn();
             let mut var_to_origin = self.reg_var_to_origin.borrow_mut();
-            let prev = var_to_origin.insert(vid, ctxt);
+            var_to_origin.insert(vid, ctxt);
-
-            // This only makes sense if not called in a canonicalization context. If this
-            // ever changes we either want to get rid of `BorrowckInferContext::reg_var_to_origin`
-            // or modify how we track nll region vars for that map.
-            assert!(matches!(prev, None));
         }
 
         next_region
@@ -539,16 +534,11 @@ impl<'cx, 'tcx> BorrowckInferCtxt<'cx, 'tcx> {
             .as_var()
             .unwrap_or_else(|| bug!("expected RegionKind::RegionVar on {:?}", next_region));
 
-        if cfg!(debug_assertions) {
+        if cfg!(debug_assertions) && !self.inside_canonicalization_ctxt() {
             debug!("inserting vid {:?} with origin {:?} into var_to_origin", vid, origin);
             let ctxt = get_ctxt_fn();
             let mut var_to_origin = self.reg_var_to_origin.borrow_mut();
-            let prev = var_to_origin.insert(vid, ctxt);
+            var_to_origin.insert(vid, ctxt);
-
-            // This only makes sense if not called in a canonicalization context. If this
-            // ever changes we either want to get rid of `BorrowckInferContext::reg_var_to_origin`
-            // or modify how we track nll region vars for that map.
-            assert!(matches!(prev, None));
         }
 
         next_region

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -256,11 +256,12 @@ fn sccs_info<'cx, 'tcx>(
 
     let mut var_to_origin_sorted = var_to_origin.clone().into_iter().collect::<Vec<_>>();
     var_to_origin_sorted.sort_by_key(|vto| vto.0);
-    let mut debug_str = "region variables to origins:\n".to_string();
+
+    let mut reg_vars_to_origins_str = "region variables to origins:\n".to_string();
     for (reg_var, origin) in var_to_origin_sorted.into_iter() {
-        debug_str.push_str(&format!("{:?}: {:?}\n", reg_var, origin));
+        reg_vars_to_origins_str.push_str(&format!("{:?}: {:?}\n", reg_var, origin));
     }
-    debug!(debug_str);
+    debug!("{}", reg_vars_to_origins_str);
 
     let num_components = sccs.scc_data().ranges().len();
     let mut components = vec![FxIndexSet::default(); num_components];
@@ -275,12 +276,12 @@ fn sccs_info<'cx, 'tcx>(
     for (scc_idx, reg_vars_origins) in components.iter().enumerate() {
         let regions_info = reg_vars_origins.clone().into_iter().collect::<Vec<_>>();
         components_str.push_str(&format!(
-            "{:?}: {:?})",
+            "{:?}: {:?},\n)",
             ConstraintSccIndex::from_usize(scc_idx),
             regions_info,
         ))
     }
-    debug!(components_str);
+    debug!("{}", components_str);
 
     // calculate the best representative for each component
     let components_representatives = components

compiler/rustc_borrowck/src/type_check/relate_tys.rs

@@ -132,9 +132,12 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx>
 
         let reg_var =
             reg.as_var().unwrap_or_else(|| bug!("expected region {:?} to be of kind ReVar", reg));
-        let mut var_to_origin = self.type_checker.infcx.reg_var_to_origin.borrow_mut();
+
-        let prev = var_to_origin.insert(reg_var, RegionCtxt::Placeholder(reg_info));
+        if cfg!(debug_assertions) && !self.type_checker.infcx.inside_canonicalization_ctxt() {
-        assert!(matches!(prev, None));
+            let mut var_to_origin = self.type_checker.infcx.reg_var_to_origin.borrow_mut();
+            debug!(?reg_var);
+            var_to_origin.insert(reg_var, RegionCtxt::Placeholder(reg_info));
+        }
 
         reg
     }
@@ -149,14 +152,9 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx>
         let reg_var =
             reg.as_var().unwrap_or_else(|| bug!("expected region {:?} to be of kind ReVar", reg));
 
-        if cfg!(debug_assertions) {
+        if cfg!(debug_assertions) && !self.type_checker.infcx.inside_canonicalization_ctxt() {
             let mut var_to_origin = self.type_checker.infcx.reg_var_to_origin.borrow_mut();
-            let prev = var_to_origin.insert(reg_var, RegionCtxt::Existential(None));
+            var_to_origin.insert(reg_var, RegionCtxt::Existential(None));
-
-            // It only makes sense to track region vars in non-canonicalization contexts. If this
-            // ever changes we either want to get rid of `BorrowckInferContext::reg_var_to_origin`
-            // or modify how we track nll region vars for that map.
-            assert!(matches!(prev, None));
         }
 
         reg

compiler/rustc_infer/src/infer/at.rs

@@ -30,6 +30,8 @@ use super::*;
 use rustc_middle::ty::relate::{Relate, TypeRelation};
 use rustc_middle::ty::{Const, ImplSubject};
 
+use std::cell::Cell;
+
 /// Whether we should define opaque types or just treat them opaquely.
 ///
 /// Currently only used to prevent predicate matching from matching anything
@@ -82,6 +84,7 @@ impl<'tcx> InferCtxt<'tcx> {
             in_snapshot: self.in_snapshot.clone(),
             universe: self.universe.clone(),
             intercrate: self.intercrate,
+            inside_canonicalization_ctxt: Cell::new(self.inside_canonicalization_ctxt()),
         }
     }
 }

compiler/rustc_infer/src/infer/canonical/canonicalizer.rs

@@ -561,6 +561,8 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
     where
         V: TypeFoldable<TyCtxt<'tcx>>,
     {
+        let _inside_canonical_ctxt_guard = infcx.set_canonicalization_ctxt();
+
         let needs_canonical_flags = if canonicalize_region_mode.any() {
             TypeFlags::NEEDS_INFER |
             TypeFlags::HAS_FREE_REGIONS | // `HAS_RE_PLACEHOLDER` implies `HAS_FREE_REGIONS`

compiler/rustc_infer/src/infer/mod.rs

@@ -39,6 +39,7 @@ use rustc_span::Span;
 
 use std::cell::{Cell, RefCell};
 use std::fmt;
+use std::ops::Drop;
 
 use self::combine::CombineFields;
 use self::error_reporting::TypeErrCtxt;
@@ -342,6 +343,11 @@ pub struct InferCtxt<'tcx> {
     /// there is no type that the user could *actually name* that
     /// would satisfy it. This avoids crippling inference, basically.
     pub intercrate: bool,
+
+    /// Flag that is set when we enter canonicalization. Used for debugging to ensure
+    /// that we only collect region information for `BorrowckInferCtxt::reg_var_to_origin`
+    /// inside non-canonicalization contexts.
+    inside_canonicalization_ctxt: Cell<bool>,
 }
 
 /// See the `error_reporting` module for more details.
@@ -633,6 +639,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
             skip_leak_check: Cell::new(false),
             universe: Cell::new(ty::UniverseIndex::ROOT),
             intercrate,
+            inside_canonicalization_ctxt: Cell::new(false),
         }
     }
 }
@@ -1728,6 +1735,31 @@ impl<'tcx> InferCtxt<'tcx> {
             }
         }
     }
+
+    pub fn inside_canonicalization_ctxt(&self) -> bool {
+        self.inside_canonicalization_ctxt.get()
+    }
+
+    pub fn set_canonicalization_ctxt(&self) -> CanonicalizationCtxtGuard<'_, 'tcx> {
+        let prev_ctxt = self.inside_canonicalization_ctxt();
+        self.inside_canonicalization_ctxt.set(true);
+        CanonicalizationCtxtGuard { prev_ctxt, infcx: self }
+    }
+
+    fn set_canonicalization_ctxt_to(&self, ctxt: bool) {
+        self.inside_canonicalization_ctxt.set(ctxt);
+    }
+}
+
+pub struct CanonicalizationCtxtGuard<'cx, 'tcx> {
+    prev_ctxt: bool,
+    infcx: &'cx InferCtxt<'tcx>,
+}
+
+impl<'cx, 'tcx> Drop for CanonicalizationCtxtGuard<'cx, 'tcx> {
+    fn drop(&mut self) {
+        self.infcx.set_canonicalization_ctxt_to(self.prev_ctxt)
+    }
 }
 
 impl<'tcx> TypeErrCtxt<'_, 'tcx> {

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

@@ -28,6 +28,7 @@ use crate::traits::{Obligation, PredicateObligations};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::traits::ObligationCause;
 use rustc_middle::ty::error::TypeError;
+use rustc_middle::ty::fold::FnMutDelegate;
 use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation};
 use rustc_middle::ty::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor};
 use rustc_middle::ty::{self, InferConst, Ty, TyCtxt};
@@ -55,21 +56,6 @@ where
     ambient_variance: ty::Variance,
 
     ambient_variance_info: ty::VarianceDiagInfo<'tcx>,
-
-    /// When we pass through a set of binders (e.g., when looking into
-    /// a `fn` type), we push a new bound region scope onto here. This
-    /// will contain the instantiated region for each region in those
-    /// binders. When we then encounter a `ReLateBound(d, br)`, we can
-    /// use the De Bruijn index `d` to find the right scope, and then
-    /// bound region name `br` to find the specific instantiation from
-    /// within that scope. See `replace_bound_region`.
-    ///
-    /// This field stores the instantiations for late-bound regions in
-    /// the `a` type.
-    a_scopes: Vec<BoundRegionScope<'tcx>>,
-
-    /// Same as `a_scopes`, but for the `b` type.
-    b_scopes: Vec<BoundRegionScope<'tcx>>,
 }
 
 pub trait TypeRelatingDelegate<'tcx> {
@@ -147,8 +133,6 @@ where
             delegate,
             ambient_variance,
             ambient_variance_info: ty::VarianceDiagInfo::default(),
-            a_scopes: vec![],
-            b_scopes: vec![],
         }
     }
 
@@ -166,88 +150,6 @@ where
         }
     }
 
-    fn create_scope(
-        &mut self,
-        value: ty::Binder<'tcx, impl Relate<'tcx>>,
-        universally_quantified: UniversallyQuantified,
-    ) -> BoundRegionScope<'tcx> {
-        let mut scope = BoundRegionScope::default();
-
-        // Create a callback that creates (via the delegate) either an
-        // existential or placeholder region as needed.
-        let mut next_region = {
-            let delegate = &mut self.delegate;
-            let mut lazy_universe = None;
-            move |br: ty::BoundRegion| {
-                if universally_quantified.0 {
-                    // The first time this closure is called, create a
-                    // new universe for the placeholders we will make
-                    // from here out.
-                    let universe = lazy_universe.unwrap_or_else(|| {
-                        let universe = delegate.create_next_universe();
-                        lazy_universe = Some(universe);
-                        universe
-                    });
-
-                    let placeholder = ty::PlaceholderRegion { universe, name: br.kind };
-                    delegate.next_placeholder_region(placeholder)
-                } else {
-                    delegate.next_existential_region_var(true, br.kind.get_name())
-                }
-            }
-        };
-
-        value.skip_binder().visit_with(&mut ScopeInstantiator {
-            next_region: &mut next_region,
-            target_index: ty::INNERMOST,
-            bound_region_scope: &mut scope,
-        });
-
-        scope
-    }
-
-    /// When we encounter binders during the type traversal, we record
-    /// the value to substitute for each of the things contained in
-    /// that binder. (This will be either a universal placeholder or
-    /// an existential inference variable.) Given the De Bruijn index
-    /// `debruijn` (and name `br`) of some binder we have now
-    /// encountered, this routine finds the value that we instantiated
-    /// the region with; to do so, it indexes backwards into the list
-    /// of ambient scopes `scopes`.
-    fn lookup_bound_region(
-        debruijn: ty::DebruijnIndex,
-        br: &ty::BoundRegion,
-        first_free_index: ty::DebruijnIndex,
-        scopes: &[BoundRegionScope<'tcx>],
-    ) -> ty::Region<'tcx> {
-        // The debruijn index is a "reverse index" into the
-        // scopes listing. So when we have INNERMOST (0), we
-        // want the *last* scope pushed, and so forth.
-        let debruijn_index = debruijn.index() - first_free_index.index();
-        let scope = &scopes[scopes.len() - debruijn_index - 1];
-
-        // Find this bound region in that scope to map to a
-        // particular region.
-        scope.map[br]
-    }
-
-    /// If `r` is a bound region, find the scope in which it is bound
-    /// (from `scopes`) and return the value that we instantiated it
-    /// with. Otherwise just return `r`.
-    fn replace_bound_region(
-        &self,
-        r: ty::Region<'tcx>,
-        first_free_index: ty::DebruijnIndex,
-        scopes: &[BoundRegionScope<'tcx>],
-    ) -> ty::Region<'tcx> {
-        debug!("replace_bound_regions(scopes={:?})", scopes);
-        if let ty::ReLateBound(debruijn, br) = *r {
-            Self::lookup_bound_region(debruijn, &br, first_free_index, scopes)
-        } else {
-            r
-        }
-    }
-
     /// Push a new outlives requirement into our output set of
     /// constraints.
     fn push_outlives(
@@ -314,18 +216,9 @@ where
 
         self.infcx.inner.borrow_mut().type_variables().instantiate(vid, generalized_ty);
 
-        // The generalized values we extract from `canonical_var_values` have
-        // been fully instantiated and hence the set of scopes we have
-        // doesn't matter -- just to be sure, put an empty vector
-        // in there.
-        let old_a_scopes = std::mem::take(pair.vid_scopes(self));
-
         // Relate the generalized kind to the original one.
         let result = pair.relate_generalized_ty(self, generalized_ty);
 
-        // Restore the old scopes now.
-        *pair.vid_scopes(self) = old_a_scopes;
-
         debug!("relate_ty_var: complete, result = {:?}", result);
         result
     }
@@ -379,6 +272,97 @@ where
         trace!(a = ?a.kind(), b = ?b.kind(), "opaque type instantiated");
         Ok(a)
     }
+
+    #[instrument(skip(self), level = "debug")]
+    fn instantiate_binder_with_placeholders<T>(&mut self, binder: ty::Binder<'tcx, T>) -> T
+    where
+        T: ty::TypeFoldable<TyCtxt<'tcx>> + Copy,
+    {
+        if let Some(inner) = binder.no_bound_vars() {
+            return inner;
+        }
+
+        let mut next_region = {
+            let nll_delegate = &mut self.delegate;
+            let mut lazy_universe = None;
+
+            move |br: ty::BoundRegion| {
+                // The first time this closure is called, create a
+                // new universe for the placeholders we will make
+                // from here out.
+                let universe = lazy_universe.unwrap_or_else(|| {
+                    let universe = nll_delegate.create_next_universe();
+                    lazy_universe = Some(universe);
+                    universe
+                });
+
+                let placeholder = ty::PlaceholderRegion { universe, name: br.kind };
+                debug!(?placeholder);
+                let placeholder_reg = nll_delegate.next_placeholder_region(placeholder);
+                debug!(?placeholder_reg);
+
+                placeholder_reg
+            }
+        };
+
+        let delegate = FnMutDelegate {
+            regions: &mut next_region,
+            types: &mut |_bound_ty: ty::BoundTy| {
+                unreachable!("we only replace regions in nll_relate, not types")
+            },
+            consts: &mut |_bound_var: ty::BoundVar, _ty| {
+                unreachable!("we only replace regions in nll_relate, not consts")
+            },
+        };
+
+        let replaced = self.infcx.tcx.replace_bound_vars_uncached(binder, delegate);
+        debug!(?replaced);
+
+        replaced
+    }
+
+    #[instrument(skip(self), level = "debug")]
+    fn instantiate_binder_with_existentials<T>(&mut self, binder: ty::Binder<'tcx, T>) -> T
+    where
+        T: ty::TypeFoldable<TyCtxt<'tcx>> + Copy,
+    {
+        if let Some(inner) = binder.no_bound_vars() {
+            return inner;
+        }
+
+        let mut next_region = {
+            let nll_delegate = &mut self.delegate;
+            let mut reg_map = FxHashMap::default();
+
+            move |br: ty::BoundRegion| {
+                if let Some(ex_reg_var) = reg_map.get(&br) {
+                    return *ex_reg_var;
+                } else {
+                    let ex_reg_var =
+                        nll_delegate.next_existential_region_var(true, br.kind.get_name());
+                    debug!(?ex_reg_var);
+                    reg_map.insert(br, ex_reg_var);
+
+                    ex_reg_var
+                }
+            }
+        };
+
+        let delegate = FnMutDelegate {
+            regions: &mut next_region,
+            types: &mut |_bound_ty: ty::BoundTy| {
+                unreachable!("we only replace regions in nll_relate, not types")
+            },
+            consts: &mut |_bound_var: ty::BoundVar, _ty| {
+                unreachable!("we only replace regions in nll_relate, not consts")
+            },
+        };
+
+        let replaced = self.infcx.tcx.replace_bound_vars_uncached(binder, delegate);
+        debug!(?replaced);
+
+        replaced
+    }
 }
 
 /// When we instantiate an inference variable with a value in
@@ -396,14 +380,6 @@ trait VidValuePair<'tcx>: Debug {
     /// opposite part of the tuple from the vid).
     fn value_ty(&self) -> Ty<'tcx>;
 
-    /// Extract the scopes that apply to whichever side of the tuple
-    /// the vid was found on. See the comment where this is called
-    /// for more details on why we want them.
-    fn vid_scopes<'r, D: TypeRelatingDelegate<'tcx>>(
-        &self,
-        relate: &'r mut TypeRelating<'_, 'tcx, D>,
-    ) -> &'r mut Vec<BoundRegionScope<'tcx>>;
-
     /// Given a generalized type G that should replace the vid, relate
     /// G to the value, putting G on whichever side the vid would have
     /// appeared.
@@ -425,16 +401,6 @@ impl<'tcx> VidValuePair<'tcx> for (ty::TyVid, Ty<'tcx>) {
         self.1
     }
 
-    fn vid_scopes<'r, D>(
-        &self,
-        relate: &'r mut TypeRelating<'_, 'tcx, D>,
-    ) -> &'r mut Vec<BoundRegionScope<'tcx>>
-    where
-        D: TypeRelatingDelegate<'tcx>,
-    {
-        &mut relate.a_scopes
-    }
-
     fn relate_generalized_ty<D>(
         &self,
         relate: &mut TypeRelating<'_, 'tcx, D>,
@@ -457,16 +423,6 @@ impl<'tcx> VidValuePair<'tcx> for (Ty<'tcx>, ty::TyVid) {
         self.0
     }
 
-    fn vid_scopes<'r, D>(
-        &self,
-        relate: &'r mut TypeRelating<'_, 'tcx, D>,
-    ) -> &'r mut Vec<BoundRegionScope<'tcx>>
-    where
-        D: TypeRelatingDelegate<'tcx>,
-    {
-        &mut relate.b_scopes
-    }
-
     fn relate_generalized_ty<D>(
         &self,
         relate: &mut TypeRelating<'_, 'tcx, D>,
@@ -602,20 +558,14 @@ where
     ) -> RelateResult<'tcx, ty::Region<'tcx>> {
         debug!(?self.ambient_variance);
 
-        let v_a = self.replace_bound_region(a, ty::INNERMOST, &self.a_scopes);
-        let v_b = self.replace_bound_region(b, ty::INNERMOST, &self.b_scopes);
-
-        debug!(?v_a);
-        debug!(?v_b);
-
         if self.ambient_covariance() {
             // Covariant: &'a u8 <: &'b u8. Hence, `'a: 'b`.
-            self.push_outlives(v_a, v_b, self.ambient_variance_info);
+            self.push_outlives(a, b, self.ambient_variance_info);
         }
 
         if self.ambient_contravariance() {
             // Contravariant: &'b u8 <: &'a u8. Hence, `'b: 'a`.
-            self.push_outlives(v_b, v_a, self.ambient_variance_info);
+            self.push_outlives(b, a, self.ambient_variance_info);
         }
 
         Ok(a)
@@ -689,15 +639,6 @@ where
             // instantiation of B (i.e., B instantiated with
             // universals).
 
-            let b_scope = self.create_scope(b, UniversallyQuantified(true));
-            let a_scope = self.create_scope(a, UniversallyQuantified(false));
-
-            debug!(?a_scope, "(existential)");
-            debug!(?b_scope, "(universal)");
-
-            self.b_scopes.push(b_scope);
-            self.a_scopes.push(a_scope);
-
             // Reset the ambient variance to covariant. This is needed
             // to correctly handle cases like
             //
@@ -718,12 +659,14 @@ where
             //   subtyping (i.e., `&'b u32 <: &{P} u32`).
             let variance = std::mem::replace(&mut self.ambient_variance, ty::Variance::Covariant);
 
-            self.relate(a.skip_binder(), b.skip_binder())?;
+            // Note: the order here is important. Create the placeholders first, otherwise
+            // we assign the wrong universe to the existential!
+            let b_replaced = self.instantiate_binder_with_placeholders(b);
+            let a_replaced = self.instantiate_binder_with_existentials(a);
+
+            self.relate(a_replaced, b_replaced)?;
 
             self.ambient_variance = variance;
-
-            self.b_scopes.pop().unwrap();
-            self.a_scopes.pop().unwrap();
         }
 
         if self.ambient_contravariance() {
@@ -733,26 +676,17 @@ where
             // instantiation of B (i.e., B instantiated with
             // existentials). Opposite of above.
 
-            let a_scope = self.create_scope(a, UniversallyQuantified(true));
-            let b_scope = self.create_scope(b, UniversallyQuantified(false));
-
-            debug!(?a_scope, "(universal)");
-            debug!(?b_scope, "(existential)");
-
-            self.a_scopes.push(a_scope);
-            self.b_scopes.push(b_scope);
-
             // Reset ambient variance to contravariance. See the
             // covariant case above for an explanation.
             let variance =
                 std::mem::replace(&mut self.ambient_variance, ty::Variance::Contravariant);
 
-            self.relate(a.skip_binder(), b.skip_binder())?;
+            let a_replaced = self.instantiate_binder_with_placeholders(a);
+            let b_replaced = self.instantiate_binder_with_existentials(b);
+
+            self.relate(a_replaced, b_replaced)?;
 
             self.ambient_variance = variance;
-
-            self.b_scopes.pop().unwrap();
-            self.a_scopes.pop().unwrap();
         }
 
         Ok(a)