Skip to content

Commit 7dfa04d

Browse files
diannedianne
committed
best_blame_constraint: don't filter constraints by sup SCC
The SCCs of the region graph are not a reliable heuristic to use for blaming an interesting constraint for diagnostics. For region errors, if the outlived region is `'static`, or the involved types are invariant in their lifetiems, there will be cycles in the constraint graph containing both the target region and the most interesting constraints to blame. To get better diagnostics in these cases, this commit removes that heuristic.
1 parent 06de82a commit 7dfa04d

File tree

101 files changed

+594
-516
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

101 files changed

+594
-516
lines changed

Diff for: ‎compiler/rustc_borrowck/src/region_infer/mod.rs

+11-49
Original file line numberDiff line numberDiff line change
@@ -1940,7 +1940,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
19401940
target_test: impl Fn(RegionVid) -> bool,
19411941
) -> (BlameConstraint<'tcx>, Vec<OutlivesConstraint<'tcx>>) {
19421942
// Find all paths
1943-
let (path, target_region) =
1943+
let (path, _) =
19441944
self.find_constraint_paths_between_regions(from_region, target_test).unwrap();
19451945
debug!(
19461946
"path={:#?}",
@@ -1972,29 +1972,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
19721972
})
19731973
.unwrap_or_else(|| ObligationCauseCode::Misc);
19741974

1975-
// To find the best span to cite, we first try to look for the
1976-
// final constraint that is interesting and where the `sup` is
1977-
// not unified with the ultimate target region. The reason
1978-
// for this is that we have a chain of constraints that lead
1979-
// from the source to the target region, something like:
1980-
//
1981-
// '0: '1 ('0 is the source)
1982-
// '1: '2
1983-
// '2: '3
1984-
// '3: '4
1985-
// '4: '5
1986-
// '5: '6 ('6 is the target)
1987-
//
1988-
// Some of those regions are unified with `'6` (in the same
1989-
// SCC). We want to screen those out. After that point, the
1990-
// "closest" constraint we have to the end is going to be the
1991-
// most likely to be the point where the value escapes -- but
1992-
// we still want to screen for an "interesting" point to
1993-
// highlight (e.g., a call site or something).
1994-
let target_scc = self.constraint_sccs.scc(target_region);
1995-
1996-
// As noted above, when reporting an error, there is typically a chain of constraints
1997-
// leading from some "source" region which must outlive some "target" region.
1975+
// When reporting an error, there is typically a chain of constraints leading from some
1976+
// "source" region which must outlive some "target" region.
19981977
// In most cases, we prefer to "blame" the constraints closer to the target --
19991978
// but there is one exception. When constraints arise from higher-ranked subtyping,
20001979
// we generally prefer to blame the source value,
@@ -2036,30 +2015,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
20362015
};
20372016

20382017
let interesting_to_blame = |constraint: &OutlivesConstraint<'tcx>| {
2039-
let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup);
2040-
2041-
if blame_source {
2042-
match constraint.category {
2043-
ConstraintCategory::OpaqueType
2018+
!matches!(
2019+
constraint.category,
2020+
ConstraintCategory::OpaqueType
20442021
| ConstraintCategory::Boring
20452022
| ConstraintCategory::BoringNoLocation
20462023
| ConstraintCategory::Internal
2047-
| ConstraintCategory::Predicate(_) => false,
2048-
ConstraintCategory::TypeAnnotation
2049-
| ConstraintCategory::Return(_)
2050-
| ConstraintCategory::Yield => true,
2051-
_ => constraint_sup_scc != target_scc,
2052-
}
2053-
} else {
2054-
!matches!(
2055-
constraint.category,
2056-
ConstraintCategory::OpaqueType
2057-
| ConstraintCategory::Boring
2058-
| ConstraintCategory::BoringNoLocation
2059-
| ConstraintCategory::Internal
2060-
| ConstraintCategory::Predicate(_)
2061-
)
2062-
}
2024+
| ConstraintCategory::Predicate(_)
2025+
)
20632026
};
20642027

