Skip to content

Commit

Permalink
Auto merge of #108691 - aliemjay:closure-subject, r=jackh726
Browse files Browse the repository at this point in the history 
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 #108635
Fixes #107426

Commit 2:
Fixes #108639

Commit 3:
Fixes #107516
  • Loading branch information
@bors
bors committed Mar 7, 2023
2 parents 81be7b8 + 427dc18 commit 8824994
Show file tree
Hide file tree
Showing 12 changed files with 214 additions and 154 deletions.
32 changes: 18 additions & 14 deletions compiler/rustc_borrowck/src/nll.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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, "|")?;
}
}
Expand Down Expand Up @@ -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>,
) {
Expand Down Expand Up @@ -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(())
})
Expand All @@ -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(())
}
Expand Down
137 changes: 51 additions & 86 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_borrowck/src/type_check/constraint_conversion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 0 additions & 25 deletions compiler/rustc_borrowck/src/type_check/free_region_relations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
60 changes: 48 additions & 12 deletions compiler/rustc_middle/src/mir/query.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand Down
Loading

0 comments on commit 8824994

Please sign in to comment.