Skip to content

Commit 16939a5

Browse files
CentrilCentril
authored
Rollup merge of rust-lang#60437 - davidtwco:issue-60236, r=nikomatsakis
Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments. Fixes rust-lang#60236 and fixes rust-lang#60438. This PR modifies the lowering of `async fn` arguments so that the drop order matches the equivalent `fn`. Previously, async function arguments were lowered as shown below: async fn foo(<pattern>: <ty>) { async move { } } // <-- dropped as you "exit" the fn // ...becomes... fn foo(__arg0: <ty>) { async move { let <pattern> = __arg0; } // <-- dropped as you "exit" the async block } After this PR, async function arguments will be lowered as: async fn foo(<pattern>: <ty>, <pattern>: <ty>, <pattern>: <ty>) { async move { } } // <-- dropped as you "exit" the fn // ...becomes... fn foo(__arg0: <ty>, __arg1: <ty>, __arg2: <ty>) { async move { let __arg2 = __arg2; let <pattern> = __arg2; let __arg1 = __arg1; let <pattern> = __arg1; let __arg0 = __arg0; let <pattern> = __arg0; } // <-- dropped as you "exit" the async block } If `<pattern>` is a simple ident, then it is lowered to a single `let <pattern> = <pattern>;` statement as an optimization. This PR also stops users from referring to the generated `__argN` identifiers. r? @nikomatsakis
2 parents 12bf981 + 1fedb0a commit 16939a5

File tree

13 files changed

+505
-232
lines changed

13 files changed

+505
-232
lines changed