20652028
let best_choice = if blame_source {
@@ -2100,10 +2063,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
21002063
Some(i) => path[i],
21012064

21022065
None => {
2103-
// If that search fails, that is.. unusual. Maybe everything
2104-
// is in the same SCC or something. In that case, find what
2105-
// appears to be the most interesting point to report to the
2106-
// user via an even more ad-hoc guess.
2066+
// If that search fails, the only constraints on the path are those that we try not
2067+
// to blame. In that case, find what appears to be the most interesting point to
2068+
// report to the user via an even more ad-hoc guess.
21072069
*path.iter().min_by_key(|p| p.category).unwrap()
21082070
}
21092071
};

Diff for: ‎tests/ui/associated-types/associated-types-project-from-hrtb-in-fn-body.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ LL | fn bar<'a, 'b, I : for<'x> Foo<&'x isize>>(
77
| lifetime `'a` defined here
88
...
99
LL | let z: I::A = if cond { x } else { y };
10-
| ^ assignment requires that `'a` must outlive `'b`
10+
| ^ assignment requires that `'b` must outlive `'a`
1111
|
12-
= help: consider adding the following bound: `'a: 'b`
12+
= help: consider adding the following bound: `'b: 'a`
1313

1414
error: lifetime may not live long enough
1515
--> $DIR/associated-types-project-from-hrtb-in-fn-body.rs:22:40
@@ -20,9 +20,9 @@ LL | fn bar<'a, 'b, I : for<'x> Foo<&'x isize>>(
2020
| lifetime `'a` defined here
2121
...
2222
LL | let z: I::A = if cond { x } else { y };
23-
| ^ assignment requires that `'b` must outlive `'a`
23+
| ^ assignment requires that `'a` must outlive `'b`
2424
|
25-
= help: consider adding the following bound: `'b: 'a`
25+
= help: consider adding the following bound: `'a: 'b`
2626

2727
help: `'a` and `'b` must be the same: replace one with the other
2828

Diff for: ‎tests/ui/associated-types/cache/project-fn-ret-invariant.oneuse.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ LL | fn baz<'a, 'b>(x: Type<'a>, y: Type<'b>) -> (Type<'a>, Type<'b>) {
77
| lifetime `'a` defined here
88
LL | let f = foo; // <-- No consistent type can be inferred for `f` here.
99
LL | let a = bar(f, x);
10-
| ^^^^^^^^^ argument requires that `'a` must outlive `'b`
10+
| ^^^^^^^^^ argument requires that `'b` must outlive `'a`
1111
|
12-
= help: consider adding the following bound: `'a: 'b`
12+
= help: consider adding the following bound: `'b: 'a`
1313
= note: requirement occurs because of the type `Type<'_>`, which makes the generic argument `'_` invariant
1414
= note: the struct `Type<'a>` is invariant over the parameter `'a`
1515
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
@@ -23,9 +23,9 @@ LL | fn baz<'a, 'b>(x: Type<'a>, y: Type<'b>) -> (Type<'a>, Type<'b>) {
2323
| lifetime `'a` defined here
2424
...
2525
LL | let b = bar(f, y);
26-
| ^^^^^^^^^ argument requires that `'b` must outlive `'a`
26+
| ^^^^^^^^^ argument requires that `'a` must outlive `'b`
2727
|
28-
= help: consider adding the following bound: `'b: 'a`
28+
= help: consider adding the following bound: `'a: 'b`
2929
= note: requirement occurs because of the type `Type<'_>`, which makes the generic argument `'_` invariant
3030
= note: the struct `Type<'a>` is invariant over the parameter `'a`
3131
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

Diff for: ‎tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.stderr

+24-24
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ LL | let c = async || { println!("{}", *x); };
77
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ borrowed value does not live long enough
88
LL | outlives::<'a>(c());
99
LL | outlives::<'a>(call_once(c));
10-
| ------------ argument requires that `x` is borrowed for `'a`
10+
| ---------------------------- argument requires that `x` is borrowed for `'a`
1111
...
1212
LL | }
1313
| - `x` dropped here while still borrowed
@@ -21,10 +21,10 @@ LL | fn simple<'a>(x: &'a i32) {
2121
LL | let c = async move || { println!("{}", *x); };
2222
| - binding `c` declared here
2323
LL | outlives::<'a>(c());
24-
| ^--
25-
| |
26-
| borrowed value does not live long enough
27-
| argument requires that `c` is borrowed for `'a`
24+
| ---------------^---
25+
| | |
26+
| | borrowed value does not live long enough
27+
| argument requires that `c` is borrowed for `'a`
2828
LL | outlives::<'a>(call_once(c));
2929
LL | }
3030
| - `c` dropped here while still borrowed
@@ -38,7 +38,7 @@ LL | let c = async || { println!("{}", *x.0); };
3838
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ borrowed value does not live long enough
3939
LL | outlives::<'a>(c());
4040
LL | outlives::<'a>(call_once(c));
41-
| ------------ argument requires that `x` is borrowed for `'a`
41+
| ---------------------------- argument requires that `x` is borrowed for `'a`
4242
...
4343
LL | }
4444
| - `x` dropped here while still borrowed
@@ -52,7 +52,7 @@ LL | let c = async || { println!("{}", *x.0); };
5252
| ---------------------------------- borrow of `x` occurs here
5353
LL | outlives::<'a>(c());
5454
LL | outlives::<'a>(call_once(c));
55-
| ------------ argument requires that `x` is borrowed for `'a`
55+
| ---------------------------- argument requires that `x` is borrowed for `'a`
5656
LL |
5757
LL | let c = async move || { println!("{}", *x.0); };
5858
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ move out of `x` occurs here
@@ -66,10 +66,10 @@ LL | fn through_field<'a>(x: S<'a>) {
6666
LL | let c = async move || { println!("{}", *x.0); };
6767
| - binding `c` declared here
6868
LL | outlives::<'a>(c());
69-
| ^--
70-
| |
71-
| borrowed value does not live long enough
72-
| argument requires that `c` is borrowed for `'a`
69+
| ---------------^---
70+
| | |
71+
| | borrowed value does not live long enough
72+
| argument requires that `c` is borrowed for `'a`
7373
LL | outlives::<'a>(call_once(c));
7474
LL | }
7575
| - `c` dropped here while still borrowed
@@ -83,10 +83,10 @@ LL | fn through_field<'a>(x: S<'a>) {
8383
LL | let c = async move || { println!("{}", *x.0); };
8484
| - binding `c` declared here
8585
LL | outlives::<'a>(c());
86-
| ---
87-
| |
88-
| borrow of `c` occurs here
89-
| argument requires that `c` is borrowed for `'a`
86+
| -------------------
87+
| | |
88+
| | borrow of `c` occurs here
89+
| argument requires that `c` is borrowed for `'a`
9090
LL | outlives::<'a>(call_once(c));
9191
| ^ move out of `c` occurs here
9292

@@ -99,18 +99,18 @@ LL | let c = async || { println!("{}", *x.0); };
9999
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ borrowed value does not live long enough
100100
LL | outlives::<'a>(c());
101101
LL | outlives::<'a>(call_once(c));
102-
| ------------ argument requires that `x` is borrowed for `'a`
102+
| ---------------------------- argument requires that `x` is borrowed for `'a`
103103
LL | }
104104
| - `x` dropped here while still borrowed
105105

