Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make member constraints pick static if no upper bounds #89056

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// free region that must outlive the member region `R0` (`UB:
// R0`). Therefore, we need only keep an option `O` if `UB: O`
// for all UB.
let fr_static = self.universal_regions.fr_static;
let rev_scc_graph = self.reverse_scc_graph();
let universal_region_relations = &self.universal_region_relations;
for ub in rev_scc_graph.upper_bounds(scc) {
let mut any_upper_bounds = false;
for ub in rev_scc_graph.upper_bounds(scc).filter(|ub| *ub != fr_static) {
debug!("apply_member_constraint: ub={:?}", ub);
choice_regions.retain(|&o_r| universal_region_relations.outlives(ub, o_r));
any_upper_bounds = true;
}
debug!("apply_member_constraint: after ub, choice_regions={:?}", choice_regions);

Expand Down Expand Up @@ -756,20 +759,38 @@ impl<'tcx> RegionInferenceContext<'tcx> {
"apply_member_constraint: {:?} and {:?} are incomparable; no min choice",
min_choice, other_option,
);

// If there is no minimum choice, then we *may* wish to use `'static`
// instead, but only if there are no upper bounds, and `'static` is a valid
// choice.
if !any_upper_bounds && choice_regions.contains(&fr_static) {
return self.apply_member_constraint_choice(
scc,
member_constraint_index,
fr_static,
);
}

// Otherwise, we give up.
return false;
}
}
}
self.apply_member_constraint_choice(scc, member_constraint_index, min_choice)
}

