Skip to content

Commit e6b84eb

Browse files
estebankestebank
committed
Suggest move in nested closure when appropriate
Fix #64008.
1 parent 97872b7 commit e6b84eb

File tree

5 files changed

+98
-21
lines changed

5 files changed

+98
-21
lines changed

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

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -583,10 +583,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
583583
let err = FnMutError {
584584
span: *span,
585585
ty_err: match output_ty.kind() {
586-
ty::Closure(_, _) => FnMutReturnTypeErr::ReturnClosure { span: *span },
587586
ty::Generator(def, ..) if self.infcx.tcx.generator_is_async(*def) => {
588587
FnMutReturnTypeErr::ReturnAsyncBlock { span: *span }
589588
}
589+
_ if output_ty.contains_closure() => {
590+
FnMutReturnTypeErr::ReturnClosure { span: *span }
591+
}
590592
_ => FnMutReturnTypeErr::ReturnRef { span: *span },
591593
},
592594
};
@@ -997,7 +999,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
997999
fn suggest_move_on_borrowing_closure(&self, diag: &mut Diagnostic) {
9981000
let map = self.infcx.tcx.hir();
9991001
let body_id = map.body_owned_by(self.mir_def_id());
1000-
let expr = &map.body(body_id).value;
1002+
let expr = &map.body(body_id).value.peel_blocks();
10011003
let mut closure_span = None::<rustc_span::Span>;
10021004
match expr.kind {
10031005
hir::ExprKind::MethodCall(.., args, _) => {
@@ -1012,20 +1014,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
10121014
}
10131015
}
10141016
}
1015-
hir::ExprKind::Block(blk, _) => {
1016-
if let Some(expr) = blk.expr {
1017-
// only when the block is a closure
1018-
if let hir::ExprKind::Closure(hir::Closure {
1019-
capture_clause: hir::CaptureBy::Ref,
1020-
body,
1021-
..
1022-
}) = expr.kind
1023-
{
1024-
let body = map.body(*body);
1025-
if !matches!(body.generator_kind, Some(hir::GeneratorKind::Async(..))) {
1026-
closure_span = Some(expr.span.shrink_to_lo());
1027-
}
1028-
}
1017+
hir::ExprKind::Closure(hir::Closure {
1018+
capture_clause: hir::CaptureBy::Ref,
1019+
body,
1020+
..
1021+
}) => {
1022+
let body = map.body(*body);
1023+
if !matches!(body.generator_kind, Some(hir::GeneratorKind::Async(..))) {
1024+
closure_span = Some(expr.span.shrink_to_lo());
10291025
}
10301026
}
10311027
_ => {}

‎compiler/rustc_middle/src/ty/sty.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,6 +2043,28 @@ impl<'tcx> Ty<'tcx> {
20432043
cf.is_break()
20442044
}
20452045

2046+
/// Checks whether a type recursively contains any closure
2047+
///
2048+
/// Example: `Option<[[email protected]:4:20]>` returns true
2049+
pub fn contains_closure(self) -> bool {
2050+
struct ContainsClosureVisitor;
2051+
2052+
impl<'tcx> TypeVisitor<'tcx> for ContainsClosureVisitor {
2053+
type BreakTy = ();
2054+
2055+
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
2056+
if let ty::Closure(_, _) = t.kind() {
2057+
ControlFlow::Break(())
2058+
} else {
2059+
t.super_visit_with(self)
2060+
}
2061+
}
2062+
}
2063+
2064+
let cf = self.visit_with(&mut ContainsClosureVisitor);
2065+
cf.is_break()
2066+
}
2067+
20462068
/// Returns the type and mutability of `*ty`.
20472069
///
20482070
/// The parameter `explicit` indicates if this is an *explicit* dereference.

‎tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.fixed‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// run-rustfix
2+
#![allow(dead_code, path_statements)]
3+
fn foo1(s: &str) -> impl Iterator<Item = String> + '_ {
4+
None.into_iter()
5+
.flat_map(move |()| s.chars().map(move |c| format!("{}{}", c, s)))
6+
//~^ ERROR captured variable cannot escape `FnMut` closure body
7+
//~| HELP consider adding 'move' keyword before the nested closure
8+
}
9+
10+
fn foo2(s: &str) -> impl Sized + '_ {
11+
move |()| s.chars().map(move |c| format!("{}{}", c, s))
12+
//~^ ERROR lifetime may not live long enough
13+
//~| HELP consider adding 'move' keyword before the nested closure
14+
}
15+
16+
pub struct X;
17+
pub fn foo3<'a>(
18+
bar: &'a X,
19+
) -> impl Iterator<Item = ()> + 'a {
20+
Some(()).iter().flat_map(move |()| {
21+
Some(()).iter().map(move |()| { bar; }) //~ ERROR captured variable cannot escape
22+
//~^ HELP consider adding 'move' keyword before the nested closure
23+
})
24+
}
25+
26+
fn main() {}

