Skip to content

Commit 4aba2c5

Browse files
estebankestebank
committed
Modify find_expr from Span to better account for closures
Start pointing to where bindings were declared when they are captured in closures: ``` error[E0597]: `x` does not live long enough --> $DIR/suggest-return-closure.rs:23:9 | LL | let x = String::new(); | - binding `x` declared here ... LL | |c| { | --- value captured here LL | x.push(c); | ^ borrowed value does not live long enough ... LL | } | -- borrow later used here | | | `x` dropped here while still borrowed ``` Suggest cloning in more cases involving closures: ``` error[E0507]: cannot move out of `foo` in pattern guard --> $DIR/issue-27282-move-ref-mut-into-guard.rs:11:19 | LL | if { (|| { let mut bar = foo; bar.take() })(); false } => {}, | ^^ --- move occurs because `foo` has type `&mut Option<&i32>`, which does not implement the `Copy` trait | | | `foo` is moved here | = note: variables bound in patterns cannot be moved from until after the end of the pattern guard help: consider cloning the value if the performance cost is acceptable | LL | if { (|| { let mut bar = foo.clone(); bar.take() })(); false } => {}, | ++++++++ ```
1 parent ef8b9dc commit 4aba2c5

31 files changed

+202
-31
lines changed

‎compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,7 +1964,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
19641964
pub(crate) fn find_expr(&self, span: Span) -> Option<&hir::Expr<'_>> {
19651965
let tcx = self.infcx.tcx;
19661966
let body_id = tcx.hir_node(self.mir_hir_id()).body_id()?;
1967-
let mut expr_finder = FindExprBySpan::new(span);
1967+
let mut expr_finder = FindExprBySpan::new(span, tcx);
19681968
expr_finder.visit_expr(tcx.hir().body(body_id).value);
19691969
expr_finder.result
19701970
}
@@ -1998,14 +1998,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
19981998
};
19991999

20002000
let mut expr_finder =
2001-
FindExprBySpan::new(self.body.local_decls[*index1].source_info.span);
2001+
FindExprBySpan::new(self.body.local_decls[*index1].source_info.span, tcx);
20022002
expr_finder.visit_expr(hir.body(body_id).value);
20032003
let Some(index1) = expr_finder.result else {
20042004
note_default_suggestion();
20052005
return;
20062006
};
20072007

2008-
expr_finder = FindExprBySpan::new(self.body.local_decls[*index2].source_info.span);
2008+
expr_finder = FindExprBySpan::new(self.body.local_decls[*index2].source_info.span, tcx);
20092009
expr_finder.visit_expr(hir.body(body_id).value);
20102010
let Some(index2) = expr_finder.result else {
20112011
note_default_suggestion();

‎compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
7676
&& let Some(body_id) = node.body_id()
7777
{
7878
let body = tcx.hir().body(body_id);
79-
let mut expr_finder = FindExprBySpan::new(span);
79+
let mut expr_finder = FindExprBySpan::new(span, tcx);
8080
expr_finder.visit_expr(body.value);
8181
if let Some(mut expr) = expr_finder.result {
8282
while let hir::ExprKind::AddrOf(_, _, inner)

‎compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,22 @@ pub struct FindExprBySpan<'hir> {
5050
pub span: Span,
5151
pub result: Option<&'hir hir::Expr<'hir>>,
5252
pub ty_result: Option<&'hir hir::Ty<'hir>>,
53+
pub tcx: TyCtxt<'hir>,
5354
}
5455

5556
impl<'hir> FindExprBySpan<'hir> {
56-
pub fn new(span: Span) -> Self {
57-
Self { span, result: None, ty_result: None }
57+
pub fn new(span: Span, tcx: TyCtxt<'hir>) -> Self {
58+
Self { span, result: None, ty_result: None, tcx }
5859
}
5960
}
6061

6162
impl<'v> Visitor<'v> for FindExprBySpan<'v> {
63+
type NestedFilter = rustc_middle::hir::nested_filter::OnlyBodies;
64+
65+
fn nested_visit_map(&mut self) -> Self::Map {
66+
self.tcx.hir()
67+
}
68+
6269
fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
6370
if self.span == ex.span {
6471
self.result = Some(ex);

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
901901
// Remove all the desugaring and macro contexts.
902902
span.remove_mark();
903903
}
904-
let mut expr_finder = FindExprBySpan::new(span);
904+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
905905
let Some(body_id) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
906906
return;
907907
};
@@ -1367,7 +1367,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
13671367
return false;
13681368
};
13691369
let body = self.tcx.hir().body(body_id);
1370-
let mut expr_finder = FindExprBySpan::new(span);
1370+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
13711371
expr_finder.visit_expr(body.value);
13721372
let Some(expr) = expr_finder.result else {
13731373
return false;
@@ -1469,7 +1469,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
14691469
// Remove all the hir desugaring contexts while maintaining the macro contexts.
14701470
span.remove_mark();
14711471
}
1472-
let mut expr_finder = super::FindExprBySpan::new(span);
1472+
let mut expr_finder = super::FindExprBySpan::new(span, self.tcx);
14731473
let Some(body_id) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
14741474
return false;
14751475
};

