Auto merge of rust-lang#108691 - aliemjay:closure-subject, r=jackh726

fix multiple issues when promoting type-test subject

Multiple interdependent fixes.  See linked issues for a short description of each.

When Promoting a type-test `T: 'a` from within the closure back to its parent function, there are a couple pre-existing bugs and limitations. They were exposed by the recent changes to opaque types because the type-test subject (`T`) is no longer a simple ParamTy.

Commit 1:
Fixes rust-lang#108635
Fixes rust-lang#107426

Commit 2:
Fixes rust-lang#108639

Commit 3:
Fixes rust-lang#107516

Author: bors

compiler/rustc_borrowck/src/nll.rs

@@ -10,10 +10,9 @@ use rustc_middle::mir::{
     BasicBlock, Body, ClosureOutlivesSubject, ClosureRegionRequirements, LocalKind, Location,
     Promoted,
 };
-use rustc_middle::ty::{self, OpaqueHiddenType, Region, RegionVid};
+use rustc_middle::ty::{self, OpaqueHiddenType, Region, RegionVid, TyCtxt};
 use rustc_span::symbol::sym;
 use std::env;
-use std::fmt::Debug;
 use std::io;
 use std::path::PathBuf;
 use std::rc::Rc;
@@ -325,7 +324,7 @@ pub(super) fn dump_mir_results<'tcx>(
     infcx: &BorrowckInferCtxt<'_, 'tcx>,
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
-    closure_region_requirements: &Option<ClosureRegionRequirements<'_>>,
+    closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
 ) {
     if !dump_enabled(infcx.tcx, "nll", body.source.def_id()) {
         return;
@@ -340,9 +339,11 @@ pub(super) fn dump_mir_results<'tcx>(
 
                 if let Some(closure_region_requirements) = closure_region_requirements {
                     writeln!(out, "| Free Region Constraints")?;
-                    for_each_region_constraint(closure_region_requirements, &mut |msg| {
-                        writeln!(out, "| {}", msg)
-                    })?;
+                    for_each_region_constraint(
+                        infcx.tcx,
+                        closure_region_requirements,
+                        &mut |msg| writeln!(out, "| {}", msg),
+                    )?;
                     writeln!(out, "|")?;
                 }
             }
@@ -375,7 +376,7 @@ pub(super) fn dump_annotation<'tcx>(
     infcx: &BorrowckInferCtxt<'_, 'tcx>,
     body: &Body<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
-    closure_region_requirements: &Option<ClosureRegionRequirements<'_>>,
+    closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
     opaque_type_values: &VecMap<LocalDefId, OpaqueHiddenType<'tcx>>,
     errors: &mut crate::error::BorrowckErrors<'tcx>,
 ) {
@@ -405,7 +406,7 @@ pub(super) fn dump_annotation<'tcx>(
 
         // Dump the region constraints we are imposing *between* those
         // newly created variables.
-        for_each_region_constraint(closure_region_requirements, &mut |msg| {
+        for_each_region_constraint(tcx, closure_region_requirements, &mut |msg| {
             err.note(msg);
             Ok(())
         })
@@ -426,16 +427,19 @@ pub(super) fn dump_annotation<'tcx>(
     errors.buffer_non_error_diag(err);
 }
 
-fn for_each_region_constraint(
-    closure_region_requirements: &ClosureRegionRequirements<'_>,
+fn for_each_region_constraint<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    closure_region_requirements: &ClosureRegionRequirements<'tcx>,
     with_msg: &mut dyn FnMut(&str) -> io::Result<()>,
 ) -> io::Result<()> {
     for req in &closure_region_requirements.outlives_requirements {
-        let subject: &dyn Debug = match &req.subject {
-            ClosureOutlivesSubject::Region(subject) => subject,
-            ClosureOutlivesSubject::Ty(ty) => ty,
+        let subject = match req.subject {
+            ClosureOutlivesSubject::Region(subject) => format!("{:?}", subject),
+            ClosureOutlivesSubject::Ty(ty) => {
+                format!("{:?}", ty.instantiate(tcx, |vid| tcx.mk_re_var(vid)))
+            }
         };
-        with_msg(&format!("where {:?}: {:?}", subject, req.outlived_free_region,))?;
+        with_msg(&format!("where {}: {:?}", subject, req.outlived_free_region,))?;
     }
     Ok(())
 }

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -12,8 +12,9 @@ use rustc_infer::infer::outlives::test_type_match;
 use rustc_infer::infer::region_constraints::{GenericKind, VarInfos, VerifyBound, VerifyIfEq};
 use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin, RegionVariableOrigin};
 use rustc_middle::mir::{
-    Body, ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements,
-    ConstraintCategory, Local, Location, ReturnConstraint, TerminatorKind,
+    Body, ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureOutlivesSubjectTy,
+    ClosureRegionRequirements, ConstraintCategory, Local, Location, ReturnConstraint,
+    TerminatorKind,
 };
 use rustc_middle::traits::ObligationCause;
 use rustc_middle::traits::ObligationCauseCode;
@@ -1084,18 +1085,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         true
     }
 
-    /// When we promote a type test `T: 'r`, we have to convert the
-    /// type `T` into something we can store in a query result (so
-    /// something allocated for `'tcx`). This is problematic if `ty`
-    /// contains regions. During the course of NLL region checking, we
-    /// will have replaced all of those regions with fresh inference
-    /// variables. To create a test subject, we want to replace those
-    /// inference variables with some region from the closure
-    /// signature -- this is not always possible, so this is a
-    /// fallible process. Presuming we do find a suitable region, we
-    /// will use it's *external name*, which will be a `RegionKind`
-    /// variant that can be used in query responses such as
-    /// `ReEarlyBound`.
+    /// When we promote a type test `T: 'r`, we have to replace all region
+    /// variables in the type `T` with an equal universal region from the
+    /// closure signature.
+    /// This is not always possible, so this is a fallible process.
     #[instrument(level = "debug", skip(self, infcx))]
     fn try_promote_type_test_subject(
         &self,
@@ -1104,91 +1097,63 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     ) -> Option<ClosureOutlivesSubject<'tcx>> {
         let tcx = infcx.tcx;
 
+        // Opaque types' substs may include useless lifetimes.
+        // We will replace them with ReStatic.
+        struct OpaqueFolder<'tcx> {
+            tcx: TyCtxt<'tcx>,
+        }
+        impl<'tcx> ty::TypeFolder<TyCtxt<'tcx>> for OpaqueFolder<'tcx> {
+            fn interner(&self) -> TyCtxt<'tcx> {
+                self.tcx
+            }
+            fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
+                use ty::TypeSuperFoldable as _;
+                let tcx = self.tcx;
+                let &ty::Alias(ty::Opaque, ty::AliasTy { substs, def_id, .. }) = t.kind() else {
+                    return t.super_fold_with(self);
+                };
+                let substs =
+                    std::iter::zip(substs, tcx.variances_of(def_id)).map(|(arg, v)| {
+                        match (arg.unpack(), v) {
+                            (ty::GenericArgKind::Lifetime(_), ty::Bivariant) => {
+                                tcx.lifetimes.re_static.into()
+                            }
+                            _ => arg.fold_with(self),
+                        }
+                    });
+                tcx.mk_opaque(def_id, tcx.mk_substs_from_iter(substs))
+            }
+        }
+
+        let ty = ty.fold_with(&mut OpaqueFolder { tcx });
+
         let ty = tcx.fold_regions(ty, |r, _depth| {
-            let region_vid = self.to_region_vid(r);
+            let r_vid = self.to_region_vid(r);
+            let r_scc = self.constraint_sccs.scc(r_vid);
 
             // The challenge if this. We have some region variable `r`
             // whose value is a set of CFG points and universal
             // regions. We want to find if that set is *equivalent* to
             // any of the named regions found in the closure.
-            //
-            // To do so, we compute the
-            // `non_local_universal_upper_bound`. This will be a
-            // non-local, universal region that is greater than `r`.
-            // However, it might not be *contained* within `r`, so
-            // then we further check whether this bound is contained
-            // in `r`. If so, we can say that `r` is equivalent to the
-            // bound.
-            //
-            // Let's work through a few examples. For these, imagine
-            // that we have 3 non-local regions (I'll denote them as
-            // `'static`, `'a`, and `'b`, though of course in the code
-            // they would be represented with indices) where:
-            //
-            // - `'static: 'a`
-            // - `'static: 'b`
-            //
-            // First, let's assume that `r` is some existential
-            // variable with an inferred value `{'a, 'static}` (plus
-            // some CFG nodes). In this case, the non-local upper
-            // bound is `'static`, since that outlives `'a`. `'static`
-            // is also a member of `r` and hence we consider `r`
-            // equivalent to `'static` (and replace it with
-            // `'static`).
-            //
-            // Now let's consider the inferred value `{'a, 'b}`. This
-            // means `r` is effectively `'a | 'b`. I'm not sure if
-            // this can come about, actually, but assuming it did, we
-            // would get a non-local upper bound of `'static`. Since
-            // `'static` is not contained in `r`, we would fail to
-            // find an equivalent.
-            let upper_bound = self.non_local_universal_upper_bound(region_vid);
-            if self.region_contains(region_vid, upper_bound) {
-                self.definitions[upper_bound].external_name.unwrap_or(r)
-            } else {
-                // In the case of a failure, use a `ReVar` result. This will
-                // cause the `needs_infer` later on to return `None`.
-                r
-            }
+            // To do so, we simply check every candidate `u_r` for equality.
+            self.scc_values
+                .universal_regions_outlived_by(r_scc)
+                .filter(|&u_r| !self.universal_regions.is_local_free_region(u_r))
+                .find(|&u_r| self.eval_equal(u_r, r_vid))
+                .map(|u_r| tcx.mk_re_var(u_r))
+                // In the case of a failure, use `ReErased`. We will eventually
+                // return `None` in this case.
+                .unwrap_or(tcx.lifetimes.re_erased)
         });
 
         debug!("try_promote_type_test_subject: folded ty = {:?}", ty);
 