‎src/librustc/hir/lowering.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2996,8 +2996,33 @@ impl<'a> LoweringContext<'a> {
29962996
if let IsAsync::Async { closure_id, ref arguments, .. } = asyncness {
29972997
let mut body = body.clone();
29982998

2999+
// Async function arguments are lowered into the closure body so that they are
3000+
// captured and so that the drop order matches the equivalent non-async functions.
3001+
//
3002+
// async fn foo(<pattern>: <ty>, <pattern>: <ty>, <pattern>: <ty>) {
3003+
// async move {
3004+
// }
3005+
// }
3006+
//
3007+
// // ...becomes...
3008+
// fn foo(__arg0: <ty>, __arg1: <ty>, __arg2: <ty>) {
3009+
// async move {
3010+
// let __arg2 = __arg2;
3011+
// let <pattern> = __arg2;
3012+
// let __arg1 = __arg1;
3013+
// let <pattern> = __arg1;
3014+
// let __arg0 = __arg0;
3015+
// let <pattern> = __arg0;
3016+
// }
3017+
// }
3018+
//
3019+
// If `<pattern>` is a simple ident, then it is lowered to a single
3020+
// `let <pattern> = <pattern>;` statement as an optimization.
29993021
for a in arguments.iter().rev() {
3000-
body.stmts.insert(0, a.stmt.clone());
3022+
if let Some(pat_stmt) = a.pat_stmt.clone() {
3023+
body.stmts.insert(0, pat_stmt);
3024+
}
3025+
body.stmts.insert(0, a.move_stmt.clone());
30013026
}
30023027

30033028
let async_expr = this.make_async_expr(
@@ -3093,7 +3118,11 @@ impl<'a> LoweringContext<'a> {
30933118
let mut decl = decl.clone();
30943119
// Replace the arguments of this async function with the generated
30953120
// arguments that will be moved into the closure.
3096-
decl.inputs = arguments.clone().drain(..).map(|a| a.arg).collect();
3121+
for (i, a) in arguments.clone().drain(..).enumerate() {
3122+
if let Some(arg) = a.arg {
3123+
decl.inputs[i] = arg;
3124+
}
3125+
}
30973126
lower_fn(&decl)
30983127
} else {
30993128
lower_fn(decl)
@@ -3590,7 +3619,11 @@ impl<'a> LoweringContext<'a> {
35903619
let mut sig = sig.clone();
35913620
// Replace the arguments of this async function with the generated
35923621
// arguments that will be moved into the closure.
3593-
sig.decl.inputs = arguments.clone().drain(..).map(|a| a.arg).collect();
3622+
for (i, a) in arguments.clone().drain(..).enumerate() {
3623+
if let Some(arg) = a.arg {
3624+
sig.decl.inputs[i] = arg;
3625+
}
3626+
}
35943627
lower_method(&sig)
35953628
} else {
35963629
lower_method(sig)

‎src/librustc/hir/map/def_collector.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ impl<'a> DefCollector<'a> {
9494
// Walk the generated arguments for the `async fn`.
9595
for a in arguments {
9696
use visit::Visitor;
97-
this.visit_ty(&a.arg.ty);
97+
if let Some(arg) = &a.arg {
98+
this.visit_ty(&arg.ty);
99+
}
98100
}
99101

100102
// We do not invoke `walk_fn_decl` as this will walk the arguments that are being
@@ -105,10 +107,13 @@ impl<'a> DefCollector<'a> {
105107
*closure_id, DefPathData::ClosureExpr, REGULAR_SPACE, span,
106108
);
107109
this.with_parent(closure_def, |this| {
110+
use visit::Visitor;
111+
// Walk each of the generated statements before the regular block body.
108112
for a in arguments {
109-
use visit::Visitor;
110-
// Walk each of the generated statements before the regular block body.
111-
this.visit_stmt(&a.stmt);
113+
this.visit_stmt(&a.move_stmt);
114+
if let Some(pat_stmt) = &a.pat_stmt {
115+
this.visit_stmt(&pat_stmt);
116+
}
112117
}
113118

114119
visit::walk_block(this, &body);

‎src/librustc/lint/context.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,14 +1334,19 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
13341334
if let ast::IsAsync::Async { ref arguments, .. } = header.asyncness.node {
13351335
for a in arguments {
13361336
// Visit the argument..
1337-
self.visit_pat(&a.arg.pat);
1338-
if let ast::ArgSource::AsyncFn(pat) = &a.arg.source {
1339-
self.visit_pat(pat);
1337+
if let Some(arg) = &a.arg {
1338+
self.visit_pat(&arg.pat);
1339+
if let ast::ArgSource::AsyncFn(pat) = &arg.source {
1340+
self.visit_pat(pat);
1341+
}
1342+
self.visit_ty(&arg.ty);
13401343
}
1341-
self.visit_ty(&a.arg.ty);
13421344

13431345
// ..and the statement.
1344-
self.visit_stmt(&a.stmt);
1346+
self.visit_stmt(&a.move_stmt);
1347+
if let Some(pat_stmt) = &a.pat_stmt {
1348+
self.visit_stmt(&pat_stmt);
1349+
}
13451350
}
13461351
}
13471352
}

‎src/librustc_resolve/lib.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,13 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
862862
// Walk the generated async arguments if this is an `async fn`, otherwise walk the
863863
// normal arguments.
864864
if let IsAsync::Async { ref arguments, .. } = asyncness {
865-
for a in arguments { add_argument(&a.arg); }
865+
for (i, a) in arguments.iter().enumerate() {
866+
if let Some(arg) = &a.arg {
867+
add_argument(&arg);
868+
} else {
869+
add_argument(&declaration.inputs[i]);
870+
}
871+
}
866872
} else {
867873
for a in &declaration.inputs { add_argument(a); }
868874
}
@@ -882,8 +888,11 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
882888
let mut body = body.clone();
883889
// Insert the generated statements into the body before attempting to
884890
// resolve names.
885-
for a in arguments {
886-
body.stmts.insert(0, a.stmt.clone());
891+
for a in arguments.iter().rev() {
892+
if let Some(pat_stmt) = a.pat_stmt.clone() {
893+
body.stmts.insert(0, pat_stmt);
894+
}
895+
body.stmts.insert(0, a.move_stmt.clone());
887896
}
888897
self.visit_block(&body);
889898
} else {
@@ -4174,7 +4183,7 @@ impl<'a> Resolver<'a> {
41744183
let add_module_candidates = |module: Module<'_>, names: &mut Vec<TypoSuggestion>| {
41754184
for (&(ident, _), resolution) in module.resolutions.borrow().iter() {
41764185
if let Some(binding) = resolution.borrow().binding {
4177-
if filter_fn(binding.def()) {
4186+
if !ident.name.is_gensymed() && filter_fn(binding.def()) {
41784187
names.push(TypoSuggestion {
41794188
candidate: ident.name,
41804189
article: binding.def().article(),
@@ -4192,7 +4201,7 @@ impl<'a> Resolver<'a> {
41924201
for rib in self.ribs[ns].iter().rev() {
41934202
// Locals and type parameters
41944203
for (ident, def) in &rib.bindings {
4195-
if filter_fn(*def) {
4204+
if !ident.name.is_gensymed() && filter_fn(*def) {
41964205
names.push(TypoSuggestion {
41974206
candidate: ident.name,
41984207
article: def.article(),
@@ -4219,7 +4228,7 @@ impl<'a> Resolver<'a> {
42194228
index: CRATE_DEF_INDEX,
42204229
});
42214230

4222-
if filter_fn(crate_mod) {
4231+
if !ident.name.is_gensymed() && filter_fn(crate_mod) {
42234232
Some(TypoSuggestion {
42244233
candidate: ident.name,
42254234
article: "a",
@@ -4242,13 +4251,16 @@ impl<'a> Resolver<'a> {
42424251
// Add primitive types to the mix
42434252
if filter_fn(Def::PrimTy(Bool)) {
42444253
names.extend(
4245-
self.primitive_type_table.primitive_types.iter().map(|(name, _)| {
4246-
TypoSuggestion {
4247-
candidate: *name,
4248-
article: "a",
4249-
kind: "primitive type",
4250-
}
4251-
})
4254+
self.primitive_type_table.primitive_types
4255+
.iter()
4256+
.filter(|(name, _)| !name.is_gensymed())
4257+
.map(|(name, _)| {
4258+
TypoSuggestion {
4259+
candidate: *name,
4260+
article: "a",
4261+
kind: "primitive type",
4262+
}
4263+
})
42524264
)
42534265
}
42544266
} else {

‎src/libsyntax/ast.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,10 +1865,14 @@ pub enum Unsafety {
18651865
pub struct AsyncArgument {
18661866
/// `__arg0`
18671867
pub ident: Ident,
1868-
/// `__arg0: <ty>` argument to replace existing function argument `<pat>: <ty>`.
1869-
pub arg: Arg,
1870-
/// `let <pat>: <ty> = __arg0;` statement to be inserted at the start of the block.
1871-
pub stmt: Stmt,
1868+
/// `__arg0: <ty>` argument to replace existing function argument `<pat>: <ty>`. Only if
1869+
/// argument is not a simple binding.
1870+
pub arg: Option<Arg>,
1871+
/// `let __arg0 = __arg0;` statement to be inserted at the start of the block.
1872+
pub move_stmt: Stmt,
1873+
/// `let <pat> = __arg0;` statement to be inserted at the start of the block, after matching
1874+
/// move statement. Only if argument is not a simple binding.
1875+
pub pat_stmt: Option<Stmt>,
18721876
}
18731877

18741878
#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]

‎src/libsyntax/ext/placeholders.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,10 @@ impl<'a, 'b> MutVisitor for PlaceholderExpander<'a, 'b> {
199199

200200
if let ast::IsAsync::Async { ref mut arguments, .. } = a {
201201
for argument in arguments.iter_mut() {
202-
self.next_id(&mut argument.stmt.id);
202+
self.next_id(&mut argument.move_stmt.id);
203+
if let Some(ref mut pat_stmt) = &mut argument.pat_stmt {
204+
self.next_id(&mut pat_stmt.id);
205+
}
203206
}
204207
}
205208
}

‎src/libsyntax/mut_visit.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -694,13 +694,21 @@ pub fn noop_visit_asyncness<T: MutVisitor>(asyncness: &mut IsAsync, vis: &mut T)
694694
IsAsync::Async { closure_id, return_impl_trait_id, ref mut arguments } => {
695695
vis.visit_id(closure_id);
696696
vis.visit_id(return_impl_trait_id);
697-
for AsyncArgument { ident, arg, stmt } in arguments.iter_mut() {
697+
for AsyncArgument { ident, arg, pat_stmt, move_stmt } in arguments.iter_mut() {
698698
vis.visit_ident(ident);
699-
vis.visit_arg(arg);
700-
visit_clobber(stmt, |stmt| {
699+
if let Some(arg) = arg {
700+
vis.visit_arg(arg);
701+
}
702+
visit_clobber(move_stmt, |stmt| {
701703
vis.flat_map_stmt(stmt)
702704
.expect_one("expected visitor to produce exactly one item")
703705
});
706+
visit_opt(pat_stmt, |stmt| {
707+
visit_clobber(stmt, |stmt| {
708+
vis.flat_map_stmt(stmt)
709+
.expect_one("expected visitor to produce exactly one item")
710+
})
711+
});
704712
}
705713
}
706714
IsAsync::NotAsync => {}

‎src/libsyntax/parse/parser.rs

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8720,27 +8720,46 @@ impl<'a> Parser<'a> {
87208720

87218721
// Construct a name for our temporary argument.
87228722
let name = format!("__arg{}", index);
8723-
let ident = Ident::from_str(&name);
8723+
let ident = Ident::from_str(&name).gensym();
8724+
8725+
// Check if this is a ident pattern, if so, we can optimize and avoid adding a
8726+
// `let <pat> = __argN;` statement, instead just adding a `let <pat> = <pat>;`
8727+
// statement.
8728+
let (ident, is_simple_pattern) = match input.pat.node {
8729+
PatKind::Ident(_, ident, _) => (ident, true),
8730+
_ => (ident, false),
8731+
};
87248732

87258733
// Construct an argument representing `__argN: <ty>` to replace the argument of the
8726-
// async function.
8727-
let arg = Arg {
8728-
ty: input.ty.clone(),
8729-
id,
8734+
// async function if it isn't a simple pattern.
8735+
let arg = if is_simple_pattern {
8736+
None
8737+
} else {
8738+
Some(Arg {
8739+
ty: input.ty.clone(),
8740+
id,
8741+
pat: P(Pat {
8742+
id,
8743+
node: PatKind::Ident(
8744+
BindingMode::ByValue(Mutability::Immutable), ident, None,
8745+
),
8746+
span,
8747+
}),
8748+
source: ArgSource::AsyncFn(input.pat.clone()),
8749+
})
8750+
};
8751+
8752+
// Construct a `let __argN = __argN;` statement to insert at the top of the
8753+
// async closure. This makes sure that the argument is captured by the closure and
8754+
// that the drop order is correct.
8755+
let move_local = Local {
87308756
pat: P(Pat {
87318757
id,
87328758
node: PatKind::Ident(
87338759
BindingMode::ByValue(Mutability::Immutable), ident, None,
87348760
),
87358761
span,
87368762
}),
8737-
source: ArgSource::AsyncFn(input.pat.clone()),
8738-
};
8739-
8740-
// Construct a `let <pat> = __argN;` statement to insert at the top of the
8741-
// async closure.
8742-
let local = P(Local {
8743-
pat: input.pat.clone(),
87448763
// We explicitly do not specify the type for this statement. When the user's
87458764
// argument type is `impl Trait` then this would require the
87468765
// `impl_trait_in_bindings` feature to also be present for that same type to
@@ -8760,10 +8779,25 @@ impl<'a> Parser<'a> {
87608779
span,
87618780
attrs: ThinVec::new(),
87628781
source: LocalSource::AsyncFn,
8763-
});
8764-
let stmt = Stmt { id, node: StmtKind::Local(local), span, };
8782+
};
8783+
8784+
// Construct a `let <pat> = __argN;` statement to insert at the top of the
8785+
// async closure if this isn't a simple pattern.
8786+
let pat_stmt = if is_simple_pattern {
8787+
None
8788+
} else {
8789+
Some(Stmt {
8790+
id,
8791+
node: StmtKind::Local(P(Local {
8792+
pat: input.pat.clone(),
8793+
..move_local.clone()
8794+
})),
8795+
span,
8796+
})
8797+
};
87658798

8766-
arguments.push(AsyncArgument { ident, arg, stmt });
8799+
let move_stmt = Stmt { id, node: StmtKind::Local(P(move_local)), span };
8800+
arguments.push(AsyncArgument { ident, arg, pat_stmt, move_stmt });
87678801
}
87688802
}
87698803
}

0 commit comments

Comments
 (0)