Skip to content

Commit b2bb517

Browse files
Make sure that async closures (and fns) only capture their parent callable's parameters by move, and nothing else
1 parent f067fd6 commit b2bb517

8 files changed

+127
-36
lines changed

compiler/rustc_ast_lowering/src/item.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,10 +1278,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
12781278
};
12791279
let closure_id = coroutine_kind.closure_id();
12801280
let coroutine_expr = self.make_desugared_coroutine_expr(
1281-
// FIXME(async_closures): This should only move locals,
1282-
// and not upvars. Capturing closure upvars by ref doesn't
1283-
// work right now anyways, so whatever.
1284-
CaptureBy::Value { move_kw: rustc_span::DUMMY_SP },
1281+
// The default capture mode here is by-ref. Later on during upvar analysis,
1282+
// we will force the captured arguments to by-move, but for async closures,
1283+
// we want to make sure that we avoid unnecessarily moving captures, or else
1284+
// all async closures would default to `FnOnce` as their calling mode.
1285+
CaptureBy::Ref,
12851286
closure_id,
12861287
return_type_hint,
12871288
body_span,

compiler/rustc_hir_typeck/src/upvar.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
200200
capture_information: Default::default(),
201201
fake_reads: Default::default(),
202202
};
203+
204+
// As noted in `lower_coroutine_body_with_moved_arguments`, we default the capture mode
205+
// to `ByRef` for the `async {}` block internal to async fns/closure. This means
206+
// that we would *not* be moving all of the parameters into the async block by default.
207+
//
208+
// We force all of these arguments to be captured by move before we do expr use analysis.
209+
//
210+
// FIXME(async_closures): This could be cleaned up. It's a bit janky that we're just
211+
// moving all of the `LocalSource::AsyncFn` locals here.
212+
if let Some(hir::CoroutineKind::Desugared(
213+
_,
214+
hir::CoroutineSource::Fn | hir::CoroutineSource::Closure,
215+
)) = self.tcx.coroutine_kind(closure_def_id)
216+
{
217+
let hir::ExprKind::Block(block, _) = body.value.kind else {
218+
bug!();
219+
};
220+
for stmt in block.stmts {
221+
let hir::StmtKind::Local(hir::Local {
222+
init: Some(init),
223+
source: hir::LocalSource::AsyncFn,
224+
pat,
225+
..
226+
}) = stmt.kind
227+
else {
228+
bug!();
229+
};
230+
let hir::PatKind::Binding(hir::BindingAnnotation(hir::ByRef::No, _), _, _, _) =
231+
pat.kind
232+
else {
233+
// Complex pattern, skip the non-upvar local.
234+
continue;
235+
};
236+
let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = init.kind else {
237+
bug!();
238+
};
239+
let hir::def::Res::Local(local_id) = path.res else {
240+
bug!();
241+
};
242+
let place = self.place_for_root_variable(closure_def_id, local_id);
243+
delegate.capture_information.push((
244+
place,
245+
ty::CaptureInfo {
246+
capture_kind_expr_id: Some(init.hir_id),
247+
path_expr_id: Some(init.hir_id),
248+
capture_kind: UpvarCapture::ByValue,
249+
},
250+
));
251+
}
252+
}
253+
203254
euv::ExprUseVisitor::new(
204255
&mut delegate,
205256
&self.infcx,

tests/ui/async-await/async-borrowck-escaping-closure-error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// edition:2018
2-
// check-pass
32

43
#![feature(async_closure)]
54
fn foo() -> Box<dyn std::future::Future<Output = u32>> {
65
let x = 0u32;
76
Box::new((async || x)())
7+
//~^ ERROR closure may outlive the current function, but it borrows `x`, which is owned by the current function
88
}
99

1010
fn main() {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
error[E0373]: closure may outlive the current function, but it borrows `x`, which is owned by the current function
2+
--> $DIR/async-borrowck-escaping-closure-error.rs:6:15
3+
|
4+
LL | Box::new((async || x)())
5+
| ^^^^^^^^ - `x` is borrowed here
6+
| |
7+
| may outlive borrowed value `x`
8+
|
9+
note: closure is returned here
10+
--> $DIR/async-borrowck-escaping-closure-error.rs:6:5
11+
|
12+
LL | Box::new((async || x)())
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^
14+
help: to force the closure to take ownership of `x` (and any other referenced variables), use the `move` keyword
15+
|
16+
LL | Box::new((async move || x)())
17+
| ++++
18+
19+
error: aborting due to 1 previous error
20+
21+
For more information about this error, try `rustc --explain E0373`.

tests/ui/async-await/issue-74072-lifetime-name-annotations.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub async fn async_fn(x: &mut i32) -> &i32 {
1212

1313
pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
1414
(async move || {
15+
//~^ captured variable cannot escape `FnMut` closure body
1516
let y = &*x;
1617
*x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
1718
y
@@ -20,6 +21,7 @@ pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
2021

2122
pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future<Output=&i32> {
2223
(async move || -> &i32 {
24+
//~^ captured variable cannot escape `FnMut` closure body
2325
let y = &*x;
2426
*x += 1; //~ ERROR cannot assign to `*x` because it is borrowed
2527
y

tests/ui/async-await/issue-74072-lifetime-name-annotations.stderr

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ LL | y
1111
| - returning this value requires that `*x` is borrowed for `'1`
1212

1313
error[E0506]: cannot assign to `*x` because it is borrowed
14-
--> $DIR/issue-74072-lifetime-name-annotations.rs:16:9
14+
--> $DIR/issue-74072-lifetime-name-annotations.rs:17:9
1515
|
1616
LL | let y = &*x;
1717
| --- `*x` is borrowed here
@@ -22,20 +22,61 @@ LL | y
2222
LL | })()
2323
| - return type of async closure is &'1 i32
2424

25+
error: captured variable cannot escape `FnMut` closure body
26+
--> $DIR/issue-74072-lifetime-name-annotations.rs:14:20
27+
|
28+
LL | pub fn async_closure(x: &mut i32) -> impl Future<Output=&i32> {
29+
| - variable defined here
30+
LL | (async move || {
31+
| __________________-_^
32+
| | |
33+
| | inferred to be a `FnMut` closure
34+
LL | |
35+
LL | | let y = &*x;
36+
| | - variable captured here
37+
LL | | *x += 1;
38+
LL | | y
39+
LL | | })()
40+
| |_____^ returns an `async` block that contains a reference to a captured variable, which then escapes the closure body
41+
|
42+
= note: `FnMut` closures only have access to their captured variables while they are executing...
43+
= note: ...therefore, they cannot allow references to captured variables to escape
44+
2545
error[E0506]: cannot assign to `*x` because it is borrowed
26-
--> $DIR/issue-74072-lifetime-name-annotations.rs:24:9
46+
--> $DIR/issue-74072-lifetime-name-annotations.rs:26:9
2747
|
2848
LL | (async move || -> &i32 {
2949
| - let's call the lifetime of this reference `'1`
50+
LL |
3051
LL | let y = &*x;
3152
| --- `*x` is borrowed here
3253
LL | *x += 1;
3354
| ^^^^^^^ `*x` is assigned to here but it was already borrowed
3455
LL | y
3556
| - returning this value requires that `*x` is borrowed for `'1`
3657

58+
error: captured variable cannot escape `FnMut` closure body
59+
--> $DIR/issue-74072-lifetime-name-annotations.rs:23:28
60+
|
61+
LL | pub fn async_closure_explicit_return_type(x: &mut i32) -> impl Future<Output=&i32> {
62+
| - variable defined here
63+
LL | (async move || -> &i32 {
64+
| __________________________-_^
65+
| | |
66+
| | inferred to be a `FnMut` closure
67+
LL | |
68+
LL | | let y = &*x;
69+
| | - variable captured here
70+
LL | | *x += 1;
71+
LL | | y
72+
LL | | })()
73+
| |_____^ returns an `async` block that contains a reference to a captured variable, which then escapes the closure body
74+
|
75+
= note: `FnMut` closures only have access to their captured variables while they are executing...
76+
= note: ...therefore, they cannot allow references to captured variables to escape
77+
3778
error[E0506]: cannot assign to `*x` because it is borrowed
38-
--> $DIR/issue-74072-lifetime-name-annotations.rs:32:9
79+
--> $DIR/issue-74072-lifetime-name-annotations.rs:34:9
3980
|
4081
LL | let y = &*x;
4182
| --- `*x` is borrowed here
@@ -46,6 +87,6 @@ LL | y
4687
LL | }
4788
| - return type of async block is &'1 i32
4889

49-
error: aborting due to 4 previous errors
90+
error: aborting due to 6 previous errors
5091

5192
For more information about this error, try `rustc --explain E0506`.

tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ impl Numberer {
99
//~^ ERROR `async fn` is not permitted in Rust 2015
1010
interval: Duration,
1111
//~^ ERROR cannot find type `Duration` in this scope
12-
) -> Numberer { //~WARN: changes to closure capture in Rust 2021
12+
) -> Numberer {
1313
Numberer {}
1414
}
1515
}

tests/ui/span/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.stderr

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,7 @@ help: consider importing this struct
1818
LL + use std::time::Duration;
1919
|
2020

21-
warning: changes to closure capture in Rust 2021 will affect drop order
22-
--> $DIR/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs:12:19
23-
|
24-
LL | interval: Duration,
25-
| -------- in Rust 2018, this causes the closure to capture `interval`, but in Rust 2021, it has no effect
26-
LL |
27-
LL | ) -> Numberer {
28-
| _________________-_^
29-
| | |
30-
| | in Rust 2018, `interval` is dropped here along with the closure, but in Rust 2021 `interval` is not part of the closure
31-
LL | | Numberer {}
32-
LL | | }
33-
| |_____^
34-
|
35-
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
36-
note: the lint level is defined here
37-
--> $DIR/drop-location-span-error-rust-2021-incompatible-closure-captures-96258.rs:1:9
38-
|
39-
LL | #![warn(rust_2021_incompatible_closure_captures)]
40-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
41-
help: add a dummy let to cause `interval` to be fully captured
42-
|
43-
LL | ) -> Numberer { let _ = &interval;
44-
| ++++++++++++++++++
45-
46-
error: aborting due to 2 previous errors; 1 warning emitted
21+
error: aborting due to 2 previous errors
4722

4823
Some errors have detailed explanations: E0412, E0670.
4924
For more information about an error, try `rustc --explain E0412`.

0 commit comments

Comments
 (0)