106106
error[E0621]: explicit lifetime required in the type of `x`
107-
--> $DIR/without-precise-captures-we-are-powerless.rs:40:20
107+
--> $DIR/without-precise-captures-we-are-powerless.rs:40:5
108108
|
109109
LL | fn through_field_and_ref<'a>(x: &S<'a>) {
110110
| ------ help: add explicit lifetime `'a` to the type of `x`: `&'a S<'a>`
111111
...
112112
LL | outlives::<'a>(call_once(c));
113-
| ^^^^^^^^^^^^ lifetime `'a` required
113+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime `'a` required
114114

115115
error[E0597]: `c` does not live long enough
116116
--> $DIR/without-precise-captures-we-are-powerless.rs:45:20
@@ -120,22 +120,22 @@ LL | fn through_field_and_ref_move<'a>(x: &S<'a>) {
120120
LL | let c = async move || { println!("{}", *x.0); };
121121
| - binding `c` declared here
122122
LL | outlives::<'a>(c());
123-
| ^--
124-
| |
125-
| borrowed value does not live long enough
126-
| argument requires that `c` is borrowed for `'a`
123+
| ---------------^---
124+
| | |
125+
| | borrowed value does not live long enough
126+
| argument requires that `c` is borrowed for `'a`
127127
LL | outlives::<'a>(call_once(c));
128128
LL | }
129129
| - `c` dropped here while still borrowed
130130

