Skip to content

Commit 494f6d7

Browse files
estebankestebank
committed
Modify find_expr from Span to better account for closures
Start pointing to where bindings where 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 c8d9753 commit 494f6d7

31 files changed

+202
-31
lines changed

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -2017,7 +2017,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
20172017
pub(crate) fn find_expr(&self, span: Span) -> Option<&hir::Expr<'_>> {
20182018
let tcx = self.infcx.tcx;
20192019
let body_id = tcx.hir_node(self.mir_hir_id()).body_id()?;
2020-
let mut expr_finder = FindExprBySpan::new(span);
2020+
let mut expr_finder = FindExprBySpan::new(span, tcx);
20212021
expr_finder.visit_expr(tcx.hir().body(body_id).value);
20222022
expr_finder.result
20232023
}
@@ -2051,14 +2051,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
20512051
};
20522052

20532053
let mut expr_finder =
2054-
FindExprBySpan::new(self.body.local_decls[*index1].source_info.span);
2054+
FindExprBySpan::new(self.body.local_decls[*index1].source_info.span, tcx);
20552055
expr_finder.visit_expr(hir.body(body_id).value);
20562056
let Some(index1) = expr_finder.result else {
20572057
note_default_suggestion();
20582058
return;
20592059
};
20602060

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

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

+1-1
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

+9-2
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

+3-3
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
903903
// Remove all the desugaring and macro contexts.
904904
span.remove_mark();
905905
}
906-
let mut expr_finder = FindExprBySpan::new(span);
906+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
907907
let Some(body_id) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
908908
return;
909909
};
@@ -1369,7 +1369,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
13691369
return false;
13701370
};
13711371
let body = self.tcx.hir().body(body_id);
1372-
let mut expr_finder = FindExprBySpan::new(span);
1372+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
13731373
expr_finder.visit_expr(body.value);
13741374
let Some(expr) = expr_finder.result else {
13751375
return false;
@@ -1471,7 +1471,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
14711471
// Remove all the hir desugaring contexts while maintaining the macro contexts.
14721472
span.remove_mark();
14731473
}
1474-
let mut expr_finder = super::FindExprBySpan::new(span);
1474+
let mut expr_finder = super::FindExprBySpan::new(span, self.tcx);
14751475
let Some(body_id) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
14761476
return false;
14771477
};

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -2451,7 +2451,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
24512451
&& let Some(body_id) =
24522452
self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id)
24532453
{
2454-
let mut expr_finder = FindExprBySpan::new(span);
2454+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
24552455
expr_finder.visit_expr(self.tcx.hir().body(body_id).value);
24562456

24572457
if let Some(hir::Expr {

‎tests/ui/coroutine/borrowing.stderr

+2
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 || 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 | || {
2224
| -- value captured here by coroutine
2325
LL | yield &a

‎tests/ui/coroutine/dropck.stderr

+3
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:15:18
2020
|
21+
LL | let ref_ = Box::leak(Box::new(Some(cell.borrow_mut())));
22+
| ---- binding `ref_` declared here
23+
...
2124
LL | gen = || {
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

+1
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

+6-3
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

+6
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

+3
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

+2
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

+2
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

+23
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

+3-3
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

+12-4
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

+8
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

+2
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

+66
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)