-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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>, | ||
|
@@ -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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notice the check here is simpler than the one with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Conjecture on my part: I wouldn't expect it to ? It wouldn't constrain the variable to add it as an upper bound, it's already the We'll see when we do the Zoom session ^^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was the zoom session recorded somewhere btw? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'll likely happen tomorrow |
||
&& 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 | ||
|
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() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, unrelated to this PR, just out of curiosity / to make sure I understand this part correctly: would this second branch already cover the first? (at the cost of breaking the "symmetry" in the code)