131131
error[E0621]: explicit lifetime required in the type of `x`
132-
--> $DIR/without-precise-captures-we-are-powerless.rs:46:20
132+
--> $DIR/without-precise-captures-we-are-powerless.rs:46:5
133133
|
134134
LL | fn through_field_and_ref_move<'a>(x: &S<'a>) {
135135
| ------ help: add explicit lifetime `'a` to the type of `x`: `&'a S<'a>`
136136
...
137137
LL | outlives::<'a>(call_once(c));
138-
| ^^^^^^^^^^^^ lifetime `'a` required
138+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime `'a` required
139139

140140
error: aborting due to 10 previous errors
141141

Diff for: ‎tests/ui/async-await/issue-76547.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ impl<'a> Future for ListFut<'a> {
1717
}
1818

1919
async fn fut(bufs: &mut [&mut [u8]]) {
20-
ListFut(bufs).await
2120
//~^ ERROR lifetime may not live long enough
21+
ListFut(bufs).await
2222
}
2323

2424
pub struct ListFut2<'a>(&'a mut [&'a mut [u8]]);
@@ -31,8 +31,8 @@ impl<'a> Future for ListFut2<'a> {
3131
}
3232

3333
async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
34-
ListFut2(bufs).await
3534
//~^ ERROR lifetime may not live long enough
35+
ListFut2(bufs).await
3636
}
3737

3838
fn main() {}

Diff for: ‎tests/ui/async-await/issue-76547.stderr

+10-12
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,25 @@
11
error: lifetime may not live long enough
2-
--> $DIR/issue-76547.rs:20:13
2+
--> $DIR/issue-76547.rs:19:14
33
|
44
LL | async fn fut(bufs: &mut [&mut [u8]]) {
5-
| - - let's call the lifetime of this reference `'2`
6-
| |
7-
| let's call the lifetime of this reference `'1`
8-
LL | ListFut(bufs).await
9-
| ^^^^ this usage requires that `'1` must outlive `'2`
5+
| ^^^^ - - let's call the lifetime of this reference `'2`
6+
| | |
7+
| | let's call the lifetime of this reference `'1`
8+
| assignment requires that `'1` must outlive `'2`
109
|
1110
help: consider introducing a named lifetime parameter
1211
|
1312
LL | async fn fut<'a>(bufs: &'a mut [&'a mut [u8]]) {
1413
| ++++ ++ ++
1514

1615
error: lifetime may not live long enough
17-
--> $DIR/issue-76547.rs:34:14
16+
--> $DIR/issue-76547.rs:33:15
1817
|
1918
LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
20-
| - - let's call the lifetime of this reference `'2`
21-
| |
22-
| let's call the lifetime of this reference `'1`
23-
LL | ListFut2(bufs).await
24-
| ^^^^ this usage requires that `'1` must outlive `'2`
19+
| ^^^^ - - let's call the lifetime of this reference `'2`
20+
| | |
21+
| | let's call the lifetime of this reference `'1`
22+
| assignment requires that `'1` must outlive `'2`
2523
|
2624
help: consider introducing a named lifetime parameter
2725
|

Diff for: ‎tests/ui/borrowck/fn-item-check-type-params.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ LL | extend_lt(val);
1212
| argument requires that `'a` must outlive `'static`
1313

1414
error: lifetime may not live long enough
15-
--> $DIR/fn-item-check-type-params.rs:39:12
15+
--> $DIR/fn-item-check-type-params.rs:39:31
1616
|
1717
LL | pub fn test_coercion<'a>() {
1818
| -- lifetime `'a` defined here
1919
LL | let _: fn(&'a str) -> _ = extend_lt;
20-
| ^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
20+
| ^^^^^^^^^ coercion requires that `'a` must outlive `'static`
2121

2222
error[E0716]: temporary value dropped while borrowed
2323
--> $DIR/fn-item-check-type-params.rs:48:11

Diff for: ‎tests/ui/borrowck/issue-7573.rs

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ pub fn remove_package_from_database() {
1717
lines_to_use.push(installed_id);
1818
//~^ ERROR borrowed data escapes outside of closure
1919
//~| NOTE `installed_id` escapes the closure body here
20+
//~| NOTE requirement occurs because of a mutable reference to `Vec<&CrateId>`
21+
//~| NOTE mutable references are invariant over their type parameter
2022
};
2123
list_database(push_id);
2224

Diff for: ‎tests/ui/borrowck/issue-7573.stderr

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ LL | let push_id = |installed_id: &CrateId| {
99
LL |
1010
LL | lines_to_use.push(installed_id);
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `installed_id` escapes the closure body here
12+
|
13+
= note: requirement occurs because of a mutable reference to `Vec<&CrateId>`
14+
= note: mutable references are invariant over their type parameter
15+
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
1216

1317
error: aborting due to 1 previous error
1418

Diff for: ‎tests/ui/borrowck/two-phase-surprise-no-conflict.stderr

+10-12
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,13 @@ error[E0502]: cannot borrow `*reg` as mutable because it is also borrowed as imm
7171
LL | fn register_plugins<'a>(mk_reg: impl Fn() -> &'a mut Registry<'a>) {
7272
| -- lifetime `'a` defined here
7373
...
74+
LL | let reg = mk_reg();
75+
| -------- assignment requires that `reg.sess_mut` is borrowed for `'a`
7476
LL | reg.register_univ(Box::new(CapturePass::new(&reg.sess_mut)));
75-
| ^^^^^^^^^^^^^^^^^^-----------------------------------------^
76-
| | | |
77-
| | | immutable borrow occurs here
78-
| | coercion requires that `reg.sess_mut` is borrowed for `'a`
77+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-------------^^^
78+
| | |
79+
| | immutable borrow occurs here
7980
| mutable borrow occurs here
80-
|
81-
= note: due to object lifetime defaults, `Box<dyn for<'b> LateLintPass<'b>>` actually means `Box<(dyn for<'b> LateLintPass<'b> + 'static)>`
8281

8382
error[E0502]: cannot borrow `*reg` as mutable because it is also borrowed as immutable
8483
--> $DIR/two-phase-surprise-no-conflict.rs:144:5
@@ -115,14 +114,13 @@ error[E0499]: cannot borrow `*reg` as mutable more than once at a time
115114
LL | fn register_plugins<'a>(mk_reg: impl Fn() -> &'a mut Registry<'a>) {
116115
| -- lifetime `'a` defined here
117116
...
117+
LL | let reg = mk_reg();
118+
| -------- assignment requires that `reg.sess_mut` is borrowed for `'a`
118119
LL | reg.register_univ(Box::new(CapturePass::new_mut(&mut reg.sess_mut)));
119-
| ^^^^^^^^^^^^^^^^^^-------------------------------------------------^
120-
| | | |
121-
| | | first mutable borrow occurs here
122-
| | coercion requires that `reg.sess_mut` is borrowed for `'a`
120+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------------^^^
121+
| | |
122+
| | first mutable borrow occurs here
123123
| second mutable borrow occurs here
124-
|
125-
= note: due to object lifetime defaults, `Box<dyn for<'b> LateLintPass<'b>>` actually means `Box<(dyn for<'b> LateLintPass<'b> + 'static)>`
126124

127125
error[E0499]: cannot borrow `reg.sess_mut` as mutable more than once at a time
128126
--> $DIR/two-phase-surprise-no-conflict.rs:158:53

0 commit comments

Comments
 (0)