]> git.lizzy.rs Git - rust.git/commitdiff
Suggest `move` in nested closure when appropriate
authorEsteban Küber <esteban@kuber.com.ar>
Sat, 7 Jan 2023 22:17:00 +0000 (22:17 +0000)
committerEsteban Küber <esteban@kuber.com.ar>
Thu, 2 Feb 2023 16:26:01 +0000 (16:26 +0000)
Fix #64008.

compiler/rustc_borrowck/src/diagnostics/region_errors.rs
compiler/rustc_middle/src/ty/sty.rs
tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.fixed [new file with mode: 0644]
tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.rs
tests/ui/borrowck/issue-95079-missing-move-in-nested-closure.stderr

index 87db08ef5b5108e3b6439883ce5d8bb869f90967..7901a5046abad12a466cf045b9591d4fe618f5f1 100644 (file)
@@ -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::<rustc_span::Span>;
         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());
                 }
             }
             _ => {}
index 060d864389cb0f4ffa3f3c84b9f9ab5bcb353705..98d6b68356368c14e06717b05a5c4ccc72e05915 100644 (file)
@@ -2043,6 +2043,28 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
         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<Self::BreakTy> {
+                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 (file)
index 0000000..1a08470
--- /dev/null
@@ -0,0 +1,26 @@
+// run-rustfix
+#![allow(dead_code, path_statements)]
+fn foo1(s: &str) -> impl Iterator<Item = String> + '_ {
+    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<Item = ()> + '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() {}
index 95847d8d301a65249988a6bf843bdd7fd194858d..b93292e3589d371a67bad831dc716efc96ff59d0 100644 (file)
@@ -1,3 +1,5 @@
+// run-rustfix
+#![allow(dead_code, path_statements)]
 fn foo1(s: &str) -> impl Iterator<Item = String> + '_ {
     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<Item = ()> + '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() {}
index 2eae614a2f5987504231e3960ed06c097fab10b2..776c338deacf40f4c6d2db778e8e710f44557d40 100644 (file)
@@ -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<Item = String> + '_ {
    |         - 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<Chars<'_>, [closure@$DIR/issue-95079-missing-move-in-nested-closure.rs:9:29: 9:32]>` contains a lifetime `'2`
+   |     |       return type of closure `Map<Chars<'_>, [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<Item = ()> + '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