Skip to content

Commit

Permalink
make member constraints pick static if no upper bounds
Browse files Browse the repository at this point in the history
The current member constraint algorithm has a failure mode when it encounters a
variable `'0` and the only constraint is that `':static: '0`. In that case,
there are no upper bounds, or at least no non-trivial upper bounds.  As a
result, we are not able to rule out any of the choices, so if you have a
constraint like `'0 member ['a, 'b, 'static]`, where `'a` and `'b` are
unrelated, then the algorithm gets stuck as there is no 'least choice' from
that set.

The tweak in this commit changes the algorithm so that *if* there are no upper
bounds (and hence `'0` can get as large as desired without creating a region
check error), it will just pick `'static`. This should only occur in cases
where the data is flowing out from a `'static` value.

This change is probably *not* right for impl Trait in let bindings, but those
are challenging with member constraints anyway, and not currently supported.
Furthermore, this change is not needed in a polonius-like formulation, which
effectively permits "ad-hoc intersections" of lifetimes as the value for a
region, and hence could give a value like `'a ^ 'b` as the resulting lifetime.
Therefore I think there isn't forwards compat danger here. (famous last words?)
  • Loading branch information
nikomatsakis committed Sep 17, 2021
1 parent 207d955 commit b893cff
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 37 deletions.
79 changes: 43 additions & 36 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 All @@ -730,46 +733,50 @@ impl<'tcx> RegionInferenceContext<'tcx> {
return false;
}

// Otherwise, we need to find the minimum remaining choice, if
// any, and take that.
debug!("apply_member_constraint: choice_regions remaining are {:#?}", choice_regions);
let min = |r1: ty::RegionVid, r2: ty::RegionVid| -> Option<ty::RegionVid> {
let r1_outlives_r2 = self.universal_region_relations.outlives(r1, r2);
let r2_outlives_r1 = self.universal_region_relations.outlives(r2, r1);
match (r1_outlives_r2, r2_outlives_r1) {
(true, true) => Some(r1.min(r2)),
(true, false) => Some(r2),
(false, true) => Some(r1),
(false, false) => None,
}
};
let mut min_choice = choice_regions[0];
for &other_option in &choice_regions[1..] {
debug!(
"apply_member_constraint: min_choice={:?} other_option={:?}",
min_choice, other_option,
);
match min(min_choice, other_option) {
Some(m) => min_choice = m,
None => {
debug!(
"apply_member_constraint: {:?} and {:?} are incomparable; no min choice",
min_choice, other_option,
);
return false;
// If there WERE no upper bounds (apart from static), and static is one of the options,
// then we can just pick that (c.f. #63033).
let choice = if !any_upper_bounds && choice_regions.contains(&fr_static) {
fr_static
} else {
// Otherwise, we need to find the minimum remaining choice, if
// any, and take that.
debug!("apply_member_constraint: choice_regions remaining are {:#?}", choice_regions);
let min = |r1: ty::RegionVid, r2: ty::RegionVid| -> Option<ty::RegionVid> {
let r1_outlives_r2 = self.universal_region_relations.outlives(r1, r2);
let r2_outlives_r1 = self.universal_region_relations.outlives(r2, r1);
match (r1_outlives_r2, r2_outlives_r1) {
(true, true) => Some(r1.min(r2)),
(true, false) => Some(r2),
(false, true) => Some(r1),
(false, false) => None,
}
};
let mut min_choice = choice_regions[0];
for &other_option in &choice_regions[1..] {
debug!(
"apply_member_constraint: min_choice={:?} other_option={:?}",
min_choice, other_option,
);
match min(min_choice, other_option) {
Some(m) => min_choice = m,
None => {
debug!(
"apply_member_constraint: {:?} and {:?} are incomparable; no min choice",
min_choice, other_option,
);
return false;
}
}
}
}
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) {
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
21 changes: 20 additions & 1 deletion 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,31 @@ 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);

// If there are no upper bounds, and static is a choice (in practice, it always is),
// then we should just pick static.
if member_upper_bounds.is_empty()
&& member_constraint.choice_regions.contains(&&ty::RegionKind::ReStatic)
{
debug!("enforce_member_constraint: selecting 'static since there are no upper bounds",);
*var_values.value_mut(member_vid) = VarValue::Value(self.tcx().lifetimes.re_static);
return true;
}

// 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 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() {}

0 comments on commit b893cff

Please sign in to comment.