‎tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// run-rustfix
2+
#![allow(dead_code, path_statements)]
13
fn foo1(s: &str) -> impl Iterator<Item = String> + '_ {
24
None.into_iter()
35
.flat_map(move |()| s.chars().map(|c| format!("{}{}", c, s)))
@@ -11,4 +13,14 @@ fn foo2(s: &str) -> impl Sized + '_ {
1113
//~| HELP consider adding 'move' keyword before the nested closure
1214
}
1315

16+
pub struct X;
17+
pub fn foo3<'a>(
18+
bar: &'a X,
19+
) -> impl Iterator<Item = ()> + 'a {
20+
Some(()).iter().flat_map(move |()| {
21+
Some(()).iter().map(|()| { bar; }) //~ ERROR captured variable cannot escape
22+
//~^ HELP consider adding 'move' keyword before the nested closure
23+
})
24+
}
25+
1426
fn main() {}

‎tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.stderr‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: captured variable cannot escape `FnMut` closure body
2-
--> $DIR/issue-95079-missing-move-in-nested-closure.rs:3:29
2+
--> $DIR/issue-95079-missing-move-in-nested-closure.rs:5:29
33
|
44
LL | fn foo1(s: &str) -> impl Iterator<Item = String> + '_ {
55
| - variable defined here
66
LL | None.into_iter()
77
LL | .flat_map(move |()| s.chars().map(|c| format!("{}{}", c, s)))
88
| - -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
99
| | |
10-
| | returns a reference to a captured variable which escapes the closure body
10+
| | returns a closure that contains a reference to a captured variable, which then escapes the closure body
1111
| | variable captured here
1212
| inferred to be a `FnMut` closure
1313
|
@@ -19,12 +19,12 @@ LL | .flat_map(move |()| s.chars().map(move |c| format!("{}{}", c, s)))
1919
| ++++
2020

2121
error: lifetime may not live long enough
22-
--> $DIR/issue-95079-missing-move-in-nested-closure.rs:9:15
22+
--> $DIR/issue-95079-missing-move-in-nested-closure.rs:11:15
2323
|
2424
LL | move |()| s.chars().map(|c| format!("{}{}", c, s))
2525
| --------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
2626
| | |
27-
| | return type of closure `Map<Chars<'_>, [closure@$DIR/issue-95079-missing-move-in-nested-closure.rs:9:29: 9:32]>` contains a lifetime `'2`
27+
| | return type of closure `Map<Chars<'_>, [closure@$DIR/issue-95079-missing-move-in-nested-closure.rs:11:29: 11:32]>` contains a lifetime `'2`
2828
| lifetime `'1` represents this closure's body
2929
|
3030
= note: closure implements `Fn`, so references to captured variables can't escape the closure
@@ -33,5 +33,26 @@ help: consider adding 'move' keyword before the nested closure
3333
LL | move |()| s.chars().map(move |c| format!("{}{}", c, s))
3434
| ++++
3535

36-
error: aborting due to 2 previous errors
36+
error: captured variable cannot escape `FnMut` closure body
37+
--> $DIR/issue-95079-missing-move-in-nested-closure.rs:21:9
38+
|
39+
LL | bar: &'a X,
40+
| --- variable defined here
41+
LL | ) -> impl Iterator<Item = ()> + 'a {
42+
LL | Some(()).iter().flat_map(move |()| {
43+
| - inferred to be a `FnMut` closure
44+
LL | Some(()).iter().map(|()| { bar; })
45+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^---^^^^
46+
| | |
47+
| | variable captured here
48+
| returns a closure that contains a reference to a captured variable, which then escapes the closure body
49+
|
50+
= note: `FnMut` closures only have access to their captured variables while they are executing...
51+
= note: ...therefore, they cannot allow references to captured variables to escape
52+
help: consider adding 'move' keyword before the nested closure
53+
|
54+
LL | Some(()).iter().map(move |()| { bar; })
55+
| ++++
56+
57+
error: aborting due to 3 previous errors
3758

0 commit comments

Comments
 (0)