Make sure that async closures (and fns) only capture their parent callable's parameters by move, and nothing else

8 files changed, 127 insertions, 36 deletions

diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index 6b772c1295f..a122b4c5113 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/compiler/rustc_ast_lowering/src/item.rs
@@ -1278,10 +1278,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
         };
         let closure_id = coroutine_kind.closure_id();
         let coroutine_expr = self.make_desugared_coroutine_expr(
-            // FIXME(async_closures): This should only move locals,
-            // and not upvars. Capturing closure upvars by ref doesn't
-            // work right now anyways, so whatever.
-            CaptureBy::Value { move_kw: rustc_span::DUMMY_SP },
+            // The default capture mode here is by-ref. Later on during upvar analysis,
+            // we will force the captured arguments to by-move, but for async closures,
+            // we want to make sure that we avoid unnecessarily moving captures, or else
+            // all async closures would default to `FnOnce` as their calling mode.
+            CaptureBy::Ref,
             closure_id,
             return_type_hint,
             body_span,
diff --git a/compiler/rustc_hir_typeck/src/upvar.rs b/compiler/rustc_hir_typeck/src/upvar.rs
index 82c5e566f16..864acf51025 100644
--- a/compiler/rustc_hir_typeck/src/upvar.rs
+++ b/compiler/rustc_hir_typeck/src/upvar.rs
@@ -200,6 +200,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             capture_information: Default::default(),
             fake_reads: Default::default(),
         };
+
+        // As noted in `lower_coroutine_body_with_moved_arguments`, we default the capture mode
+        // to `ByRef` for the `async {}` block internal to async fns/closure. This means
+        // that we would *not* be moving all of the parameters into the async block by default.
+        //
+        // We force all of these arguments to be captured by move before we do expr use analysis.
+        //
+        // FIXME(async_closures): This could be cleaned up. It's a bit janky that we're just
+        // moving all of the `LocalSource::AsyncFn` locals here.
+        if let Some(hir::CoroutineKind::Desugared(
+            _,
+            hir::CoroutineSource::Fn | hir::CoroutineSource::Closure,
+        )) = self.tcx.coroutine_kind(closure_def_id)
+        {
+            let hir::ExprKind::Block(block, _) = body.value.kind else {
+                bug!();
+            };
+            for stmt in block.stmts {
+                let hir::StmtKind::Local(hir::Local {
+                    init: Some(init),
+                    source: hir::LocalSource::AsyncFn,
+                    pat,
+                    ..
+                }) = stmt.kind
+                else {
+                    bug!();
+                };
+                let hir::PatKind::Binding(hir::BindingAnnotation(hir::ByRef::No, _), _, _, _) =
+                    pat.kind
+                else {
+                    // Complex pattern, skip the non-upvar local.
+                    continue;
+                };
+                let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = init.kind else {
+                    bug!();
+                };
+                let hir::def::Res::Local(local_id) = path.res else {
+                    bug!();
+                };
+                let place = self.place_for_root_variable(closure_def_id, local_id);
+                delegate.capture_information.push((
+                    place,
+                    ty::CaptureInfo {
+                        capture_kind_expr_id: Some(init.hir_id),
+                        path_expr_id: Some(init.hir_id),
+                        capture_kind: UpvarCapture::ByValue,
+                    },
+                ));
+            }
+        }
+
         euv::ExprUseVisitor::new(
             &mut delegate,
             &self.infcx,
diff --git a/tests/ui/async-await/async-borrowck-escaping-closure-error.rs b/tests/ui/async-await/async-borrowck-escaping-closure-error.rs
index f8ff9186842..c02bac2d7dd 100644
--- a/tests/ui/async-await/async-borrowck-escaping-closure-error.rs
+++ b/tests/ui/async-await/async-borrowck-escaping-closure-error.rs
@@ -1,10 +1,10 @@
 // edition:2018
-// check-pass
 
 #![feature(async_closure)]
 fn foo() -> Box<dyn std::future::Future<Output = u32>> {
     let x = 0u32;
     Box::new((async || x)())
+    //~^ ERROR closure may outlive the current function, but it borrows `x`, which is owned by the current function
 }
 
 fn main() {
diff --git a/tests/ui/async-await/async-borrowck-escaping-closure-error.stderr b/tests/ui/async-await/async-borrowck-escaping-closure-error.stderr
new file mode 100644
index 00000000000..87851e1ae5b
--- /dev/null
+++ b/tests/ui/async-await/async-borrowck-escaping-closure-error.stderr
@@ -0,0 +1,21 @@
+error[E0373]: closure may outlive the current function, but it borrows `x`, which is owned by the current function
+  --> $DIR/async-borrowck-escaping-closure-error.rs:6:15
+   |
+LL |     Box::new((async || x)())
+   |               ^^^^^^^^ - `x` is borrowed here
+   |               |
+   |               may outlive borrowed value `x`
+   |
+note: closure is returned here
+  --> $DIR/async-borrowck-escaping-closure-error.rs:6:5
+   |
+LL |     Box::new((async || x)())
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+help: to force the closure to take ownership of `x` (and any other referenced variables), use the `move` keyword
+   |
+LL |     Box::new((async move || x)())
+   |                     ++++
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0373`.
diff --git a/tests/ui/async-await/issue-74072-lifetime-name-annotations.rs b/tests/ui/async-await/issue-74072-lifetime-name-annotations.rs
index 95683241aba..2d453e7891e 100644
--- a/tests/ui/async-await/issue-74072-lifetime-name-annotations.rs
+++ b/tests/ui/async-await/issue-74072-lifetime-name-annotations.rs
@@ -12,6 +12,7 @@ pub async fn async_fn(x: &mut i32) -> &i32 {
 
 pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
     (async move || {
+        //~^ captured variable cannot escape `FnMut` closure body
         let y = &*x;
         *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
         y
@@ -20,6 +21,7 @@ pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
 
 pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future<Output=&i32> {
     (async move || -> &i32 {
+        //~^ captured variable cannot escape `FnMut` closure body
         let y = &*x;
         *x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
         y
diff --git a/tests/ui/async-await/issue-74072-lifetime-name-annotations.stderr b/tests/ui/async-await/issue-74072-lifetime-name-annotations.stderr
index 628ba1a4818..9120d78164e 100644
--- a/tests/ui/async-await/issue-74072-lifetime-name-annotations.stderr
+++ b/tests/ui/async-await/issue-74072-lifetime-name-annotations.stderr
@@ -11,7 +11,7 @@ LL |     y
    |     - returning this value requires that `*x` is borrowed for `'1`
 
 error[E0506]: cannot assign to `*x` because it is borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:16:9
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:17:9
    |
 LL |         let y = &*x;
    |                 --- `*x` is borrowed here
@@ -22,11 +22,32 @@ LL |         y
 LL |     })()
    |     - return type of async closure is &'1 i32
 
