From b05d5db87bd9a3f31729a5c48dc5dd5bec6dbd2d Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 1 May 2019 13:35:34 +0100 Subject: [PATCH 1/3] Ensure that drop order of `async fn` matches `fn`. This commit 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(: ) { async move { } } // <-- dropped as you "exit" the fn // ...becomes... fn foo(__arg0: ) { async move { let = __arg0; } // <-- dropped as you "exit" the async block } After this PR, async function arguments will be lowered as: async fn foo(: , : , : ) { async move { } } // <-- dropped as you "exit" the fn // ...becomes... fn foo(__arg0: , __arg1: , __arg2: ) { async move { let __arg2 = __arg2; let = __arg2; let __arg1 = __arg1; let = __arg1; let __arg0 = __arg0; let = __arg0; } // <-- dropped as you "exit" the async block } If `` is a simple ident, then it is lowered to a single `let = ;` statement as an optimization. --- src/librustc/hir/lowering.rs | 39 ++- src/librustc/hir/map/def_collector.rs | 13 +- src/librustc/lint/context.rs | 15 +- src/librustc_resolve/lib.rs | 15 +- src/libsyntax/ast.rs | 12 +- src/libsyntax/ext/placeholders.rs | 5 +- src/libsyntax/mut_visit.rs | 14 +- src/libsyntax/parse/parser.rs | 62 ++++- ...wait-drop-order-for-async-fn-parameters.rs | 263 ++++++++++++++++++ src/test/run-pass/issue-54716.rs | 184 ------------ 10 files changed, 401 insertions(+), 221 deletions(-) create mode 100644 src/test/run-pass/async-await-drop-order-for-async-fn-parameters.rs delete mode 100644 src/test/run-pass/issue-54716.rs diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index df455a725c5ba..e7fe9bf40ba79 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2996,8 +2996,33 @@ impl<'a> LoweringContext<'a> { if let IsAsync::Async { closure_id, ref arguments, .. } = asyncness { let mut body = body.clone(); + // Async function arguments are lowered into the closure body so that they are + // captured and so that the drop order matches the equivalent non-async functions. + // + // async fn foo(: , : , : ) { + // async move { + // } + // } + // + // // ...becomes... + // fn foo(__arg0: , __arg1: , __arg2: ) { + // async move { + // let __arg2 = __arg2; + // let = __arg2; + // let __arg1 = __arg1; + // let = __arg1; + // let __arg0 = __arg0; + // let = __arg0; + // } + // } + // + // If `` is a simple ident, then it is lowered to a single + // `let = ;` statement as an optimization. for a in arguments.iter().rev() { - body.stmts.insert(0, a.stmt.clone()); + if let Some(pat_stmt) = a.pat_stmt.clone() { + body.stmts.insert(0, pat_stmt); + } + body.stmts.insert(0, a.move_stmt.clone()); } let async_expr = this.make_async_expr( @@ -3093,7 +3118,11 @@ impl<'a> LoweringContext<'a> { let mut decl = decl.clone(); // Replace the arguments of this async function with the generated // arguments that will be moved into the closure. - decl.inputs = arguments.clone().drain(..).map(|a| a.arg).collect(); + for (i, a) in arguments.clone().drain(..).enumerate() { + if let Some(arg) = a.arg { + decl.inputs[i] = arg; + } + } lower_fn(&decl) } else { lower_fn(decl) @@ -3590,7 +3619,11 @@ impl<'a> LoweringContext<'a> { let mut sig = sig.clone(); // Replace the arguments of this async function with the generated // arguments that will be moved into the closure. - sig.decl.inputs = arguments.clone().drain(..).map(|a| a.arg).collect(); + for (i, a) in arguments.clone().drain(..).enumerate() { + if let Some(arg) = a.arg { + sig.decl.inputs[i] = arg; + } + } lower_method(&sig) } else { lower_method(sig) diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index 0fa9738532215..78de85398594e 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -94,7 +94,9 @@ impl<'a> DefCollector<'a> { // Walk the generated arguments for the `async fn`. for a in arguments { use visit::Visitor; - this.visit_ty(&a.arg.ty); + if let Some(arg) = &a.arg { + this.visit_ty(&arg.ty); + } } // We do not invoke `walk_fn_decl` as this will walk the arguments that are being @@ -105,10 +107,13 @@ impl<'a> DefCollector<'a> { *closure_id, DefPathData::ClosureExpr, REGULAR_SPACE, span, ); this.with_parent(closure_def, |this| { + use visit::Visitor; + // Walk each of the generated statements before the regular block body. for a in arguments { - use visit::Visitor; - // Walk each of the generated statements before the regular block body. - this.visit_stmt(&a.stmt); + this.visit_stmt(&a.move_stmt); + if let Some(pat_stmt) = &a.pat_stmt { + this.visit_stmt(&pat_stmt); + } } visit::walk_block(this, &body); diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index f5cb4cfa29f5c..8d5c1798e0fa4 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -1334,14 +1334,19 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> if let ast::IsAsync::Async { ref arguments, .. } = header.asyncness.node { for a in arguments { // Visit the argument.. - self.visit_pat(&a.arg.pat); - if let ast::ArgSource::AsyncFn(pat) = &a.arg.source { - self.visit_pat(pat); + if let Some(arg) = &a.arg { + self.visit_pat(&arg.pat); + if let ast::ArgSource::AsyncFn(pat) = &arg.source { + self.visit_pat(pat); + } + self.visit_ty(&arg.ty); } - self.visit_ty(&a.arg.ty); // ..and the statement. - self.visit_stmt(&a.stmt); + self.visit_stmt(&a.move_stmt); + if let Some(pat_stmt) = &a.pat_stmt { + self.visit_stmt(&pat_stmt); + } } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index be68f30353715..281be201a66ce 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -862,7 +862,13 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { // Walk the generated async arguments if this is an `async fn`, otherwise walk the // normal arguments. if let IsAsync::Async { ref arguments, .. } = asyncness { - for a in arguments { add_argument(&a.arg); } + for (i, a) in arguments.iter().enumerate() { + if let Some(arg) = &a.arg { + add_argument(&arg); + } else { + add_argument(&declaration.inputs[i]); + } + } } else { for a in &declaration.inputs { add_argument(a); } } @@ -882,8 +888,11 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { let mut body = body.clone(); // Insert the generated statements into the body before attempting to // resolve names. - for a in arguments { - body.stmts.insert(0, a.stmt.clone()); + for a in arguments.iter().rev() { + if let Some(pat_stmt) = a.pat_stmt.clone() { + body.stmts.insert(0, pat_stmt); + } + body.stmts.insert(0, a.move_stmt.clone()); } self.visit_block(&body); } else { diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index a20bf91a6ad43..33b8c76bb531a 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: ` argument to replace existing function argument `: `. - pub arg: Arg, - /// `let : = __arg0;` statement to be inserted at the start of the block. - pub stmt: Stmt, + /// `__arg0: ` argument to replace existing function argument `: `. Only if + /// argument is not a simple binding. + pub arg: Option, + /// `let __arg0 = __arg0;` statement to be inserted at the start of the block. + pub move_stmt: Stmt, + /// `let = __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, } #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs index 68cd3c28676f9..f5e18e98436e6 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 d3441a2039b17..2e09235ca77b0 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -694,13 +694,21 @@ pub fn noop_visit_asyncness(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 8efe84cdf016f..a10ee17b7e79e 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -8880,11 +8880,37 @@ impl<'a> Parser<'a> { let name = format!("__arg{}", index); let ident = Ident::from_str(&name); + // Check if this is a ident pattern, if so, we can optimize and avoid adding a + // `let = __argN;` statement, instead just adding a `let = ;` + // statement. + let (ident, is_simple_pattern) = match input.pat.node { + PatKind::Ident(_, ident, _) => (ident, true), + _ => (ident, false), + }; + // Construct an argument representing `__argN: ` 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( @@ -8892,13 +8918,6 @@ impl<'a> Parser<'a> { ), span, }), - source: ArgSource::AsyncFn(input.pat.clone()), - }; - - // Construct a `let = __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 @@ -8918,10 +8937,25 @@ impl<'a> Parser<'a> { span, attrs: ThinVec::new(), source: LocalSource::AsyncFn, - }); - let stmt = Stmt { id, node: StmtKind::Local(local), span, }; + }; + + // Construct a `let = __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 }); } } } diff --git a/src/test/run-pass/async-await-drop-order-for-async-fn-parameters.rs b/src/test/run-pass/async-await-drop-order-for-async-fn-parameters.rs new file mode 100644 index 0000000000000..708c570498460 --- /dev/null +++ b/src/test/run-pass/async-await-drop-order-for-async-fn-parameters.rs @@ -0,0 +1,263 @@ +// aux-build:arc_wake.rs +// edition:2018 +// run-pass + +#![allow(unused_variables)] +#![feature(async_await, await_macro)] + +// Test that the drop order for parameters in a fn and async fn matches up. Also test that +// parameters (used or unused) are not dropped until the async fn completes execution. +// See also #54716. + +extern crate arc_wake; + +use arc_wake::ArcWake; +use std::cell::RefCell; +use std::future::Future; +use std::marker::PhantomData; +use std::sync::Arc; +use std::rc::Rc; +use std::task::Context; + +struct EmptyWaker; + +impl ArcWake for EmptyWaker { + fn wake(self: Arc) {} +} + +#[derive(Debug, Eq, PartialEq)] +enum DropOrder { + Function, + Val(&'static str), +} + +type DropOrderListPtr = Rc>>; + +struct D(&'static str, DropOrderListPtr); + +impl Drop for D { + fn drop(&mut self) { + self.1.borrow_mut().push(DropOrder::Val(self.0)); + } +} + +/// Check that unused bindings are dropped after the function is polled. +async fn foo_async(x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +fn foo_sync(x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +/// Check that underscore patterns are dropped after the function is polled. +async fn bar_async(x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +fn bar_sync(x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +/// Check that underscore patterns within more complex patterns are dropped after the function +/// is polled. +async fn baz_async((x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); +} + +fn baz_sync((x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); +} + +/// Check that underscore and unused bindings within and outwith more complex patterns are dropped +/// after the function is polled. +async fn foobar_async(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +fn foobar_sync(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +struct Foo; + +impl Foo { + /// Check that unused bindings are dropped after the method is polled. + async fn foo_async(x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn foo_sync(x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + /// Check that underscore patterns are dropped after the method is polled. + async fn bar_async(x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn bar_sync(x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + /// Check that underscore patterns within more complex patterns are dropped after the method + /// is polled. + async fn baz_async((x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn baz_sync((x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); + } + + /// Check that underscore and unused bindings within and outwith more complex patterns are + /// dropped after the method is polled. + async fn foobar_async(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn foobar_sync(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } +} + +struct Bar<'a>(PhantomData<&'a ()>); + +impl<'a> Bar<'a> { + /// Check that unused bindings are dropped after the method with self is polled. + async fn foo_async(&'a self, x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn foo_sync(&'a self, x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + /// Check that underscore patterns are dropped after the method with self is polled. + async fn bar_async(&'a self, x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn bar_sync(&'a self, x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + /// Check that underscore patterns within more complex patterns are dropped after the method + /// with self is polled. + async fn baz_async(&'a self, (x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn baz_sync(&'a self, (x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); + } + + /// Check that underscore and unused bindings within and outwith more complex patterns are + /// dropped after the method with self is polled. + async fn foobar_async(&'a self, x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + fn foobar_sync(&'a self, x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } +} + +fn assert_drop_order_after_poll>( + f: impl FnOnce(DropOrderListPtr) -> Fut, + g: impl FnOnce(DropOrderListPtr), +) { + let empty = Arc::new(EmptyWaker); + let waker = ArcWake::into_waker(empty); + let mut cx = Context::from_waker(&waker); + + let actual_order = Rc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(f(actual_order.clone())); + let _ = fut.as_mut().poll(&mut cx); + + let expected_order = Rc::new(RefCell::new(Vec::new())); + g(expected_order.clone()); + + assert_eq!(*actual_order.borrow(), *expected_order.borrow()); +} + +fn main() { + // Free functions (see doc comment on function for what it tests). + assert_drop_order_after_poll(|l| foo_async(D("x", l.clone()), D("_y", l.clone())), + |l| foo_sync(D("x", l.clone()), D("_y", l.clone()))); + assert_drop_order_after_poll(|l| bar_async(D("x", l.clone()), D("_", l.clone())), + |l| bar_sync(D("x", l.clone()), D("_", l.clone()))); + assert_drop_order_after_poll(|l| baz_async((D("x", l.clone()), D("_", l.clone()))), + |l| baz_sync((D("x", l.clone()), D("_", l.clone())))); + assert_drop_order_after_poll( + |l| { + foobar_async( + D("x", l.clone()), + (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), + D("_", l.clone()), + D("_y", l.clone()), + ) + }, + |l| { + foobar_sync( + D("x", l.clone()), + (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), + D("_", l.clone()), + D("_y", l.clone()), + ) + }, + ); + + // Methods w/out self (see doc comment on function for what it tests). + assert_drop_order_after_poll(|l| Foo::foo_async(D("x", l.clone()), D("_y", l.clone())), + |l| Foo::foo_sync(D("x", l.clone()), D("_y", l.clone()))); + assert_drop_order_after_poll(|l| Foo::bar_async(D("x", l.clone()), D("_", l.clone())), + |l| Foo::bar_sync(D("x", l.clone()), D("_", l.clone()))); + assert_drop_order_after_poll(|l| Foo::baz_async((D("x", l.clone()), D("_", l.clone()))), + |l| Foo::baz_sync((D("x", l.clone()), D("_", l.clone())))); + assert_drop_order_after_poll( + |l| { + Foo::foobar_async( + D("x", l.clone()), + (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), + D("_", l.clone()), + D("_y", l.clone()), + ) + }, + |l| { + Foo::foobar_sync( + D("x", l.clone()), + (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), + D("_", l.clone()), + D("_y", l.clone()), + ) + }, + ); + + // Methods (see doc comment on function for what it tests). + let b = Bar(Default::default()); + assert_drop_order_after_poll(|l| b.foo_async(D("x", l.clone()), D("_y", l.clone())), + |l| b.foo_sync(D("x", l.clone()), D("_y", l.clone()))); + assert_drop_order_after_poll(|l| b.bar_async(D("x", l.clone()), D("_", l.clone())), + |l| b.bar_sync(D("x", l.clone()), D("_", l.clone()))); + assert_drop_order_after_poll(|l| b.baz_async((D("x", l.clone()), D("_", l.clone()))), + |l| b.baz_sync((D("x", l.clone()), D("_", l.clone())))); + assert_drop_order_after_poll( + |l| { + b.foobar_async( + D("x", l.clone()), + (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), + D("_", l.clone()), + D("_y", l.clone()), + ) + }, + |l| { + b.foobar_sync( + D("x", l.clone()), + (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), + D("_", l.clone()), + D("_y", l.clone()), + ) + }, + ); +} diff --git a/src/test/run-pass/issue-54716.rs b/src/test/run-pass/issue-54716.rs deleted file mode 100644 index 961c412f5ecb2..0000000000000 --- a/src/test/run-pass/issue-54716.rs +++ /dev/null @@ -1,184 +0,0 @@ -// aux-build:arc_wake.rs -// edition:2018 -// run-pass - -#![allow(unused_variables)] -#![feature(async_await, await_macro)] - -extern crate arc_wake; - -use arc_wake::ArcWake; -use std::cell::RefCell; -use std::future::Future; -use std::marker::PhantomData; -use std::sync::Arc; -use std::rc::Rc; -use std::task::Context; - -struct EmptyWaker; - -impl ArcWake for EmptyWaker { - fn wake(self: Arc) {} -} - -#[derive(Debug, Eq, PartialEq)] -enum DropOrder { - Function, - Val(&'static str), -} - -type DropOrderListPtr = Rc>>; - -struct D(&'static str, DropOrderListPtr); - -impl Drop for D { - fn drop(&mut self) { - self.1.borrow_mut().push(DropOrder::Val(self.0)); - } -} - -/// Check that unused bindings are dropped after the function is polled. -async fn foo(x: D, _y: D) { - x.1.borrow_mut().push(DropOrder::Function); -} - -/// Check that underscore patterns are dropped after the function is polled. -async fn bar(x: D, _: D) { - x.1.borrow_mut().push(DropOrder::Function); -} - -/// Check that underscore patterns within more complex patterns are dropped after the function -/// is polled. -async fn baz((x, _): (D, D)) { - x.1.borrow_mut().push(DropOrder::Function); -} - -/// Check that underscore and unused bindings within and outwith more complex patterns are dropped -/// after the function is polled. -async fn foobar(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { - x.1.borrow_mut().push(DropOrder::Function); -} - -struct Foo; - -impl Foo { - /// Check that unused bindings are dropped after the method is polled. - async fn foo(x: D, _y: D) { - x.1.borrow_mut().push(DropOrder::Function); - } - - /// Check that underscore patterns are dropped after the method is polled. - async fn bar(x: D, _: D) { - x.1.borrow_mut().push(DropOrder::Function); - } - - /// Check that underscore patterns within more complex patterns are dropped after the method - /// is polled. - async fn baz((x, _): (D, D)) { - x.1.borrow_mut().push(DropOrder::Function); - } - - /// Check that underscore and unused bindings within and outwith more complex patterns are - /// dropped after the method is polled. - async fn foobar(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { - x.1.borrow_mut().push(DropOrder::Function); - } -} - -struct Bar<'a>(PhantomData<&'a ()>); - -impl<'a> Bar<'a> { - /// Check that unused bindings are dropped after the method with self is polled. - async fn foo(&'a self, x: D, _y: D) { - x.1.borrow_mut().push(DropOrder::Function); - } - - /// Check that underscore patterns are dropped after the method with self is polled. - async fn bar(&'a self, x: D, _: D) { - x.1.borrow_mut().push(DropOrder::Function); - } - - /// Check that underscore patterns within more complex patterns are dropped after the method - /// with self is polled. - async fn baz(&'a self, (x, _): (D, D)) { - x.1.borrow_mut().push(DropOrder::Function); - } - - /// Check that underscore and unused bindings within and outwith more complex patterns are - /// dropped after the method with self is polled. - async fn foobar(&'a self, x: D, (a, _, _c): (D, D, D), _: D, _y: D) { - x.1.borrow_mut().push(DropOrder::Function); - } -} - -fn assert_drop_order_after_poll>( - f: impl FnOnce(DropOrderListPtr) -> Fut, - expected_order: &[DropOrder], -) { - let empty = Arc::new(EmptyWaker); - let waker = ArcWake::into_waker(empty); - let mut cx = Context::from_waker(&waker); - - let actual_order = Rc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(f(actual_order.clone())); - let _ = fut.as_mut().poll(&mut cx); - - assert_eq!(*actual_order.borrow(), expected_order); -} - -fn main() { - use DropOrder::*; - - // At time of writing (23/04/19), the `bar` and `foobar` tests do not output the same order as - // the equivalent non-async functions. This is because the drop order of captured variables - // doesn't match the drop order of arguments in a function. - - // Free functions (see doc comment on function for what it tests). - assert_drop_order_after_poll(|l| foo(D("x", l.clone()), D("_y", l.clone())), - &[Function, Val("_y"), Val("x")]); - assert_drop_order_after_poll(|l| bar(D("x", l.clone()), D("_", l.clone())), - &[Function, Val("x"), Val("_")]); - assert_drop_order_after_poll(|l| baz((D("x", l.clone()), D("_", l.clone()))), - &[Function, Val("x"), Val("_")]); - assert_drop_order_after_poll(|l| { - foobar( - D("x", l.clone()), - (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), - D("_", l.clone()), - D("_y", l.clone()), - ) - }, &[Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_")]); - - // Methods w/out self (see doc comment on function for what it tests). - assert_drop_order_after_poll(|l| Foo::foo(D("x", l.clone()), D("_y", l.clone())), - &[Function, Val("_y"), Val("x")]); - assert_drop_order_after_poll(|l| Foo::bar(D("x", l.clone()), D("_", l.clone())), - &[Function, Val("x"), Val("_")]); - assert_drop_order_after_poll(|l| Foo::baz((D("x", l.clone()), D("_", l.clone()))), - &[Function, Val("x"), Val("_")]); - assert_drop_order_after_poll(|l| { - Foo::foobar( - D("x", l.clone()), - (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), - D("_", l.clone()), - D("_y", l.clone()), - ) - }, &[Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_")]); - - // Methods (see doc comment on function for what it tests). - let b = Bar(Default::default()); - assert_drop_order_after_poll(|l| b.foo(D("x", l.clone()), D("_y", l.clone())), - &[Function, Val("_y"), Val("x")]); - assert_drop_order_after_poll(|l| b.bar(D("x", l.clone()), D("_", l.clone())), - &[Function, Val("x"), Val("_")]); - assert_drop_order_after_poll(|l| b.baz((D("x", l.clone()), D("_", l.clone()))), - &[Function, Val("x"), Val("_")]); - assert_drop_order_after_poll(|l| { - b.foobar( - D("x", l.clone()), - (D("a", l.clone()), D("_", l.clone()), D("_c", l.clone())), - D("_", l.clone()), - D("_y", l.clone()), - ) - }, &[Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_")]); -} From f47735c3dc596c0b55c2221a18915a7b25ed1492 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 1 May 2019 14:31:27 +0100 Subject: [PATCH 2/3] Ensure that users cannot use generated arguments. This commit gensyms the generated ident for replacement arguments so that users cannot refer to them. It also ensures that levenshtein distance suggestions do not suggest gensymed identifiers. --- src/librustc_resolve/lib.rs | 23 ++++--- src/libsyntax/parse/parser.rs | 2 +- ...sync-await-drop-order-locals-are-hidden.rs | 11 ++++ ...-await-drop-order-locals-are-hidden.stderr | 15 +++++ src/test/ui/auxiliary/arc_wake.rs | 64 +++++++++++++++++++ 5 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 src/test/ui/async-await-drop-order-locals-are-hidden.rs create mode 100644 src/test/ui/async-await-drop-order-locals-are-hidden.stderr create mode 100644 src/test/ui/auxiliary/arc_wake.rs diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 281be201a66ce..dcfe00069c5bd 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -4183,7 +4183,7 @@ impl<'a> Resolver<'a> { let add_module_candidates = |module: Module<'_>, names: &mut Vec| { for (&(ident, _), resolution) in module.resolutions.borrow().iter() { if let Some(binding) = resolution.borrow().binding { - if filter_fn(binding.def()) { + if !ident.name.is_gensymed() && filter_fn(binding.def()) { names.push(TypoSuggestion { candidate: ident.name, article: binding.def().article(), @@ -4201,7 +4201,7 @@ impl<'a> Resolver<'a> { for rib in self.ribs[ns].iter().rev() { // Locals and type parameters for (ident, def) in &rib.bindings { - if filter_fn(*def) { + if !ident.name.is_gensymed() && filter_fn(*def) { names.push(TypoSuggestion { candidate: ident.name, article: def.article(), @@ -4228,7 +4228,7 @@ impl<'a> Resolver<'a> { index: CRATE_DEF_INDEX, }); - if filter_fn(crate_mod) { + if !ident.name.is_gensymed() && filter_fn(crate_mod) { Some(TypoSuggestion { candidate: ident.name, article: "a", @@ -4251,13 +4251,16 @@ impl<'a> Resolver<'a> { // Add primitive types to the mix if filter_fn(Def::PrimTy(Bool)) { names.extend( - self.primitive_type_table.primitive_types.iter().map(|(name, _)| { - TypoSuggestion { - candidate: *name, - article: "a", - kind: "primitive type", - } - }) + self.primitive_type_table.primitive_types + .iter() + .filter(|(name, _)| !name.is_gensymed()) + .map(|(name, _)| { + TypoSuggestion { + candidate: *name, + article: "a", + kind: "primitive type", + } + }) ) } } else { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a10ee17b7e79e..8d95b3900014d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -8878,7 +8878,7 @@ 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 = __argN;` statement, instead just adding a `let = ;` diff --git a/src/test/ui/async-await-drop-order-locals-are-hidden.rs b/src/test/ui/async-await-drop-order-locals-are-hidden.rs new file mode 100644 index 0000000000000..10dc5e27f6f9f --- /dev/null +++ b/src/test/ui/async-await-drop-order-locals-are-hidden.rs @@ -0,0 +1,11 @@ +// edition:2018 + +#![allow(unused_variables)] +#![feature(async_await)] + +async fn foobar_async(x: u32, (a, _, _c): (u32, u32, u32), _: u32, _y: u32) { + assert_eq!(__arg1, (1, 2, 3)); //~ ERROR cannot find value `__arg1` in this scope [E0425] + assert_eq!(__arg2, 4); //~ ERROR cannot find value `__arg2` in this scope [E0425] +} + +fn main() {} diff --git a/src/test/ui/async-await-drop-order-locals-are-hidden.stderr b/src/test/ui/async-await-drop-order-locals-are-hidden.stderr new file mode 100644 index 0000000000000..b988b85d63d1b --- /dev/null +++ b/src/test/ui/async-await-drop-order-locals-are-hidden.stderr @@ -0,0 +1,15 @@ +error[E0425]: cannot find value `__arg1` in this scope + --> $DIR/async-await-drop-order-locals-are-hidden.rs:7:16 + | +LL | assert_eq!(__arg1, (1, 2, 3)); + | ^^^^^^ not found in this scope + +error[E0425]: cannot find value `__arg2` in this scope + --> $DIR/async-await-drop-order-locals-are-hidden.rs:8:16 + | +LL | assert_eq!(__arg2, 4); + | ^^^^^^ not found in this scope + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0425`. diff --git a/src/test/ui/auxiliary/arc_wake.rs b/src/test/ui/auxiliary/arc_wake.rs new file mode 100644 index 0000000000000..c21886f26f467 --- /dev/null +++ b/src/test/ui/auxiliary/arc_wake.rs @@ -0,0 +1,64 @@ +// edition:2018 + +use std::sync::Arc; +use std::task::{ + Waker, RawWaker, RawWakerVTable, +}; + +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable::new( + clone_arc_raw::<$ty>, + wake_arc_raw::<$ty>, + wake_by_ref_arc_raw::<$ty>, + drop_arc_raw::<$ty>, + ) + }; +} + +pub trait ArcWake { + fn wake(self: Arc); + + fn wake_by_ref(arc_self: &Arc) { + arc_self.clone().wake() + } + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const (); + + unsafe { + Waker::from_raw(RawWaker::new(ptr, waker_vtable!(Self))) + } + } +} + +unsafe fn increase_refcount(data: *const ()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} + +unsafe fn clone_arc_raw(data: *const ()) -> RawWaker { + increase_refcount::(data); + RawWaker::new(data, waker_vtable!(T)) +} + +unsafe fn drop_arc_raw(data: *const ()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const ()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(arc); +} + +unsafe fn wake_by_ref_arc_raw(data: *const ()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake_by_ref(&arc); + let _ = Arc::into_raw(arc); +} From 1fedb0a20bfe03e1bf7d5c2576806c761b4e3963 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 1 May 2019 15:03:42 +0100 Subject: [PATCH 3/3] Unify tests under async-await directory. --- src/test/ui/{ => async-await}/auxiliary/arc_wake.rs | 0 .../async-await/drop-order-for-async-fn-parameters.rs} | 0 .../drop-order-locals-are-hidden.rs} | 0 .../drop-order-locals-are-hidden.stderr} | 4 ++-- 4 files changed, 2 insertions(+), 2 deletions(-) rename src/test/ui/{ => async-await}/auxiliary/arc_wake.rs (100%) rename src/test/{run-pass/async-await-drop-order-for-async-fn-parameters.rs => ui/async-await/drop-order-for-async-fn-parameters.rs} (100%) rename src/test/ui/{async-await-drop-order-locals-are-hidden.rs => async-await/drop-order-locals-are-hidden.rs} (100%) rename src/test/ui/{async-await-drop-order-locals-are-hidden.stderr => async-await/drop-order-locals-are-hidden.stderr} (77%) diff --git a/src/test/ui/auxiliary/arc_wake.rs b/src/test/ui/async-await/auxiliary/arc_wake.rs similarity index 100% rename from src/test/ui/auxiliary/arc_wake.rs rename to src/test/ui/async-await/auxiliary/arc_wake.rs diff --git a/src/test/run-pass/async-await-drop-order-for-async-fn-parameters.rs b/src/test/ui/async-await/drop-order-for-async-fn-parameters.rs similarity index 100% rename from src/test/run-pass/async-await-drop-order-for-async-fn-parameters.rs rename to src/test/ui/async-await/drop-order-for-async-fn-parameters.rs diff --git a/src/test/ui/async-await-drop-order-locals-are-hidden.rs b/src/test/ui/async-await/drop-order-locals-are-hidden.rs similarity index 100% rename from src/test/ui/async-await-drop-order-locals-are-hidden.rs rename to src/test/ui/async-await/drop-order-locals-are-hidden.rs diff --git a/src/test/ui/async-await-drop-order-locals-are-hidden.stderr b/src/test/ui/async-await/drop-order-locals-are-hidden.stderr similarity index 77% rename from src/test/ui/async-await-drop-order-locals-are-hidden.stderr rename to src/test/ui/async-await/drop-order-locals-are-hidden.stderr index b988b85d63d1b..ca0da6b7c962a 100644 --- a/src/test/ui/async-await-drop-order-locals-are-hidden.stderr +++ b/src/test/ui/async-await/drop-order-locals-are-hidden.stderr @@ -1,11 +1,11 @@ error[E0425]: cannot find value `__arg1` in this scope - --> $DIR/async-await-drop-order-locals-are-hidden.rs:7:16 + --> $DIR/drop-order-locals-are-hidden.rs:7:16 | LL | assert_eq!(__arg1, (1, 2, 3)); | ^^^^^^ not found in this scope error[E0425]: cannot find value `__arg2` in this scope - --> $DIR/async-await-drop-order-locals-are-hidden.rs:8:16 + --> $DIR/drop-order-locals-are-hidden.rs:8:16 | LL | assert_eq!(__arg2, 4); | ^^^^^^ not found in this scope