From e6b84eb7973fb751744a1802cee404254d67673e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 7 Jan 2023 22:17:00 +0000 Subject: [PATCH] Suggest `move` in nested closure when appropriate Fix #64008. --- .../src/diagnostics/region_errors.rs | 28 +++++++---------- compiler/rustc_middle/src/ty/sty.rs | 22 +++++++++++++ ...95079-missing-move-in-nested-closure.fixed | 26 ++++++++++++++++ ...ue-95079-missing-move-in-nested-closure.rs | 12 +++++++ ...5079-missing-move-in-nested-closure.stderr | 31 ++++++++++++++++--- 5 files changed, 98 insertions(+), 21 deletions(-) create mode 100644 tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.fixed diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index 87db08ef5b5..7901a5046ab 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -583,10 +583,12 @@ fn report_fnmut_error( let err = FnMutError { span: *span, ty_err: match output_ty.kind() { - ty::Closure(_, _) => FnMutReturnTypeErr::ReturnClosure { span: *span }, ty::Generator(def, ..) if self.infcx.tcx.generator_is_async(*def) => { FnMutReturnTypeErr::ReturnAsyncBlock { span: *span } } + _ if output_ty.contains_closure() => { + FnMutReturnTypeErr::ReturnClosure { span: *span } + } _ => FnMutReturnTypeErr::ReturnRef { span: *span }, }, }; @@ -997,7 +999,7 @@ fn suggest_adding_lifetime_params( fn suggest_move_on_borrowing_closure(&self, diag: &mut Diagnostic) { let map = self.infcx.tcx.hir(); let body_id = map.body_owned_by(self.mir_def_id()); - let expr = &map.body(body_id).value; + let expr = &map.body(body_id).value.peel_blocks(); let mut closure_span = None::; match expr.kind { hir::ExprKind::MethodCall(.., args, _) => { @@ -1012,20 +1014,14 @@ fn suggest_move_on_borrowing_closure(&self, diag: &mut Diagnostic) { } } } - hir::ExprKind::Block(blk, _) => { - if let Some(expr) = blk.expr { - // only when the block is a closure - if let hir::ExprKind::Closure(hir::Closure { - capture_clause: hir::CaptureBy::Ref, - body, - .. - }) = expr.kind - { - let body = map.body(*body); - if !matches!(body.generator_kind, Some(hir::GeneratorKind::Async(..))) { - closure_span = Some(expr.span.shrink_to_lo()); - } - } + hir::ExprKind::Closure(hir::Closure { + capture_clause: hir::CaptureBy::Ref, + body, + .. + }) => { + let body = map.body(*body); + if !matches!(body.generator_kind, Some(hir::GeneratorKind::Async(..))) { + closure_span = Some(expr.span.shrink_to_lo()); } } _ => {} diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 060d864389c..98d6b683563 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -2043,6 +2043,28 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { cf.is_break() } + /// Checks whether a type recursively contains any closure + /// + /// Example: `Option<[closure@file.rs:4:20]>` returns true + pub fn contains_closure(self) -> bool { + struct ContainsClosureVisitor; + + impl<'tcx> TypeVisitor<'tcx> for ContainsClosureVisitor { + type BreakTy = (); + + fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { + if let ty::Closure(_, _) = t.kind() { + ControlFlow::Break(()) + } else { + t.super_visit_with(self) + } + } + } + + let cf = self.visit_with(&mut ContainsClosureVisitor); + cf.is_break() + } + /// Returns the type and mutability of `*ty`. /// /// The parameter `explicit` indicates if this is an *explicit* dereference. diff --git a/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.fixed b/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.fixed new file mode 100644 index 00000000000..1a08470064c --- /dev/null +++ b/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.fixed @@ -0,0 +1,26 @@ +// run-rustfix +#![allow(dead_code, path_statements)] +fn foo1(s: &str) -> impl Iterator + '_ { + None.into_iter() + .flat_map(move |()| s.chars().map(move |c| format!("{}{}", c, s))) + //~^ ERROR captured variable cannot escape `FnMut` closure body + //~| HELP consider adding 'move' keyword before the nested closure +} + +fn foo2(s: &str) -> impl Sized + '_ { + move |()| s.chars().map(move |c| format!("{}{}", c, s)) + //~^ ERROR lifetime may not live long enough + //~| HELP consider adding 'move' keyword before the nested closure +} + +pub struct X; +pub fn foo3<'a>( + bar: &'a X, +) -> impl Iterator + 'a { + Some(()).iter().flat_map(move |()| { + Some(()).iter().map(move |()| { bar; }) //~ ERROR captured variable cannot escape + //~^ HELP consider adding 'move' keyword before the nested closure + }) +} + +fn main() {} diff --git a/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.rs b/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.rs index 95847d8d301..b93292e3589 100644 --- a/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.rs +++ b/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.rs @@ -1,3 +1,5 @@ +// run-rustfix +#![allow(dead_code, path_statements)] fn foo1(s: &str) -> impl Iterator + '_ { None.into_iter() .flat_map(move |()| s.chars().map(|c| format!("{}{}", c, s))) @@ -11,4 +13,14 @@ fn foo2(s: &str) -> impl Sized + '_ { //~| HELP consider adding 'move' keyword before the nested closure } +pub struct X; +pub fn foo3<'a>( + bar: &'a X, +) -> impl Iterator + 'a { + Some(()).iter().flat_map(move |()| { + Some(()).iter().map(|()| { bar; }) //~ ERROR captured variable cannot escape + //~^ HELP consider adding 'move' keyword before the nested closure + }) +} + fn main() {} diff --git a/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.stderr b/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.stderr index 2eae614a2f5..776c338deac 100644 --- a/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.stderr +++ b/tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.stderr @@ -1,5 +1,5 @@ error: captured variable cannot escape `FnMut` closure body - --> $DIR/issue-95079-missing-move-in-nested-closure.rs:3:29 + --> $DIR/issue-95079-missing-move-in-nested-closure.rs:5:29 | LL | fn foo1(s: &str) -> impl Iterator + '_ { | - variable defined here @@ -7,7 +7,7 @@ LL | None.into_iter() LL | .flat_map(move |()| s.chars().map(|c| format!("{}{}", c, s))) | - -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | - | | returns a reference to a captured variable which escapes the closure body + | | returns a closure that contains a reference to a captured variable, which then escapes the closure body | | variable captured here | inferred to be a `FnMut` closure | @@ -19,12 +19,12 @@ LL | .flat_map(move |()| s.chars().map(move |c| format!("{}{}", c, s))) | ++++ error: lifetime may not live long enough - --> $DIR/issue-95079-missing-move-in-nested-closure.rs:9:15 + --> $DIR/issue-95079-missing-move-in-nested-closure.rs:11:15 | LL | move |()| s.chars().map(|c| format!("{}{}", c, s)) | --------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` | | | - | | return type of closure `Map, [closure@$DIR/issue-95079-missing-move-in-nested-closure.rs:9:29: 9:32]>` contains a lifetime `'2` + | | return type of closure `Map, [closure@$DIR/issue-95079-missing-move-in-nested-closure.rs:11:29: 11:32]>` contains a lifetime `'2` | lifetime `'1` represents this closure's body | = 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 LL | move |()| s.chars().map(move |c| format!("{}{}", c, s)) | ++++ -error: aborting due to 2 previous errors +error: captured variable cannot escape `FnMut` closure body + --> $DIR/issue-95079-missing-move-in-nested-closure.rs:21:9 + | +LL | bar: &'a X, + | --- variable defined here +LL | ) -> impl Iterator + 'a { +LL | Some(()).iter().flat_map(move |()| { + | - inferred to be a `FnMut` closure +LL | Some(()).iter().map(|()| { bar; }) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^---^^^^ + | | | + | | variable captured here + | returns a closure that contains a reference to a captured variable, which then escapes the closure body + | + = note: `FnMut` closures only have access to their captured variables while they are executing... + = note: ...therefore, they cannot allow references to captured variables to escape +help: consider adding 'move' keyword before the nested closure + | +LL | Some(()).iter().map(move |()| { bar; }) + | ++++ + +error: aborting due to 3 previous errors