Skip to content

Commit

Permalink
Prefer regions with an external_name in approx_universal_upper_bound
Browse files Browse the repository at this point in the history
Fixes #75785

When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static

To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue #75785

This PR `approx_universal_upper_bound` to prefer regions with an
`external_name`. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.
  • Loading branch information
Aaron1011 committed Dec 17, 2020
1 parent d23e084 commit 419d3ae
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 2 deletions.
18 changes: 17 additions & 1 deletion compiler/rustc_mir/src/borrow_check/region_infer/mod.rs
Expand Up @@ -1145,8 +1145,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
for ur in self.scc_values.universal_regions_outlived_by(r_scc) {
let new_lub = self.universal_region_relations.postdom_upper_bound(lub, ur);
debug!("approx_universal_upper_bound: ur={:?} lub={:?} new_lub={:?}", ur, lub, new_lub);
// The upper bound of two non-static regions is static: this
// means we know nothing about the relationship between these
// two regions. Pick a 'better' one to use when constructing
// a diagnostic
if ur != static_r && lub != static_r && new_lub == static_r {
lub = std::cmp::min(ur, lub);
// Prefer the region with an `external_name` - this
// indicates that the region is early-bound, so working with
// it can produce a nicer error.
if self.region_definition(ur).external_name.is_some() {
lub = ur;
} else if self.region_definition(lub).external_name.is_some() {
// Leave lub unchanged
} else {
// If we get here, we don't have any reason to prefer
// one region over the other. Just pick the
// one with the lower index for now.
lub = std::cmp::min(ur, lub);
}
} else {
lub = new_lub;
}
Expand Down
Expand Up @@ -2,7 +2,7 @@ error[E0506]: cannot assign to `*x` because it is borrowed
--> $DIR/issue-74072-lifetime-name-annotations.rs:9:5
|
LL | pub async fn async_fn(x: &mut i32) -> &i32 {
| - let's call the lifetime of this reference `'1`
| - let's call the lifetime of this reference `'1`
LL | let y = &*x;
| --- borrow of `*x` occurs here
LL | *x += 1;
Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/async-await/issue-75785-confusing-named-region.rs
@@ -0,0 +1,13 @@
// edition:2018
//
// Regression test for issue #75785
// Tests that we don't point to a confusing named
// region when emitting a diagnostic

pub async fn async_fn(x: &mut i32) -> (&i32, &i32) {
let y = &*x;
*x += 1; //~ ERROR cannot assign to
(&32, y)
}

fn main() {}
15 changes: 15 additions & 0 deletions src/test/ui/async-await/issue-75785-confusing-named-region.stderr
@@ -0,0 +1,15 @@
error[E0506]: cannot assign to `*x` because it is borrowed
--> $DIR/issue-75785-confusing-named-region.rs:9:5
|
LL | pub async fn async_fn(x: &mut i32) -> (&i32, &i32) {
| - let's call the lifetime of this reference `'1`
LL | let y = &*x;
| --- borrow of `*x` occurs here
LL | *x += 1;
| ^^^^^^^ assignment to borrowed `*x` occurs here
LL | (&32, y)
| -------- returning this value requires that `*x` is borrowed for `'1`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0506`.

0 comments on commit 419d3ae

Please sign in to comment.