+error: captured variable cannot escape `FnMut` closure body
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:14:20
+   |
+LL |   pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
+   |                        - variable defined here
+LL |       (async move || {
+   |  __________________-_^
+   | |                  |
+   | |                  inferred to be a `FnMut` closure
+LL | |
+LL | |         let y = &*x;
+   | |                   - variable captured here
+LL | |         *x += 1;
+LL | |         y
+LL | |     })()
+   | |_____^ returns an `async` block 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
+
 error[E0506]: cannot assign to `*x` because it is borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:24:9
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:26:9
    |
 LL |     (async move || -> &i32 {
    |                       - let's call the lifetime of this reference `'1`
+LL |
 LL |         let y = &*x;
    |                 --- `*x` is borrowed here
 LL |         *x += 1;
@@ -34,8 +55,28 @@ LL |         *x += 1;
 LL |         y
    |         - returning this value requires that `*x` is borrowed for `'1`
 
+error: captured variable cannot escape `FnMut` closure body
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:23:28
+   |
+LL |   pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future<Output=&i32> {
+   |                                             - variable defined here
+LL |       (async move || -> &i32 {
+   |  __________________________-_^
+   | |                          |
+   | |                          inferred to be a `FnMut` closure
+LL | |
+LL | |         let y = &*x;
+   | |                   - variable captured here
+LL | |         *x += 1;
+LL | |         y
+LL | |     })()
+   | |_____^ returns an `async` block 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
+
 error[E0506]: cannot assign to `*x` because it is borrowed
-  --> $DIR/issue-74072-lifetime-name-annotations.rs:32:9
+  --> $DIR/issue-74072-lifetime-name-annotations.rs:34:9
    |
 LL |         let y = &*x;
    |                 --- `*x` is borrowed here
@@ -46,6 +87,6 @@ LL |         y
 LL |     }
    |     - return type of async block is &'1 i32
 
-error: aborting due to 4 previous errors
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0506`.
diff --git a/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs b/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs
index a9d678c1e6a..66a432be357 100644
--- a/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs
+++ b/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs
@@ -9,7 +9,7 @@ impl Numberer {
     //~^ ERROR `async fn` is not permitted in Rust 2015
         interval: Duration,
         //~^ ERROR cannot find type `Duration` in this scope
-    ) -> Numberer { //~WARN: changes to closure capture in Rust 2021
+    ) -> Numberer {
         Numberer {}
     }
 }
diff --git a/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.stderr b/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.stderr
index 71e9e7602e8..60433e1c284 100644
--- a/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.stderr
+++ b/tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.stderr
@@ -18,32 +18,7 @@ help: consider importing this struct
 LL + use std::time::Duration;
    |
 
-warning: changes to closure capture in Rust 2021 will affect drop order
-  --> $DIR/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs:12:19
-   |
-LL |           interval: Duration,
-   |           -------- in Rust 2018, this causes the closure to capture `interval`, but in Rust 2021, it has no effect
-LL |
-LL |       ) -> Numberer {
-   |  _________________-_^
-   | |                 |
-   | |                 in Rust 2018, `interval` is dropped here along with the closure, but in Rust 2021 `interval` is not part of the closure
-LL | |         Numberer {}
-LL | |     }
-   | |_____^
-   |
-   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
-note: the lint level is defined here
-  --> $DIR/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs:1:9
-   |
-LL | #![warn(rust_2021_incompatible_closure_captures)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-help: add a dummy let to cause `interval` to be fully captured
-   |
-LL |     ) -> Numberer { let _ = &interval;
-   |                     ++++++++++++++++++
-
-error: aborting due to 2 previous errors; 1 warning emitted
+error: aborting due to 2 previous errors
 
 Some errors have detailed explanations: E0412, E0670.
 For more information about an error, try `rustc --explain E0412`.