let min_choice_scc = self.constraint_sccs.scc(min_choice);
debug!(
"apply_member_constraint: min_choice={:?} best_choice_scc={:?}",
min_choice, min_choice_scc,
);
if self.scc_values.add_region(scc, min_choice_scc) {
fn apply_member_constraint_choice(
&mut self,
scc: ConstraintSccIndex,
member_constraint_index: NllMemberConstraintIndex,
choice: ty::RegionVid,
) -> bool {
let choice_scc = self.constraint_sccs.scc(choice);
debug!("apply_member_constraint: min_choice={:?} best_choice_scc={:?}", choice, choice_scc,);
if self.scc_values.add_region(scc, choice_scc) {
self.member_constraints_applied.push(AppliedMemberConstraint {
member_region_scc: scc,
min_choice,
min_choice: choice,
member_constraint_index,
});

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,9 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
}

let fr_fn_body = self.infcx.next_nll_region_var(FR).to_region_vid();
let num_universals = self.infcx.num_region_vars();
debug!("build: fr_fn_Body = {:?}", fr_fn_body);

let num_universals = self.infcx.num_region_vars();
debug!("build: global regions = {}..{}", FIRST_GLOBAL_INDEX, first_extern_index);
debug!("build: extern regions = {}..{}", first_extern_index, first_local_index);
debug!("build: local regions = {}..{}", first_local_index, num_universals);
Expand Down
43 changes: 31 additions & 12 deletions compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
///
/// From that list, we look for a *minimal* option `'c_min`. If we
/// find one, then we can enforce that `'r: 'c_min`.
///
/// Alternatively, if we find that there are *NO* upper bounds of `'r`
/// apart from `'static`, and `'static` is one of choices, then
/// we set `'r` to `'static` (c.f. #63033).
fn enforce_member_constraint(
&self,
graph: &RegionGraph<'tcx>,
Expand All @@ -287,16 +291,21 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
VarValue::ErrorValue => return false,
VarValue::Value(r) => r,
};
debug!("enforce_member_constraint: lower_bound={:#?}", member_lower_bound);
if member_constraint.choice_regions.contains(&member_lower_bound) {
debug!("enforce_member_constraint: lower bound is already a valid choice");
return false;
}

// Find all the "upper bounds" -- that is, each region `b` such that
// `r0 <= b` must hold.
let (member_upper_bounds, ..) =
self.collect_bounding_regions(graph, member_vid, OUTGOING, None);
debug!("enforce_member_constraint: upper_bounds={:#?}", member_upper_bounds);

// Get an iterator over the *available choice* -- that is,
// each choice region `c` where `lb <= c` and `c <= ub` for all the
// upper bounds `ub`.
debug!("enforce_member_constraint: upper_bounds={:#?}", member_upper_bounds);
let mut options = member_constraint.choice_regions.iter().filter(|option| {
self.sub_concrete_regions(member_lower_bound, option)
&& member_upper_bounds
Expand All @@ -307,19 +316,32 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
// If there is more than one option, we only make a choice if
// there is a single *least* choice -- i.e., some available
// region that is `<=` all the others.
let mut least_choice: ty::Region<'tcx> = match options.next() {
let mut choice: ty::Region<'tcx> = match options.next() {
Some(&r) => r,
None => return false,
};
debug!("enforce_member_constraint: least_choice={:?}", least_choice);
debug!("enforce_member_constraint: least_choice={:?}", choice);
for &option in options {
debug!("enforce_member_constraint: option={:?}", option);
if !self.sub_concrete_regions(least_choice, option) {
if self.sub_concrete_regions(option, least_choice) {
if !self.sub_concrete_regions(choice, option) {
if self.sub_concrete_regions(option, choice) {
debug!("enforce_member_constraint: new least choice");
least_choice = option;
choice = option;
} else {
debug!("enforce_member_constraint: no least choice");

// Pick static if:
// * There is no least choice (which we just found to be true)
// * There are no upper bounds (so the region can grow to any size)
// * Static is a choice (in practice, it always is)
if member_upper_bounds.is_empty()
&& member_constraint.choice_regions.contains(&&ty::RegionKind::ReStatic)
{
choice = self.tcx().lifetimes.re_static;
break;
}

// Otherwise give up
return false;
}
}
Expand All @@ -334,13 +356,10 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
// choice is needed here so that we don't end up in a cycle of
// `expansion` changing the region one way and the code here changing
// it back.
let lub = self.lub_concrete_regions(least_choice, member_lower_bound);
debug!(
"enforce_member_constraint: final least choice = {:?}\nlub = {:?}",
least_choice, lub
);
let lub = self.lub_concrete_regions(choice, member_lower_bound);
debug!("enforce_member_constraint: final choice = {:?}\nlub = {:?}", choice, lub);
if lub != member_lower_bound {
*var_values.value_mut(member_vid) = VarValue::Value(least_choice);
*var_values.value_mut(member_vid) = VarValue::Value(choice);
true
} else {
false
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/async-await/multiple-lifetimes/two-refs-and-a-dyn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Regression test for #63033. The scenario here is:
//
// - The returned future captures the `Box<dyn T>`, which is shorthand for `Box<dyn T + 'static>`.
// - The actual value that gets captured is `Box<dyn T + '?0>` where `'static: '?0`
// - We generate a member constraint `'?0 member ['a, 'b, 'static]`
// - None of those regions are a "least choice", so we got stuck
//
// After the fix, we now select `'static` in cases where there are no upper bounds (apart from
// 'static).
//
// edition:2018
// check-pass

#![allow(dead_code)]
trait T {}
struct S;
impl S {
async fn f<'a, 'b>(_a: &'a S, _b: &'b S, _c: Box<dyn T>) {}
}
fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Regression test for #63033. The scenario here is:
//
// - The returned future captures the &'static String`
// - The actual value that gets captured is `&'?0 String` where `'static: '?0`
// - We generate a member constraint `'?0 member ['a, 'b, 'static]`
// - None of those regions are a "least choice", so we got stuck
//
// After the fix, we now select `'static` in cases where there are no upper bounds (apart from
// 'static).
//
// edition:2018
// check-pass

async fn test<'a, 'b>(test: &'a String, test2: &'b String, test3: &'static String) {}

fn main() {}
19 changes: 19 additions & 0 deletions src/test/ui/upper-bound-from-type-param.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test found by crater run. The scenario here is that we have
// a region variable `?0` from the `impl Future` with no upper bounds
// and a member constraint of `?0 in ['a, 'static]`. If we pick `'static`,
// however, we then fail the check that `F: ?0` (since we only know that
// F: ?a). The problem here is that our upper bound detection
// doesn't consider "type tests" like `F: 'x`. This was fixed by
// preferring to pick a least choice and only using static as a last resort.
//
// edition:2018
// check-pass

trait Future {}
impl Future for () {}

fn sink_error1<'a, F: 'a>(f: F) -> impl Future + 'a {
sink_error2(f) // error: `F` may not live long enough
}
fn sink_error2<'a, F: 'a>(_: F) -> impl Future + 'a {}
fn main() {}