-        // `needs_infer` will only be true if we failed to promote some region.
-        if ty.needs_infer() {
+        // This will be true if we failed to promote some region.
+        if ty.has_erased_regions() {
             return None;
         }
 
-        Some(ClosureOutlivesSubject::Ty(ty))
-    }
-
-    /// Given some universal or existential region `r`, finds a
-    /// non-local, universal region `r+` that outlives `r` at entry to (and
-    /// exit from) the closure. In the worst case, this will be
-    /// `'static`.
-    ///
-    /// This is used for two purposes. First, if we are propagated
-    /// some requirement `T: r`, we can use this method to enlarge `r`
-    /// to something we can encode for our creator (which only knows
-    /// about non-local, universal regions). It is also used when
-    /// encoding `T` as part of `try_promote_type_test_subject` (see
-    /// that fn for details).
-    ///
-    /// This is based on the result `'y` of `universal_upper_bound`,
-    /// except that it converts further takes the non-local upper
-    /// bound of `'y`, so that the final result is non-local.
-    fn non_local_universal_upper_bound(&self, r: RegionVid) -> RegionVid {
-        debug!("non_local_universal_upper_bound(r={:?}={})", r, self.region_value_str(r));
-
-        let lub = self.universal_upper_bound(r);
-
-        // Grow further to get smallest universal region known to
-        // creator.
-        let non_local_lub = self.universal_region_relations.non_local_upper_bound(lub);
-
-        debug!("non_local_universal_upper_bound: non_local_lub={:?}", non_local_lub);
-
-        non_local_lub
+        Some(ClosureOutlivesSubject::Ty(ClosureOutlivesSubjectTy::bind(tcx, ty)))
     }
 
     /// Returns a universally quantified region that outlives the

