Skip to content

Commit 00f821f

Browse files
estebankestebank
committed
Suggest move in nested closure when appropriate
Fix #64008.
1 parent b22c152 commit 00f821f

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
@@ -474,10 +474,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
474474
let err = FnMutError {
475475
span: *span,
476476
ty_err: match output_ty.kind() {
477-
ty::Closure(_, _) => FnMutReturnTypeErr::ReturnClosure { span: *span },
478477
ty::Generator(def, ..) if self.infcx.tcx.generator_is_async(*def) => {
479478
FnMutReturnTypeErr::ReturnAsyncBlock { span: *span }
480479
}
480+
_ if output_ty.contains_closure() => {
481+
FnMutReturnTypeErr::ReturnClosure { span: *span }
482+
}
481483
_ => FnMutReturnTypeErr::ReturnRef { span: *span },
482484
},
483485
};
@@ -867,7 +869,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
867869
fn suggest_move_on_borrowing_closure(&self, diag: &mut Diagnostic) {
868870
let map = self.infcx.tcx.hir();
869871
let body_id = map.body_owned_by(self.mir_def_id());
870-
let expr = &map.body(body_id).value;
872+
let expr = &map.body(body_id).value.peel_blocks();
871873
let mut closure_span = None::<rustc_span::Span>;
872874
match expr.kind {
873875
hir::ExprKind::MethodCall(.., args, _) => {
@@ -882,20 +884,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
882884
}
883885
}
884886
}
885-
hir::ExprKind::Block(blk, _) => {
886-
if let Some(expr) = blk.expr {
887-
// only when the block is a closure
888-
if let hir::ExprKind::Closure(hir::Closure {
889-
capture_clause: hir::CaptureBy::Ref,
890-
body,
891-
..
892-
}) = expr.kind
893-
{
894-
let body = map.body(*body);
895-
if !matches!(body.generator_kind, Some(hir::GeneratorKind::Async(..))) {
896-
closure_span = Some(expr.span.shrink_to_lo());
897-
}
898-
}
887+
hir::ExprKind::Closure(hir::Closure {
888+
capture_clause: hir::CaptureBy::Ref,
889+
body,
890+
..
891+
}) => {
892+
let body = map.body(*body);
893+
if !matches!(body.generator_kind, Some(hir::GeneratorKind::Async(..))) {
894+
closure_span = Some(expr.span.shrink_to_lo());
899895
}
900896
}
901897
_ => {}

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

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

1940+
/// Checks whether a type recursively contains any closure
1941+
///
1942+
/// Example: `Option<[[email protected]:4:20]>` returns true
1943+
pub fn contains_closure(self) -> bool {
1944+
struct ContainsClosureVisitor;
1945+
1946+
impl<'tcx> TypeVisitor<'tcx> for ContainsClosureVisitor {
1947+
type BreakTy = ();
1948+
1949+
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
1950+
if let ty::Closure(_, _) = t.kind() {
1951+
ControlFlow::BREAK
1952+
} else {
1953+
t.super_visit_with(self)
1954+
}
1955+
}
1956+
}
1957+
1958+
let cf = self.visit_with(&mut ContainsClosureVisitor);
1959+
cf.is_break()
1960+
}
1961+
19401962
/// Returns the type and mutability of `*ty`.
19411963
///
19421964
/// 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)