about summary refs log tree commit diff
path: root/src/libsyntax
diff options
context:
space:
mode:
authorMazdak Farrokhzad <twingoow@gmail.com>2019-05-02 01:09:29 +0200
committerGitHub <noreply@github.com>2019-05-02 01:09:29 +0200
commit16939a50ea440e72cb6ecefdaabb988addb1ec0e (patch)
treee9b880050e2903aee0d12b7b7f4156772bdab0d3 /src/libsyntax
parent12bf98155249783583a91863c5dccf9e346f1226 (diff)
parent1fedb0a20bfe03e1bf7d5c2576806c761b4e3963 (diff)
downloadrust-16939a50ea440e72cb6ecefdaabb988addb1ec0e.tar.gz
rust-16939a50ea440e72cb6ecefdaabb988addb1ec0e.zip
Rollup merge of #60437 - davidtwco:issue-60236, r=nikomatsakis
Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments.

Fixes #60236 and fixes #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
Diffstat (limited to 'src/libsyntax')
-rw-r--r--src/libsyntax/ast.rs12
-rw-r--r--src/libsyntax/ext/placeholders.rs5
-rw-r--r--src/libsyntax/mut_visit.rs14
-rw-r--r--src/libsyntax/parse/parser.rs64
4 files changed, 72 insertions, 23 deletions
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index a20bf91a6ad..33b8c76bb53 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -1865,10 +1865,14 @@ pub enum Unsafety {
 pub struct AsyncArgument {
     /// `__arg0`
     pub ident: Ident,
-    /// `__arg0: <ty>` argument to replace existing function argument `<pat>: <ty>`.
-    pub arg: Arg,
-    /// `let <pat>: <ty> = __arg0;` statement to be inserted at the start of the block.
-    pub stmt: Stmt,
+    /// `__arg0: <ty>` argument to replace existing function argument `<pat>: <ty>`. Only if
+    /// argument is not a simple binding.
+    pub arg: Option<Arg>,
+    /// `let __arg0 = __arg0;` statement to be inserted at the start of the block.
+    pub move_stmt: Stmt,
+    /// `let <pat> = __arg0;` statement to be inserted at the start of the block, after matching
+    /// move statement. Only if argument is not a simple binding.
+    pub pat_stmt: Option<Stmt>,
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs
index 68cd3c28676..f5e18e98436 100644
--- a/src/libsyntax/ext/placeholders.rs
+++ b/src/libsyntax/ext/placeholders.rs
@@ -199,7 +199,10 @@ impl<'a, 'b> MutVisitor for PlaceholderExpander<'a, 'b> {
 
         if let ast::IsAsync::Async { ref mut arguments, .. } = a {
             for argument in arguments.iter_mut() {
-                self.next_id(&mut argument.stmt.id);
+                self.next_id(&mut argument.move_stmt.id);
+                if let Some(ref mut pat_stmt) = &mut argument.pat_stmt {
+                    self.next_id(&mut pat_stmt.id);
+                }
             }
         }
     }
diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs
index d3441a2039b..2e09235ca77 100644
--- a/src/libsyntax/mut_visit.rs
+++ b/src/libsyntax/mut_visit.rs
@@ -694,13 +694,21 @@ pub fn noop_visit_asyncness<T: MutVisitor>(asyncness: &mut IsAsync, vis: &mut T)
         IsAsync::Async { closure_id, return_impl_trait_id, ref mut arguments } => {
             vis.visit_id(closure_id);
             vis.visit_id(return_impl_trait_id);
-            for AsyncArgument { ident, arg, stmt } in arguments.iter_mut() {
+            for AsyncArgument { ident, arg, pat_stmt, move_stmt } in arguments.iter_mut() {
                 vis.visit_ident(ident);
-                vis.visit_arg(arg);
-                visit_clobber(stmt, |stmt| {
+                if let Some(arg) = arg {
+                    vis.visit_arg(arg);
+                }
+                visit_clobber(move_stmt, |stmt| {
                     vis.flat_map_stmt(stmt)
                         .expect_one("expected visitor to produce exactly one item")
                 });
+                visit_opt(pat_stmt, |stmt| {
+                    visit_clobber(stmt, |stmt| {
+                        vis.flat_map_stmt(stmt)
+                            .expect_one("expected visitor to produce exactly one item")
+                    })
+                });
             }
         }
         IsAsync::NotAsync => {}
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 6c64055ded1..f70acb3e7da 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -8720,13 +8720,39 @@ impl<'a> Parser<'a> {
 
                 // Construct a name for our temporary argument.
                 let name = format!("__arg{}", index);
-                let ident = Ident::from_str(&name);
+                let ident = Ident::from_str(&name).gensym();
+
+                // Check if this is a ident pattern, if so, we can optimize and avoid adding a
+                // `let <pat> = __argN;` statement, instead just adding a `let <pat> = <pat>;`
+                // statement.
+                let (ident, is_simple_pattern) = match input.pat.node {
+                    PatKind::Ident(_, ident, _) => (ident, true),
+                    _ => (ident, false),
+                };
 
                 // Construct an argument representing `__argN: <ty>` to replace the argument of the
-                // async function.
-                let arg = Arg {
-                    ty: input.ty.clone(),
-                    id,
+                // async function if it isn't a simple pattern.
+                let arg = if is_simple_pattern {
+                    None
+                } else {
+                    Some(Arg {
+                        ty: input.ty.clone(),
+                        id,
+                        pat: P(Pat {
+                            id,
+                            node: PatKind::Ident(
+                                BindingMode::ByValue(Mutability::Immutable), ident, None,
+                            ),
+                            span,
+                        }),
+                        source: ArgSource::AsyncFn(input.pat.clone()),
+                    })
+                };
+
+                // Construct a `let __argN = __argN;` statement to insert at the top of the
+                // async closure. This makes sure that the argument is captured by the closure and
+                // that the drop order is correct.
+                let move_local = Local {
                     pat: P(Pat {
                         id,
                         node: PatKind::Ident(
@@ -8734,13 +8760,6 @@ impl<'a> Parser<'a> {
                         ),
                         span,
                     }),
-                    source: ArgSource::AsyncFn(input.pat.clone()),
-                };
-
-                // Construct a `let <pat> = __argN;` statement to insert at the top of the
-                // async closure.
-                let local = P(Local {
-                    pat: input.pat.clone(),
                     // We explicitly do not specify the type for this statement. When the user's
                     // argument type is `impl Trait` then this would require the
                     // `impl_trait_in_bindings` feature to also be present for that same type to
@@ -8760,10 +8779,25 @@ impl<'a> Parser<'a> {
                     span,
                     attrs: ThinVec::new(),
                     source: LocalSource::AsyncFn,
-                });
-                let stmt = Stmt { id, node: StmtKind::Local(local), span, };
+                };
+
+                // Construct a `let <pat> = __argN;` statement to insert at the top of the
+                // async closure if this isn't a simple pattern.
+                let pat_stmt = if is_simple_pattern {
+                    None
+                } else {
+                    Some(Stmt {
+                        id,
+                        node: StmtKind::Local(P(Local {
+                            pat: input.pat.clone(),
+                            ..move_local.clone()
+                        })),
+                        span,
+                    })
+                };
 
-                arguments.push(AsyncArgument { ident, arg, stmt });
+                let move_stmt = Stmt { id, node: StmtKind::Local(P(move_local)), span };
+                arguments.push(AsyncArgument { ident, arg, pat_stmt, move_stmt });
             }
         }
     }