‎compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2457,7 +2457,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
24572457
&& let Some(body_id) =
24582458
self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id)
24592459
{
2460-
let mut expr_finder = FindExprBySpan::new(span);
2460+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
24612461
expr_finder.visit_expr(self.tcx.hir().body(body_id).value);
24622462

24632463
if let Some(hir::Expr {

‎tests/ui/coroutine/borrowing.stderr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ error[E0597]: `a` does not live long enough
44
LL | let _b = {
55
| -- borrow later stored here
66
LL | let a = 3;
7+
| - binding `a` declared here
78
LL | Pin::new(&mut #[coroutine] || yield &a).resume(())
89
| -- ^ borrowed value does not live long enough
910
| |
@@ -18,6 +19,7 @@ error[E0597]: `a` does not live long enough
1819
LL | let _b = {
1920
| -- borrow later stored here
2021
LL | let a = 3;
22+
| - binding `a` declared here
2123
LL | #[coroutine] || {
2224
| -- value captured here by coroutine
2325
LL | yield &a

‎tests/ui/coroutine/dropck.stderr

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ LL | }
1818
error[E0597]: `ref_` does not live long enough
1919
--> $DIR/dropck.rs:16:18
2020
|
21+
LL | let ref_ = Box::leak(Box::new(Some(cell.borrow_mut())));
22+
| ---- binding `ref_` declared here
23+
...
2124
LL | || {
2225
| -- value captured here by coroutine
2326
LL | // but the coroutine can use it to drop a `Ref<'a, i32>`.

‎tests/ui/fn/suggest-return-closure.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ fn fn_mut() -> _ {
1818
//~| NOTE for more information on `Fn` traits and closure types
1919
let x = String::new();
2020
//~^ HELP: consider changing this to be mutable
21+
//~| NOTE binding `x` declared here
2122
|c| { //~ NOTE: value captured here
2223
x.push(c);
2324
//~^ ERROR: does not live long enough

‎tests/ui/fn/suggest-return-closure.stderr

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ LL | fn fn_mut() -> _ {
2121
= note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html
2222

2323
error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types
24-
--> $DIR/suggest-return-closure.rs:31:13
24+
--> $DIR/suggest-return-closure.rs:32:13
2525
|
2626
LL | fn fun() -> _ {
2727
| ^
@@ -32,7 +32,7 @@ LL | fn fun() -> _ {
3232
= note: for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html
3333

3434
error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
35-
--> $DIR/suggest-return-closure.rs:22:9
35+
--> $DIR/suggest-return-closure.rs:23:9
3636
|
3737
LL | let x = String::new();
3838
| - help: consider changing this to be mutable: `mut x`
@@ -41,8 +41,11 @@ LL | x.push(c);
4141
| ^ cannot borrow as mutable
4242

4343
error[E0597]: `x` does not live long enough
44-
--> $DIR/suggest-return-closure.rs:22:9
44+
--> $DIR/suggest-return-closure.rs:23:9
4545
|
46+
LL | let x = String::new();
47+
| - binding `x` declared here
48+
...
4649
LL | |c| {
4750
| --- value captured here
4851
LL | x.push(c);

‎tests/ui/nll/closure-borrow-spans.stderr

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ LL | f.use_ref();
2525
error[E0597]: `x` does not live long enough
2626
--> $DIR/closure-borrow-spans.rs:19:16
2727
|
28+
LL | let x = 1;
29+
| - binding `x` declared here
2830
LL | f = || x;
2931
| -- ^ borrowed value does not live long enough
3032
| |
@@ -85,6 +87,8 @@ LL | f.use_ref();
8587
error[E0597]: `x` does not live long enough
8688
--> $DIR/closure-borrow-spans.rs:52:16
8789
|
90+
LL | let mut x = 1;
91+
| ----- binding `x` declared here
8892
LL | f = || x = 0;
8993
| -- ^ borrowed value does not live long enough
9094
| |
@@ -145,6 +149,8 @@ LL | f.use_ref();
145149
error[E0597]: `x` does not live long enough
146150
--> $DIR/closure-borrow-spans.rs:86:16
147151
|
152+
LL | let x = &mut z;
153+
| - binding `x` declared here
148154
LL | f = || *x = 0;
149155
| -- ^^ borrowed value does not live long enough
150156
| |

‎tests/ui/nll/closure-requirements/escape-upvar-nested.stderr

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ LL | fn test() {
3737
error[E0597]: `y` does not live long enough
3838
--> $DIR/escape-upvar-nested.rs:21:40
3939
|
40+
LL | let y = 22;
41+
| - binding `y` declared here
42+
LL |
4043
LL | let mut closure = || {
4144
| -- value captured here
4245
LL | let mut closure1 = || p = &y;

‎tests/ui/nll/closure-requirements/escape-upvar-ref.stderr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ LL | fn test() {
2323
error[E0597]: `y` does not live long enough
2424
--> $DIR/escape-upvar-ref.rs:23:35
2525
|
26+
LL | let y = 22;
27+
| - binding `y` declared here
2628
LL | let mut closure = || p = &y;
2729
| -- ^ borrowed value does not live long enough
2830
| |

‎tests/ui/nll/closure-requirements/propagate-multiple-requirements.stderr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
error[E0597]: `local_arr` does not live long enough
22
--> $DIR/propagate-multiple-requirements.rs:15:14
33
|
4+
LL | let local_arr = other_local_arr;
5+
| --------- binding `local_arr` declared here
46
LL | let mut out: &mut &'static [i32] = &mut (&[1] as _);
57
| ------------------- type annotation requires that `local_arr` is borrowed for `'static`
68
LL | once(|mut z: &[i32], mut out_val: &mut &[i32]| {

‎tests/ui/nll/issue-27282-move-ref-mut-into-guard.fixed

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Issue 27282: Example 1: This sidesteps the AST checks disallowing
2+
// mutable borrows in match guards by hiding the mutable borrow in a
3+
// guard behind a move (of the ref mut pattern id) within a closure.
4+
//@ run-rustfix
5+
#![feature(if_let_guard)]
6+
7+
fn main() {
8+
match Some(&4) {
9+
None => {},
10+
ref mut foo
11+
if { (|| { let mut bar = foo.clone(); bar.take() })(); false } => {},
12+
//~^ ERROR cannot move out of `foo` in pattern guard [E0507]
13+
Some(s) => std::process::exit(*s),
14+
}
15+
16+
match Some(&4) {
17+
None => {},
18+
ref mut foo
19+
if let Some(()) = { (|| { let mut bar = foo.clone(); bar.take() })(); None } => {},
20+
//~^ ERROR cannot move out of `foo` in pattern guard [E0507]
21+
Some(s) => std::process::exit(*s),
22+
}
23+
}

‎tests/ui/nll/issue-27282-move-ref-mut-into-guard.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
// Issue 27282: Example 1: This sidesteps the AST checks disallowing
22
// mutable borrows in match guards by hiding the mutable borrow in a
33
// guard behind a move (of the ref mut pattern id) within a closure.
4-
4+
//@ run-rustfix
55
#![feature(if_let_guard)]
66

77
fn main() {
88
match Some(&4) {
99
None => {},
1010
ref mut foo
11-
if { (|| { let bar = foo; bar.take() })(); false } => {},
11+
if { (|| { let mut bar = foo; bar.take() })(); false } => {},
1212
//~^ ERROR cannot move out of `foo` in pattern guard [E0507]
1313
Some(s) => std::process::exit(*s),
1414
}
1515

1616
match Some(&4) {
1717
None => {},
1818
ref mut foo
19-
if let Some(()) = { (|| { let bar = foo; bar.take() })(); None } => {},
19+
if let Some(()) = { (|| { let mut bar = foo; bar.take() })(); None } => {},
2020
//~^ ERROR cannot move out of `foo` in pattern guard [E0507]
2121
Some(s) => std::process::exit(*s),
2222
}

‎tests/ui/nll/issue-27282-move-ref-mut-into-guard.stderr

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,30 @@
11
error[E0507]: cannot move out of `foo` in pattern guard
22
--> $DIR/issue-27282-move-ref-mut-into-guard.rs:11:19
33
|
4-
LL | if { (|| { let bar = foo; bar.take() })(); false } => {},
5-
| ^^ --- move occurs because `foo` has type `&mut Option<&i32>`, which does not implement the `Copy` trait
4+
LL | if { (|| { let mut bar = foo; bar.take() })(); false } => {},
5+
| ^^ --- move occurs because `foo` has type `&mut Option<&i32>`, which does not implement the `Copy` trait
66
| |
77
| `foo` is moved here
88
|
99
= note: variables bound in patterns cannot be moved from until after the end of the pattern guard
10+
help: consider cloning the value if the performance cost is acceptable
11+
|
12+
LL | if { (|| { let mut bar = foo.clone(); bar.take() })(); false } => {},
13+
| ++++++++
1014

1115
error[E0507]: cannot move out of `foo` in pattern guard
1216
--> $DIR/issue-27282-move-ref-mut-into-guard.rs:19:34
1317
|
14-
LL | if let Some(()) = { (|| { let bar = foo; bar.take() })(); None } => {},
15-
| ^^ --- move occurs because `foo` has type `&mut Option<&i32>`, which does not implement the `Copy` trait
18+
LL | if let Some(()) = { (|| { let mut bar = foo; bar.take() })(); None } => {},
19+
| ^^ --- move occurs because `foo` has type `&mut Option<&i32>`, which does not implement the `Copy` trait
1620
| |
1721
| `foo` is moved here
1822
|
1923
= note: variables bound in patterns cannot be moved from until after the end of the pattern guard
24+
help: consider cloning the value if the performance cost is acceptable
25+
|
26+
LL | if let Some(()) = { (|| { let mut bar = foo.clone(); bar.take() })(); None } => {},
27+
| ++++++++
2028

2129
error: aborting due to 2 previous errors
2230

‎tests/ui/nll/issue-27282-mutation-in-guard.stderr

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ LL | (|| { let bar = foo; bar.take() })();
77
| `foo` is moved here
88
|
99
= note: variables bound in patterns cannot be moved from until after the end of the pattern guard
10+
help: consider cloning the value if the performance cost is acceptable
11+
|
12+
LL | (|| { let bar = foo.clone(); bar.take() })();
13+
| ++++++++
1014

1115
error[E0507]: cannot move out of `foo` in pattern guard
1216
--> $DIR/issue-27282-mutation-in-guard.rs:20:18
@@ -17,6 +21,10 @@ LL | (|| { let bar = foo; bar.take() })();
1721
| `foo` is moved here
1822
|
1923
= note: variables bound in patterns cannot be moved from until after the end of the pattern guard
24+
help: consider cloning the value if the performance cost is acceptable
25+
|
26+
LL | (|| { let bar = foo.clone(); bar.take() })();
27+
| ++++++++
2028

2129
error: aborting due to 2 previous errors
2230

‎tests/ui/nll/issue-42574-diagnostic-in-nested-closure.stderr

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ LL | || doit(data);
1111
error[E0597]: `data` does not live long enough
1212
--> $DIR/issue-42574-diagnostic-in-nested-closure.rs:6:13
1313
|
14+
LL | fn doit(data: &'static mut ()) {
15+
| ---- binding `data` declared here
1416
LL | || doit(data);
1517
| -- -----^^^^-
1618
| | | |

‎tests/ui/nll/match-guards-always-borrow.fixed

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
#![feature(if_let_guard)]
2+
#![allow(unused_mut)]
3+
//@ run-rustfix
4+
5+
// Here is arielb1's basic example from rust-lang/rust#27282
6+
// that AST borrowck is flummoxed by:
7+
8+
fn should_reject_destructive_mutate_in_guard() {
9+
match Some(&4) {
10+
None => {},
11+
ref mut foo if {
12+
(|| { let mut bar = foo.clone(); bar.take() })();
13+
//~^ ERROR cannot move out of `foo` in pattern guard [E0507]
14+
false } => { },
15+
Some(s) => std::process::exit(*s),
16+
}
17+
18+
match Some(&4) {
19+
None => {},
20+
ref mut foo if let Some(()) = {
21+
(|| { let mut bar = foo.clone(); bar.take() })();
22+
//~^ ERROR cannot move out of `foo` in pattern guard [E0507]
23+
None } => { },
24+
Some(s) => std::process::exit(*s),
25+
}
26+
}
27+
28+
// Here below is a case that needs to keep working: we only use the
29+
// binding via immutable-borrow in the guard, and we mutate in the arm
30+
// body.
31+
fn allow_mutate_in_arm_body() {
32+
match Some(&4) {
33+
None => {},
34+
ref mut foo if foo.is_some() => { foo.take(); () }
35+
Some(s) => std::process::exit(*s),
36+
}
37+
38+
match Some(&4) {
39+
None => {},
40+
ref mut foo if let Some(_) = foo => { foo.take(); () }
41+
Some(s) => std::process::exit(*s),
42+
}
43+
}
44+
45+
// Here below is a case that needs to keep working: we only use the
46+
// binding via immutable-borrow in the guard, and we move into the arm
47+
// body.
48+
fn allow_move_into_arm_body() {
49+
match Some(&4) {
50+
None => {},
51+
mut foo if foo.is_some() => { foo.unwrap(); () }
52+
Some(s) => std::process::exit(*s),
53+
}
54+
55+
match Some(&4) {
56+
None => {},
57+
mut foo if let Some(_) = foo => { foo.unwrap(); () }
58+
Some(s) => std::process::exit(*s),
59+
}
60+
}
61+
62+
fn main() {
63+
should_reject_destructive_mutate_in_guard();
64+
allow_mutate_in_arm_body();
65+
allow_move_into_arm_body();
66+
}

0 commit comments

Comments
 (0)