compiler/rustc_borrowck/src/type_check/constraint_conversion.rs

@@ -116,7 +116,9 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> {
             let outlived_region = closure_mapping[outlives_requirement.outlived_free_region];
             let subject = match outlives_requirement.subject {
                 ClosureOutlivesSubject::Region(re) => closure_mapping[re].into(),
-                ClosureOutlivesSubject::Ty(ty) => ty.into(),
+                ClosureOutlivesSubject::Ty(subject_ty) => {
+                    subject_ty.instantiate(self.tcx, |vid| closure_mapping[vid]).into()
+                }
             };
 
             self.category = outlives_requirement.category;

compiler/rustc_borrowck/src/type_check/free_region_relations.rs

@@ -93,31 +93,6 @@ impl UniversalRegionRelations<'_> {
         res
     }
 
-    /// Returns the "postdominating" bound of the set of
-    /// `non_local_upper_bounds` for the given region.
-    pub(crate) fn non_local_upper_bound(&self, fr: RegionVid) -> RegionVid {
-        let upper_bounds = self.non_local_upper_bounds(fr);
-
-        // In case we find more than one, reduce to one for
-        // convenience. This is to prevent us from generating more
-        // complex constraints, but it will cause spurious errors.
-        let post_dom = self.inverse_outlives.mutual_immediate_postdominator(upper_bounds);
-
-        debug!("non_local_bound: post_dom={:?}", post_dom);
-
-        post_dom
-            .and_then(|post_dom| {
-                // If the mutual immediate postdom is not local, then
-                // there is no non-local result we can return.
-                if !self.universal_regions.is_local_free_region(post_dom) {
-                    Some(post_dom)
-                } else {
-                    None
-                }
-            })
-            .unwrap_or(self.universal_regions.fr_static)
-    }
-
     /// Finds a "lower bound" for `fr` that is not local. In other
     /// words, returns the largest (*) known region `fr1` that (a) is
     /// outlived by `fr` and (b) is not local.

compiler/rustc_middle/src/mir/query.rs

@@ -8,7 +8,7 @@ use rustc_errors::ErrorGuaranteed;
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_index::bit_set::BitMatrix;
-use rustc_index::vec::IndexVec;
+use rustc_index::vec::{Idx, IndexVec};
 use rustc_span::Span;
 use rustc_target::abi::VariantIdx;
 use smallvec::SmallVec;
@@ -289,13 +289,6 @@ pub struct ConstQualifs {
 /// instance of the closure is created, the corresponding free regions
 /// can be extracted from its type and constrained to have the given
 /// outlives relationship.
-///
-/// In some cases, we have to record outlives requirements between types and
-/// regions as well. In that case, if those types include any regions, those
-/// regions are recorded using their external names (`ReStatic`,
-/// `ReEarlyBound`, `ReFree`). We use these because in a query response we
-/// cannot use `ReVar` (which is what we use internally within the rest of the
-/// NLL code).
 #[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable)]
 pub struct ClosureRegionRequirements<'tcx> {
     /// The number of external regions defined on the closure. In our
@@ -392,16 +385,59 @@ pub enum ClosureOutlivesSubject<'tcx> {
     /// Subject is a type, typically a type parameter, but could also
     /// be a projection. Indicates a requirement like `T: 'a` being
     /// passed to the caller, where the type here is `T`.
-    ///
-    /// The type here is guaranteed not to contain any free regions at
-    /// present.
-    Ty(Ty<'tcx>),
+    Ty(ClosureOutlivesSubjectTy<'tcx>),
 
     /// Subject is a free region from the closure. Indicates a requirement
     /// like `'a: 'b` being passed to the caller; the region here is `'a`.
     Region(ty::RegionVid),
 }
 
+/// Represents a `ty::Ty` for use in [`ClosureOutlivesSubject`].
+///
+/// This abstraction is necessary because the type may include `ReVar` regions,
+/// which is what we use internally within NLL code, and they can't be used in
+/// a query response.
+///
+/// DO NOT implement `TypeVisitable` or `TypeFoldable` traits, because this
+/// type is not recognized as a binder for late-bound region.
+#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
+pub struct ClosureOutlivesSubjectTy<'tcx> {
+    inner: Ty<'tcx>,
+}
+
+impl<'tcx> ClosureOutlivesSubjectTy<'tcx> {
+    /// All regions of `ty` must be of kind `ReVar` and must represent
+    /// universal regions *external* to the closure.
+    pub fn bind(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Self {
+        let inner = tcx.fold_regions(ty, |r, depth| match r.kind() {
+            ty::ReVar(vid) => {
+                let br = ty::BoundRegion {
+                    var: ty::BoundVar::new(vid.index()),
+                    kind: ty::BrAnon(vid.as_u32(), None),
+                };
+                tcx.mk_re_late_bound(depth, br)
+            }
+            _ => bug!("unexpected region in ClosureOutlivesSubjectTy: {r:?}"),
+        });
+
+        Self { inner }
+    }
+
+    pub fn instantiate(
+        self,
+        tcx: TyCtxt<'tcx>,
+        mut map: impl FnMut(ty::RegionVid) -> ty::Region<'tcx>,
+    ) -> Ty<'tcx> {
+        tcx.fold_regions(self.inner, |r, depth| match r.kind() {
+            ty::ReLateBound(debruijn, br) => {
+                debug_assert_eq!(debruijn, depth);
+                map(ty::RegionVid::new(br.var.index()))
+            }
+            _ => bug!("unexpected region {r:?}"),
+        })
+    }
+}
+
 /// The constituent parts of a mir constant of kind ADT or array.
 #[derive(Copy, Clone, Debug, HashStable)]
 pub struct DestructuredConstant<'tcx> {