From 8b57be1bb3be3be5ac431ed9a0a310d2023b1c9d Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 7 Mar 2019 23:15:57 +0100 Subject: [PATCH 01/10] Add test for drop order in async functions. This tests that async functions drop parameters in the same order as regular functions. --- src/test/run-pass/issue-54716.rs | 89 ++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 src/test/run-pass/issue-54716.rs diff --git a/src/test/run-pass/issue-54716.rs b/src/test/run-pass/issue-54716.rs new file mode 100644 index 0000000000000..7f64c6e120eb4 --- /dev/null +++ b/src/test/run-pass/issue-54716.rs @@ -0,0 +1,89 @@ +// aux-build:arc_wake.rs +// edition:2018 +// run-pass + +#![allow(unused_variables)] +#![feature(async_await, await_macro, futures_api)] + +extern crate arc_wake; + +use arc_wake::ArcWake; +use std::cell::RefCell; +use std::future::Future; +use std::sync::Arc; +use std::task::Context; + +struct EmptyWaker; + +impl ArcWake for EmptyWaker { + fn wake(self: Arc) {} +} + +#[derive(Debug, Eq, PartialEq)] +enum DropOrder { + Function, + Val(&'static str), +} + +struct D(&'static str, Arc>>); + +impl Drop for D { + fn drop(&mut self) { + self.1.borrow_mut().push(DropOrder::Val(self.0)); + } +} + +async fn foo(x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +async fn bar(x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +async fn baz((x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); +} + +async fn foobar(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); +} + +fn main() { + let empty = Arc::new(EmptyWaker); + let waker = ArcWake::into_waker(empty); + let mut cx = Context::from_waker(&waker); + + use DropOrder::*; + + // Currently, 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. + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(foo(D("x", af.clone()), D("_y", af.clone()))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("_y"), Val("x")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(bar(D("x", af.clone()), D("_", af.clone()))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(baz((D("x", af.clone()), D("_", af.clone())))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(foobar( + D("x", af.clone()), + (D("a", af.clone()), D("_", af.clone()), D("_c", af.clone())), + D("_", af.clone()), + D("_y", af.clone()), + )); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[ + Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), + ]); +} From 41c6bb1096abb026d496c2136bb18c001eca46fe Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 12 Mar 2019 16:53:33 +0100 Subject: [PATCH 02/10] Introduce `LocalSource` into the AST. This will be used to keep track of the origin of a local in the AST. In particular, it will be used by `async fn` lowering for the locals in `let : = __arg0;` statements. --- src/librustc/hir/lowering.rs | 9 ++++++++- src/librustc/hir/mod.rs | 11 +++++++++++ src/librustc/ich/impls_hir.rs | 1 - src/librustc_mir/hair/pattern/check_match.rs | 1 + src/libsyntax/ast.rs | 11 +++++++++++ src/libsyntax/ext/build.rs | 3 +++ src/libsyntax/mut_visit.rs | 10 +++++++++- src/libsyntax/parse/parser.rs | 3 ++- src/libsyntax_ext/deriving/debug.rs | 1 + 9 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 42ad571cf2832..e4cf3cfc63db9 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2224,10 +2224,17 @@ impl<'a> LoweringContext<'a> { init: l.init.as_ref().map(|e| P(self.lower_expr(e))), span: l.span, attrs: l.attrs.clone(), - source: hir::LocalSource::Normal, + source: self.lower_local_source(l.source), }, ids) } + fn lower_local_source(&mut self, ls: LocalSource) -> hir::LocalSource { + match ls { + LocalSource::Normal => hir::LocalSource::Normal, + LocalSource::AsyncFn => hir::LocalSource::AsyncFn, + } + } + fn lower_mutability(&mut self, m: Mutability) -> hir::Mutability { match m { Mutability::Mutable => hir::MutMutable, diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 7ed8c08c92337..e873663a613b4 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1583,6 +1583,17 @@ pub enum LocalSource { Normal, /// A desugared `for _ in _ { .. }` loop. ForLoopDesugar, + /// When lowering async functions, we create locals within the `async move` so that + /// all arguments are dropped after the future is polled. + /// + /// ```ignore (pseudo-Rust) + /// async fn foo( @ x: Type) { + /// async move { + /// let = x; + /// } + /// } + /// ``` + AsyncFn, } /// Hints at the original code for a `match _ { .. }`. diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 9491a073b8f8c..65795d2b136b4 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -435,4 +435,3 @@ impl<'hir> HashStable> for attr::OptimizeAttr { mem::discriminant(self).hash_stable(hcx, hasher); } } - diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 7ded973701edc..c4f07d9c544d2 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -76,6 +76,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> { self.check_irrefutable(&loc.pat, match loc.source { hir::LocalSource::Normal => "local binding", hir::LocalSource::ForLoopDesugar => "`for` loop binding", + hir::LocalSource::AsyncFn => "async fn binding", }); // Check legality of move bindings and `@` patterns. diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 0668730b3ef01..2fe0ebcdd28e5 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -888,6 +888,17 @@ pub struct Local { pub id: NodeId, pub span: Span, pub attrs: ThinVec, + /// Origin of this local variable. + pub source: LocalSource, +} + +#[derive(Clone, Copy, RustcEncodable, RustcDecodable, Debug)] +pub enum LocalSource { + /// Local was parsed from source. + Normal, + /// Within `ast::IsAsync::Async`, a local is generated that will contain the moved arguments + /// of an `async fn`. + AsyncFn, } /// An arm of a 'match'. diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 614967bdeb447..029c45eaa7247 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -526,6 +526,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { id: ast::DUMMY_NODE_ID, span: sp, attrs: ThinVec::new(), + source: ast::LocalSource::Normal, }); ast::Stmt { id: ast::DUMMY_NODE_ID, @@ -554,6 +555,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { id: ast::DUMMY_NODE_ID, span: sp, attrs: ThinVec::new(), + source: ast::LocalSource::Normal, }); ast::Stmt { id: ast::DUMMY_NODE_ID, @@ -571,6 +573,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { id: ast::DUMMY_NODE_ID, span, attrs: ThinVec::new(), + source: ast::LocalSource::Normal, }); ast::Stmt { id: ast::DUMMY_NODE_ID, diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index 784d0049ac51f..87826ccc89171 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -208,6 +208,10 @@ pub trait MutVisitor: Sized { noop_visit_local(l, self); } + fn visit_local_source(&mut self, l: &mut LocalSource) { + noop_visit_local_source(l, self); + } + fn visit_mac(&mut self, _mac: &mut Mac) { panic!("visit_mac disabled by default"); // N.B., see note about macros above. If you really want a visitor that @@ -511,13 +515,17 @@ pub fn noop_visit_parenthesized_parameter_data(args: &mut Parenth } pub fn noop_visit_local(local: &mut P, vis: &mut T) { - let Local { id, pat, ty, init, span, attrs } = local.deref_mut(); + let Local { id, pat, ty, init, span, attrs, source } = local.deref_mut(); vis.visit_id(id); vis.visit_pat(pat); visit_opt(ty, |ty| vis.visit_ty(ty)); visit_opt(init, |init| vis.visit_expr(init)); vis.visit_span(span); visit_thin_attrs(attrs, vis); + vis.visit_local_source(source); +} + +pub fn noop_visit_local_source(_local_source: &mut LocalSource, _vis: &mut T) { } pub fn noop_visit_attribute(attr: &mut Attribute, vis: &mut T) { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 8feab373e7102..8620627765fb6 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -14,7 +14,7 @@ use crate::ast::{GenericParam, GenericParamKind}; use crate::ast::GenericArg; use crate::ast::{Ident, ImplItem, IsAsync, IsAuto, Item, ItemKind}; use crate::ast::{Label, Lifetime, Lit, LitKind}; -use crate::ast::Local; +use crate::ast::{Local, LocalSource}; use crate::ast::MacStmtStyle; use crate::ast::{Mac, Mac_, MacDelimiter}; use crate::ast::{MutTy, Mutability}; @@ -5029,6 +5029,7 @@ impl<'a> Parser<'a> { id: ast::DUMMY_NODE_ID, span: lo.to(hi), attrs, + source: LocalSource::Normal, })) } diff --git a/src/libsyntax_ext/deriving/debug.rs b/src/libsyntax_ext/deriving/debug.rs index 7c47c6ff79ac1..2fc1fc9140dc3 100644 --- a/src/libsyntax_ext/deriving/debug.rs +++ b/src/libsyntax_ext/deriving/debug.rs @@ -128,6 +128,7 @@ fn stmt_let_undescore(cx: &mut ExtCtxt<'_>, sp: Span, expr: P) -> ast id: ast::DUMMY_NODE_ID, span: sp, attrs: ThinVec::new(), + source: ast::LocalSource::Normal, }); ast::Stmt { id: ast::DUMMY_NODE_ID, From 879abb1641d97be798010f52a875b9fc83881323 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 12 Mar 2019 17:00:20 +0100 Subject: [PATCH 03/10] Add `AsyncArgument` to AST. This commit adds an `AsyncArgument` struct to the AST that contains the generated argument and statement that will be used in HIR lowering, name resolution and def collection. --- src/librustc/hir/intravisit.rs | 6 +-- src/librustc/hir/lowering.rs | 22 ++++---- src/librustc/hir/map/def_collector.rs | 7 +-- src/librustc/lint/context.rs | 16 ++++++ src/librustc_passes/ast_validation.rs | 6 +-- src/librustc_resolve/lib.rs | 10 ++-- src/librustc_save_analysis/sig.rs | 2 +- src/libsyntax/ast.rs | 29 ++++++++--- src/libsyntax/ext/placeholders.rs | 20 ++++++-- src/libsyntax/mut_visit.rs | 10 +++- src/libsyntax/parse/parser.rs | 73 +++++++++++++++++++++++++-- src/libsyntax/print/pprust.rs | 23 ++++----- 12 files changed, 171 insertions(+), 53 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 007eaef74a7ad..b653093c1f8a4 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -58,10 +58,10 @@ impl<'a> FnKind<'a> { } } - pub fn header(&self) -> Option { + pub fn header(&self) -> Option<&FnHeader> { match *self { - FnKind::ItemFn(_, _, header, _, _) => Some(header), - FnKind::Method(_, sig, _, _) => Some(sig.header), + FnKind::ItemFn(_, _, ref header, _, _) => Some(header), + FnKind::Method(_, ref sig, _, _) => Some(&sig.header), FnKind::Closure(_) => None, } } diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index e4cf3cfc63db9..93a327588fef6 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3000,13 +3000,13 @@ impl<'a> LoweringContext<'a> { fn lower_async_body( &mut self, decl: &FnDecl, - asyncness: IsAsync, + asyncness: &IsAsync, body: &Block, ) -> hir::BodyId { self.lower_body(Some(decl), |this| { if let IsAsync::Async { closure_id, .. } = asyncness { let async_expr = this.make_async_expr( - CaptureBy::Value, closure_id, None, + CaptureBy::Value, *closure_id, None, |this| { let body = this.lower_block(body, false); this.expr_block(body, ThinVec::new()) @@ -3067,14 +3067,14 @@ impl<'a> LoweringContext<'a> { value ) } - ItemKind::Fn(ref decl, header, ref generics, ref body) => { + ItemKind::Fn(ref decl, ref header, ref generics, ref body) => { let fn_def_id = self.resolver.definitions().local_def_id(id); self.with_new_scopes(|this| { // Note: we don't need to change the return type from `T` to // `impl Future` here because lower_body // only cares about the input argument patterns in the function // declaration (decl), not the return types. - let body_id = this.lower_async_body(decl, header.asyncness.node, body); + let body_id = this.lower_async_body(decl, &header.asyncness.node, body); let (generics, fn_decl) = this.add_in_band_defs( generics, @@ -3565,7 +3565,7 @@ impl<'a> LoweringContext<'a> { ) } ImplItemKind::Method(ref sig, ref body) => { - let body_id = self.lower_async_body(&sig.decl, sig.header.asyncness.node, body); + let body_id = self.lower_async_body(&sig.decl, &sig.header.asyncness.node, body); let impl_trait_return_allow = !self.is_in_trait_impl; let (generics, sig) = self.lower_method_sig( &i.generics, @@ -3767,7 +3767,7 @@ impl<'a> LoweringContext<'a> { impl_trait_return_allow: bool, is_async: Option, ) -> (hir::Generics, hir::MethodSig) { - let header = self.lower_fn_header(sig.header); + let header = self.lower_fn_header(&sig.header); let (generics, decl) = self.add_in_band_defs( generics, fn_def_id, @@ -3789,10 +3789,10 @@ impl<'a> LoweringContext<'a> { } } - fn lower_fn_header(&mut self, h: FnHeader) -> hir::FnHeader { + fn lower_fn_header(&mut self, h: &FnHeader) -> hir::FnHeader { hir::FnHeader { unsafety: self.lower_unsafety(h.unsafety), - asyncness: self.lower_asyncness(h.asyncness.node), + asyncness: self.lower_asyncness(&h.asyncness.node), constness: self.lower_constness(h.constness), abi: h.abi, } @@ -3812,7 +3812,7 @@ impl<'a> LoweringContext<'a> { } } - fn lower_asyncness(&mut self, a: IsAsync) -> hir::IsAsync { + fn lower_asyncness(&mut self, a: &IsAsync) -> hir::IsAsync { match a { IsAsync::Async { .. } => hir::IsAsync::Async, IsAsync::NotAsync => hir::IsAsync::NotAsync, @@ -4125,7 +4125,7 @@ impl<'a> LoweringContext<'a> { }) } ExprKind::Closure( - capture_clause, asyncness, movability, ref decl, ref body, fn_decl_span + capture_clause, ref asyncness, movability, ref decl, ref body, fn_decl_span ) => { if let IsAsync::Async { closure_id, .. } = asyncness { let outer_decl = FnDecl { @@ -4163,7 +4163,7 @@ impl<'a> LoweringContext<'a> { Some(&**ty) } else { None }; let async_body = this.make_async_expr( - capture_clause, closure_id, async_ret_ty, + capture_clause, *closure_id, async_ret_ty, |this| { this.with_new_scopes(|this| this.lower_expr(body)) }); diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index 1a3bbc5ecc49e..6ba2a65703bdc 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -68,7 +68,7 @@ impl<'a> DefCollector<'a> { id: NodeId, name: Name, span: Span, - header: &FnHeader, + header: &'a FnHeader, generics: &'a Generics, decl: &'a FnDecl, body: &'a Block, @@ -77,6 +77,7 @@ impl<'a> DefCollector<'a> { IsAsync::Async { closure_id, return_impl_trait_id, + .. } => (closure_id, return_impl_trait_id), _ => unreachable!(), }; @@ -290,7 +291,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { match expr.node { ExprKind::Mac(..) => return self.visit_macro_invoc(expr.id), - ExprKind::Closure(_, asyncness, ..) => { + ExprKind::Closure(_, ref asyncness, ..) => { let closure_def = self.create_def(expr.id, DefPathData::ClosureExpr, REGULAR_SPACE, @@ -300,7 +301,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { // Async closures desugar to closures inside of closures, so // we must create two defs. if let IsAsync::Async { closure_id, .. } = asyncness { - let async_def = self.create_def(closure_id, + let async_def = self.create_def(*closure_id, DefPathData::ClosureExpr, REGULAR_SPACE, expr.span); diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 4b615345a26f3..709e5c4cce4ef 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -1328,6 +1328,22 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> run_early_pass!(self, check_mac, mac); } + + fn visit_fn_header(&mut self, header: &'a ast::FnHeader) { + // Unlike in HIR lowering and name resolution, the `AsyncArgument` statements are not added + // to the function body and the arguments do not replace those in the declaration. They are + // still visited manually here so that buffered lints can be emitted. + if let ast::IsAsync::Async { ref arguments, .. } = header.asyncness.node { + for a in arguments { + // Visit the argument.. + self.visit_pat(&a.arg.pat); + self.visit_ty(&a.arg.ty); + + // ..and the statement. + self.visit_stmt(&a.stmt); + } + } + } } struct LateLintPassObjects<'a> { diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index a9a604cad8bcf..9dd8a7050fd28 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -222,7 +222,7 @@ impl<'a> AstValidator<'a> { } } - fn check_trait_fn_not_async(&self, span: Span, asyncness: IsAsync) { + fn check_trait_fn_not_async(&self, span: Span, asyncness: &IsAsync) { if asyncness.is_async() { struct_span_err!(self.session, span, E0706, "trait fns cannot be declared `async`").emit() @@ -570,7 +570,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.invalid_visibility(&impl_item.vis, None); if let ImplItemKind::Method(ref sig, _) = impl_item.node { self.check_trait_fn_not_const(sig.header.constness); - self.check_trait_fn_not_async(impl_item.span, sig.header.asyncness.node); + self.check_trait_fn_not_async(impl_item.span, &sig.header.asyncness.node); } } } @@ -642,7 +642,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.no_questions_in_bounds(bounds, "supertraits", true); for trait_item in trait_items { if let TraitItemKind::Method(ref sig, ref block) = trait_item.node { - self.check_trait_fn_not_async(trait_item.span, sig.header.asyncness.node); + self.check_trait_fn_not_async(trait_item.span, &sig.header.asyncness.node); self.check_trait_fn_not_const(sig.header.constness); if block.is_none() { self.check_decl_no_pat(&sig.decl, |span, mut_ident| { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 8df83120738c1..de2f04035e41b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -817,13 +817,13 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { debug!("(resolving function) entering function"); let (rib_kind, asyncness) = match function_kind { FnKind::ItemFn(_, ref header, ..) => - (FnItemRibKind, header.asyncness.node), + (FnItemRibKind, &header.asyncness.node), FnKind::Method(_, ref sig, _, _) => - (TraitOrImplItemRibKind, sig.header.asyncness.node), + (TraitOrImplItemRibKind, &sig.header.asyncness.node), FnKind::Closure(_) => // Async closures aren't resolved through `visit_fn`-- they're // processed separately - (ClosureRibKind(node_id), IsAsync::NotAsync), + (ClosureRibKind(node_id), &IsAsync::NotAsync), }; // Create a value rib for the function. @@ -836,16 +836,14 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { let mut bindings_list = FxHashMap::default(); for argument in &declaration.inputs { self.resolve_pattern(&argument.pat, PatternSource::FnParam, &mut bindings_list); - self.visit_ty(&argument.ty); - debug!("(resolving function) recorded argument"); } visit::walk_fn_ret_ty(self, &declaration.output); // Resolve the function body, potentially inside the body of an async closure if let IsAsync::Async { closure_id, .. } = asyncness { - let rib_kind = ClosureRibKind(closure_id); + let rib_kind = ClosureRibKind(*closure_id); self.ribs[ValueNS].push(Rib::new(rib_kind)); self.label_ribs.push(Rib::new(rib_kind)); } diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index 76034f32c741c..f9c1f02236dbc 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/src/librustc_save_analysis/sig.rs @@ -374,7 +374,7 @@ impl Sig for ast::Item { Ok(extend_sig(ty, text, defs, vec![])) } - ast::ItemKind::Fn(ref decl, header, ref generics, _) => { + ast::ItemKind::Fn(ref decl, ref header, ref generics, _) => { let mut text = String::new(); if header.constness.node == ast::Constness::Const { text.push_str("const "); diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 2fe0ebcdd28e5..e6669e0d6edf2 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1849,18 +1849,35 @@ pub enum Unsafety { Normal, } -#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)] +#[derive(Clone, RustcEncodable, RustcDecodable, Debug)] +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, +} + +#[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub enum IsAsync { Async { closure_id: NodeId, return_impl_trait_id: NodeId, + /// This field stores the arguments and statements that are used in HIR lowering to + /// ensure that `async fn` arguments are dropped at the correct time. + /// + /// The argument and statements here are generated at parse time as they are required in + /// both the hir lowering, def collection and name resolution and this stops them needing + /// to be created in each place. + arguments: Vec, }, NotAsync, } impl IsAsync { - pub fn is_async(self) -> bool { - if let IsAsync::Async { .. } = self { + pub fn is_async(&self) -> bool { + if let IsAsync::Async { .. } = *self { true } else { false @@ -1868,12 +1885,12 @@ impl IsAsync { } /// In ths case this is an `async` return, the `NodeId` for the generated `impl Trait` item. - pub fn opt_return_id(self) -> Option { + pub fn opt_return_id(&self) -> Option { match self { IsAsync::Async { return_impl_trait_id, .. - } => Some(return_impl_trait_id), + } => Some(*return_impl_trait_id), IsAsync::NotAsync => None, } } @@ -2213,7 +2230,7 @@ impl Item { /// /// All the information between the visibility and the name of the function is /// included in this struct (e.g., `async unsafe fn` or `const extern "C" fn`). -#[derive(Clone, Copy, RustcEncodable, RustcDecodable, Debug)] +#[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct FnHeader { pub unsafety: Unsafety, pub asyncness: Spanned, diff --git a/src/libsyntax/ext/placeholders.rs b/src/libsyntax/ext/placeholders.rs index 3e60dd81a3bc8..68cd3c28676f9 100644 --- a/src/libsyntax/ext/placeholders.rs +++ b/src/libsyntax/ext/placeholders.rs @@ -102,6 +102,13 @@ impl<'a, 'b> PlaceholderExpander<'a, 'b> { fn remove(&mut self, id: ast::NodeId) -> AstFragment { self.expanded_fragments.remove(&id).unwrap() } + + fn next_id(&mut self, id: &mut ast::NodeId) { + if self.monotonic { + assert_eq!(*id, ast::DUMMY_NODE_ID); + *id = self.cx.resolver.next_node_id() + } + } } impl<'a, 'b> MutVisitor for PlaceholderExpander<'a, 'b> { @@ -183,9 +190,16 @@ impl<'a, 'b> MutVisitor for PlaceholderExpander<'a, 'b> { noop_visit_block(block, self); for stmt in block.stmts.iter_mut() { - if self.monotonic { - assert_eq!(stmt.id, ast::DUMMY_NODE_ID); - stmt.id = self.cx.resolver.next_node_id(); + self.next_id(&mut stmt.id); + } + } + + fn visit_asyncness(&mut self, a: &mut ast::IsAsync) { + noop_visit_asyncness(a, self); + + if let ast::IsAsync::Async { ref mut arguments, .. } = a { + for argument in arguments.iter_mut() { + self.next_id(&mut argument.stmt.id); } } } diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index 87826ccc89171..bb9116e678efc 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -679,9 +679,17 @@ pub fn noop_visit_interpolated(nt: &mut token::Nonterminal, vis: pub fn noop_visit_asyncness(asyncness: &mut IsAsync, vis: &mut T) { match asyncness { - IsAsync::Async { closure_id, return_impl_trait_id } => { + 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() { + vis.visit_ident(ident); + vis.visit_arg(arg); + 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 8620627765fb6..26ede5a1057db 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1,4 +1,4 @@ -use crate::ast::{AngleBracketedArgs, ParenthesizedArgs, AttrStyle, BareFnTy}; +use crate::ast::{AngleBracketedArgs, AsyncArgument, ParenthesizedArgs, AttrStyle, BareFnTy}; use crate::ast::{GenericBound, TraitBoundModifier}; use crate::ast::Unsafety; use crate::ast::{Mod, AnonConst, Arg, Arm, Guard, Attribute, BindingMode, TraitItemKind}; @@ -1517,6 +1517,7 @@ impl<'a> Parser<'a> { IsAsync::Async { closure_id: ast::DUMMY_NODE_ID, return_impl_trait_id: ast::DUMMY_NODE_ID, + arguments: Vec::new(), } } else { IsAsync::NotAsync @@ -1575,7 +1576,7 @@ impl<'a> Parser<'a> { // trait item macro. (keywords::Invalid.ident(), ast::TraitItemKind::Macro(mac), ast::Generics::default()) } else { - let (constness, unsafety, asyncness, abi) = self.parse_fn_front_matter()?; + let (constness, unsafety, mut asyncness, abi) = self.parse_fn_front_matter()?; let ident = self.parse_ident()?; let mut generics = self.parse_generics()?; @@ -1589,6 +1590,7 @@ impl<'a> Parser<'a> { p.parse_arg_general(p.span.rust_2018(), true, false) })?; generics.where_clause = self.parse_where_clause()?; + self.construct_async_arguments(&mut asyncness, &d); let sig = ast::MethodSig { header: FnHeader { @@ -6567,7 +6569,7 @@ impl<'a> Parser<'a> { /// Parses an item-position function declaration. fn parse_item_fn(&mut self, unsafety: Unsafety, - asyncness: Spanned, + mut asyncness: Spanned, constness: Spanned, abi: Abi) -> PResult<'a, ItemInfo> { @@ -6576,6 +6578,7 @@ impl<'a> Parser<'a> { let decl = self.parse_fn_decl(allow_c_variadic)?; generics.where_clause = self.parse_where_clause()?; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; + self.construct_async_arguments(&mut asyncness, &decl); let header = FnHeader { unsafety, asyncness, constness, abi }; Ok((ident, ItemKind::Fn(decl, header, generics, body), Some(inner_attrs))) } @@ -6751,11 +6754,12 @@ impl<'a> Parser<'a> { Ok((keywords::Invalid.ident(), vec![], ast::Generics::default(), ast::ImplItemKind::Macro(mac))) } else { - let (constness, unsafety, asyncness, abi) = self.parse_fn_front_matter()?; + let (constness, unsafety, mut asyncness, abi) = self.parse_fn_front_matter()?; let ident = self.parse_ident()?; let mut generics = self.parse_generics()?; let decl = self.parse_fn_decl_with_self(|p| p.parse_arg())?; generics.where_clause = self.parse_where_clause()?; + self.construct_async_arguments(&mut asyncness, &decl); *at_end = true; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; let header = ast::FnHeader { abi, unsafety, constness, asyncness }; @@ -8177,6 +8181,7 @@ impl<'a> Parser<'a> { respan(async_span, IsAsync::Async { closure_id: ast::DUMMY_NODE_ID, return_impl_trait_id: ast::DUMMY_NODE_ID, + arguments: Vec::new(), }), respan(fn_span, Constness::NotConst), Abi::Rust)?; @@ -8822,6 +8827,66 @@ impl<'a> Parser<'a> { } } } + + /// When lowering a `async fn` to the HIR, we need to move all of the arguments of the function + /// into the generated closure so that they are dropped when the future is polled and not when + /// it is created. + /// + /// The arguments of the function are replaced in HIR lowering with the arguments created by + /// this function and the statements created here are inserted at the top of the closure body. + fn construct_async_arguments(&mut self, asyncness: &mut Spanned, decl: &FnDecl) { + if let IsAsync::Async { ref mut arguments, .. } = asyncness.node { + for (index, input) in decl.inputs.iter().enumerate() { + let id = ast::DUMMY_NODE_ID; + let span = input.pat.span; + + // Construct a name for our temporary argument. + let name = format!("__arg{}", index); + let ident = Ident::from_str(&name); + + // Construct an argument representing `__argN: ` to replace the argument of the + // async function. + let arg = Arg { + ty: input.ty.clone(), + id, + pat: P(Pat { + id, + node: PatKind::Ident( + BindingMode::ByValue(Mutability::Immutable), ident, None, + ), + span, + }), + }; + + // Construct a `let : = __argN;` statement to insert at the top of the + // async closure. + let local = P(Local { + pat: input.pat.clone(), + ty: Some(P(Ty { + id, + node: input.ty.node.clone(), + span: input.ty.span, + })), + init: Some(P(Expr { + id, + node: ExprKind::Path(None, ast::Path { + span, + segments: vec![PathSegment { ident, id, args: None }], + }), + span, + attrs: ThinVec::new(), + })), + id, + span, + attrs: ThinVec::new(), + source: LocalSource::AsyncFn, + }); + let stmt = Stmt { id, node: StmtKind::Local(local), span, }; + + arguments.push(AsyncArgument { ident, arg, stmt }); + } + } + } } pub fn emit_unclosed_delims(unclosed_delims: &mut Vec, handler: &errors::Handler) { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index ca05ff71c9433..d440a02f2fda6 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -372,7 +372,7 @@ pub fn vis_to_string(v: &ast::Visibility) -> String { } pub fn fun_to_string(decl: &ast::FnDecl, - header: ast::FnHeader, + header: &ast::FnHeader, name: ast::Ident, generics: &ast::Generics) -> String { @@ -1133,7 +1133,7 @@ impl<'a> State<'a> { match item.node { ast::ForeignItemKind::Fn(ref decl, ref generics) => { self.head("")?; - self.print_fn(decl, ast::FnHeader::default(), + self.print_fn(decl, &ast::FnHeader::default(), Some(item.ident), generics, &item.vis)?; self.end()?; // end head-ibox @@ -1263,7 +1263,7 @@ impl<'a> State<'a> { self.s.word(";")?; self.end()?; // end the outer cbox } - ast::ItemKind::Fn(ref decl, header, ref param_names, ref body) => { + ast::ItemKind::Fn(ref decl, ref header, ref param_names, ref body) => { self.head("")?; self.print_fn( decl, @@ -1615,7 +1615,7 @@ impl<'a> State<'a> { vis: &ast::Visibility) -> io::Result<()> { self.print_fn(&m.decl, - m.header, + &m.header, Some(ident), &generics, vis) @@ -2213,7 +2213,7 @@ impl<'a> State<'a> { self.bclose_(expr.span, INDENT_UNIT)?; } ast::ExprKind::Closure( - capture_clause, asyncness, movability, ref decl, ref body, _) => { + capture_clause, ref asyncness, movability, ref decl, ref body, _) => { self.print_movability(movability)?; self.print_asyncness(asyncness)?; self.print_capture_clause(capture_clause)?; @@ -2798,7 +2798,7 @@ impl<'a> State<'a> { pub fn print_fn(&mut self, decl: &ast::FnDecl, - header: ast::FnHeader, + header: &ast::FnHeader, name: Option, generics: &ast::Generics, vis: &ast::Visibility) -> io::Result<()> { @@ -2853,8 +2853,7 @@ impl<'a> State<'a> { } } - pub fn print_asyncness(&mut self, asyncness: ast::IsAsync) - -> io::Result<()> { + pub fn print_asyncness(&mut self, asyncness: &ast::IsAsync) -> io::Result<()> { if asyncness.is_async() { self.word_nbsp("async")?; } @@ -3126,7 +3125,7 @@ impl<'a> State<'a> { span: syntax_pos::DUMMY_SP, }; self.print_fn(decl, - ast::FnHeader { unsafety, abi, ..ast::FnHeader::default() }, + &ast::FnHeader { unsafety, abi, ..ast::FnHeader::default() }, name, &generics, &source_map::dummy_spanned(ast::VisibilityKind::Inherited))?; @@ -3189,7 +3188,7 @@ impl<'a> State<'a> { } pub fn print_fn_header_info(&mut self, - header: ast::FnHeader, + header: &ast::FnHeader, vis: &ast::Visibility) -> io::Result<()> { self.s.word(visibility_qualified(vis, ""))?; @@ -3198,7 +3197,7 @@ impl<'a> State<'a> { ast::Constness::Const => self.word_nbsp("const")? } - self.print_asyncness(header.asyncness.node)?; + self.print_asyncness(&header.asyncness.node)?; self.print_unsafety(header.unsafety)?; if header.abi != Abi::Rust { @@ -3247,7 +3246,7 @@ mod tests { assert_eq!( fun_to_string( &decl, - ast::FnHeader { + &ast::FnHeader { unsafety: ast::Unsafety::Normal, constness: source_map::dummy_spanned(ast::Constness::NotConst), asyncness: source_map::dummy_spanned(ast::IsAsync::NotAsync), From 7c6dc7a254eacec43b95862a5a943b3e8435aaa6 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 12 Mar 2019 17:12:18 +0100 Subject: [PATCH 04/10] Move `async fn` arguments into closure. This commit takes advantage of `AsyncArgument` type that was added in a previous commit to replace the arguments of the `async fn` in the HIR and add statements to move the bindings from the new arguments to the pattern from the old argument. For example, the async function `foo` below: async fn foo((x, _y): (T, V)) { async move { } } becomes: async fn foo(__arg0: (T, V)) { async move { let (x, _y) = __arg0; } } --- src/librustc/hir/lowering.rs | 88 ++++++++++++++----- src/librustc/hir/map/def_collector.rs | 34 ++++--- src/librustc_resolve/lib.rs | 26 +++++- .../mir-opt/inline-closure-borrows-arg.rs | 2 +- src/test/mir-opt/inline-closure.rs | 2 +- 5 files changed, 115 insertions(+), 37 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 93a327588fef6..5dabf10cbf83d 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -448,10 +448,9 @@ impl<'a> LoweringContext<'a> { impl<'lcx, 'interner> Visitor<'lcx> for MiscCollector<'lcx, 'interner> { fn visit_pat(&mut self, p: &'lcx Pat) { match p.node { - // Doesn't generate a Hir node + // Doesn't generate a HIR node PatKind::Paren(..) => {}, _ => { - if let Some(owner) = self.hir_id_owner { self.lctx.lower_node_id_with_owner(p.id, owner); } @@ -461,6 +460,31 @@ impl<'a> LoweringContext<'a> { visit::walk_pat(self, p) } + fn visit_fn(&mut self, fk: visit::FnKind<'lcx>, fd: &'lcx FnDecl, s: Span, _: NodeId) { + if fk.header().map(|h| h.asyncness.node.is_async()).unwrap_or(false) { + // Don't visit the original pattern for async functions as it will be + // replaced. + for arg in &fd.inputs { + self.visit_ty(&arg.ty) + } + self.visit_fn_ret_ty(&fd.output); + + match fk { + visit::FnKind::ItemFn(_, decl, _, body) => { + self.visit_fn_header(decl); + self.visit_block(body) + }, + visit::FnKind::Method(_, sig, _, body) => { + self.visit_fn_header(&sig.header); + self.visit_block(body) + }, + visit::FnKind::Closure(body) => self.visit_expr(body), + } + } else { + visit::walk_fn(self, fk, fd, s) + } + } + fn visit_item(&mut self, item: &'lcx Item) { let hir_id = self.lctx.allocate_hir_id_counter(item.id).hir_id; @@ -3003,12 +3027,18 @@ impl<'a> LoweringContext<'a> { asyncness: &IsAsync, body: &Block, ) -> hir::BodyId { - self.lower_body(Some(decl), |this| { - if let IsAsync::Async { closure_id, .. } = asyncness { + self.lower_body(Some(&decl), |this| { + if let IsAsync::Async { closure_id, ref arguments, .. } = asyncness { + let mut body = body.clone(); + + for a in arguments.iter().rev() { + body.stmts.insert(0, a.stmt.clone()); + } + let async_expr = this.make_async_expr( CaptureBy::Value, *closure_id, None, |this| { - let body = this.lower_block(body, false); + let body = this.lower_block(&body, false); this.expr_block(body, ThinVec::new()) }); this.expr(body.span, async_expr, ThinVec::new()) @@ -3070,23 +3100,39 @@ impl<'a> LoweringContext<'a> { ItemKind::Fn(ref decl, ref header, ref generics, ref body) => { let fn_def_id = self.resolver.definitions().local_def_id(id); self.with_new_scopes(|this| { - // Note: we don't need to change the return type from `T` to - // `impl Future` here because lower_body - // only cares about the input argument patterns in the function - // declaration (decl), not the return types. - let body_id = this.lower_async_body(decl, &header.asyncness.node, body); + let mut lower_fn = |decl: &FnDecl| { + // Note: we don't need to change the return type from `T` to + // `impl Future` here because lower_body + // only cares about the input argument patterns in the function + // declaration (decl), not the return types. + let body_id = this.lower_async_body(&decl, &header.asyncness.node, body); + + let (generics, fn_decl) = this.add_in_band_defs( + generics, + fn_def_id, + AnonymousLifetimeMode::PassThrough, + |this, idty| this.lower_fn_decl( + &decl, + Some((fn_def_id, idty)), + true, + header.asyncness.node.opt_return_id() + ), + ); - let (generics, fn_decl) = this.add_in_band_defs( - generics, - fn_def_id, - AnonymousLifetimeMode::PassThrough, - |this, idty| this.lower_fn_decl( - decl, - Some((fn_def_id, idty)), - true, - header.asyncness.node.opt_return_id() - ), - ); + (body_id, generics, fn_decl) + }; + + let (body_id, generics, fn_decl) = if let IsAsync::Async { + arguments, .. + } = &header.asyncness.node { + 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(); + lower_fn(&decl) + } else { + lower_fn(decl) + }; hir::ItemKind::Fn( fn_decl, diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index 6ba2a65703bdc..0fa9738532215 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -73,12 +73,12 @@ impl<'a> DefCollector<'a> { decl: &'a FnDecl, body: &'a Block, ) { - let (closure_id, return_impl_trait_id) = match header.asyncness.node { + let (closure_id, return_impl_trait_id, arguments) = match &header.asyncness.node { IsAsync::Async { closure_id, return_impl_trait_id, - .. - } => (closure_id, return_impl_trait_id), + arguments, + } => (closure_id, return_impl_trait_id, arguments), _ => unreachable!(), }; @@ -87,17 +87,31 @@ impl<'a> DefCollector<'a> { let fn_def_data = DefPathData::ValueNs(name.as_interned_str()); let fn_def = self.create_def(id, fn_def_data, ITEM_LIKE_SPACE, span); return self.with_parent(fn_def, |this| { - this.create_def(return_impl_trait_id, DefPathData::ImplTrait, REGULAR_SPACE, span); + this.create_def(*return_impl_trait_id, DefPathData::ImplTrait, REGULAR_SPACE, span); visit::walk_generics(this, generics); - visit::walk_fn_decl(this, decl); - let closure_def = this.create_def(closure_id, - DefPathData::ClosureExpr, - REGULAR_SPACE, - span); + // Walk the generated arguments for the `async fn`. + for a in arguments { + use visit::Visitor; + this.visit_ty(&a.arg.ty); + } + + // We do not invoke `walk_fn_decl` as this will walk the arguments that are being + // replaced. + visit::walk_fn_ret_ty(this, &decl.output); + + let closure_def = this.create_def( + *closure_id, DefPathData::ClosureExpr, REGULAR_SPACE, span, + ); this.with_parent(closure_def, |this| { - visit::walk_block(this, body); + for a in arguments { + use visit::Visitor; + // Walk each of the generated statements before the regular block body. + this.visit_stmt(&a.stmt); + } + + visit::walk_block(this, &body); }) }) } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index de2f04035e41b..2ef05f7efeb73 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -834,11 +834,20 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { // Add each argument to the rib. let mut bindings_list = FxHashMap::default(); - for argument in &declaration.inputs { + let mut add_argument = |argument: &ast::Arg| { self.resolve_pattern(&argument.pat, PatternSource::FnParam, &mut bindings_list); self.visit_ty(&argument.ty); debug!("(resolving function) recorded argument"); + }; + + // 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); } + } else { + for a in &declaration.inputs { add_argument(a); } } + visit::walk_fn_ret_ty(self, &declaration.output); // Resolve the function body, potentially inside the body of an async closure @@ -849,9 +858,18 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { } match function_kind { - FnKind::ItemFn(.., body) | - FnKind::Method(.., body) => { - self.visit_block(body); + FnKind::ItemFn(.., body) | FnKind::Method(.., body) => { + if let IsAsync::Async { ref arguments, .. } = asyncness { + 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()); + } + self.visit_block(&body); + } else { + self.visit_block(body); + } } FnKind::Closure(body) => { self.visit_expr(body); diff --git a/src/test/mir-opt/inline-closure-borrows-arg.rs b/src/test/mir-opt/inline-closure-borrows-arg.rs index 2c30c7f365148..84567e1b4b8f2 100644 --- a/src/test/mir-opt/inline-closure-borrows-arg.rs +++ b/src/test/mir-opt/inline-closure-borrows-arg.rs @@ -20,7 +20,7 @@ fn foo(_t: T, q: &i32) -> i32 { // ... // bb0: { // ... -// _3 = [closure@HirId { owner: DefIndex(0:4), local_id: 29 }]; +// _3 = [closure@HirId { owner: DefIndex(0:4), local_id: 31 }]; // ... // _4 = &_3; // ... diff --git a/src/test/mir-opt/inline-closure.rs b/src/test/mir-opt/inline-closure.rs index 8116a445467ff..2be48927fd3b7 100644 --- a/src/test/mir-opt/inline-closure.rs +++ b/src/test/mir-opt/inline-closure.rs @@ -16,7 +16,7 @@ fn foo(_t: T, q: i32) -> i32 { // ... // bb0: { // ... -// _3 = [closure@HirId { owner: DefIndex(0:4), local_id: 13 }]; +// _3 = [closure@HirId { owner: DefIndex(0:4), local_id: 15 }]; // ... // _4 = &_3; // ... From 61346557cef165e69beef7d4eb45583020ecf988 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Mar 2019 10:45:36 +0100 Subject: [PATCH 05/10] Enforce consistent drop order w/ async methods. This commit extends the previous commit to apply to trait methods as well as free functions. --- src/librustc/hir/lowering.rs | 36 ++++++++--- src/test/run-pass/issue-54716.rs | 103 +++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 5dabf10cbf83d..5afac999fc0e6 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3611,15 +3611,33 @@ impl<'a> LoweringContext<'a> { ) } ImplItemKind::Method(ref sig, ref body) => { - let body_id = self.lower_async_body(&sig.decl, &sig.header.asyncness.node, body); - let impl_trait_return_allow = !self.is_in_trait_impl; - let (generics, sig) = self.lower_method_sig( - &i.generics, - sig, - impl_item_def_id, - impl_trait_return_allow, - sig.header.asyncness.node.opt_return_id(), - ); + let mut lower_method = |sig: &MethodSig| { + let body_id = self.lower_async_body( + &sig.decl, &sig.header.asyncness.node, body + ); + let impl_trait_return_allow = !self.is_in_trait_impl; + let (generics, sig) = self.lower_method_sig( + &i.generics, + sig, + impl_item_def_id, + impl_trait_return_allow, + sig.header.asyncness.node.opt_return_id(), + ); + (body_id, generics, sig) + }; + + let (body_id, generics, sig) = if let IsAsync::Async { + ref arguments, .. + } = sig.header.asyncness.node { + 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(); + lower_method(&sig) + } else { + lower_method(sig) + }; + (generics, hir::ImplItemKind::Method(sig, body_id)) } ImplItemKind::Type(ref ty) => ( diff --git a/src/test/run-pass/issue-54716.rs b/src/test/run-pass/issue-54716.rs index 7f64c6e120eb4..d075d2d619106 100644 --- a/src/test/run-pass/issue-54716.rs +++ b/src/test/run-pass/issue-54716.rs @@ -10,6 +10,7 @@ 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::task::Context; @@ -49,6 +50,46 @@ async fn foobar(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { x.1.borrow_mut().push(DropOrder::Function); } +struct Foo; + +impl Foo { + async fn foo(x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + async fn bar(x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + async fn baz((x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); + } + + 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> { + async fn foo(&'a self, x: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + async fn bar(&'a self, x: D, _: D) { + x.1.borrow_mut().push(DropOrder::Function); + } + + async fn baz(&'a self, (x, _): (D, D)) { + x.1.borrow_mut().push(DropOrder::Function); + } + + async fn foobar(&'a self, x: D, (a, _, _c): (D, D, D), _: D, _y: D) { + x.1.borrow_mut().push(DropOrder::Function); + } +} + fn main() { let empty = Arc::new(EmptyWaker); let waker = ArcWake::into_waker(empty); @@ -60,6 +101,8 @@ fn main() { // 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 + let af = Arc::new(RefCell::new(Vec::new())); let mut fut = Box::pin(foo(D("x", af.clone()), D("_y", af.clone()))); let _ = fut.as_mut().poll(&mut cx); @@ -86,4 +129,64 @@ fn main() { assert_eq!(*af.borrow(), &[ Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), ]); + + // Methods w/out self + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(Foo::foo(D("x", af.clone()), D("_y", af.clone()))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("_y"), Val("x")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(Foo::bar(D("x", af.clone()), D("_", af.clone()))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(Foo::baz((D("x", af.clone()), D("_", af.clone())))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(Foo::foobar( + D("x", af.clone()), + (D("a", af.clone()), D("_", af.clone()), D("_c", af.clone())), + D("_", af.clone()), + D("_y", af.clone()), + )); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[ + Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), + ]); + + // Methods + + let b = Bar(Default::default()); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(b.foo(D("x", af.clone()), D("_y", af.clone()))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("_y"), Val("x")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(b.bar(D("x", af.clone()), D("_", af.clone()))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(b.baz((D("x", af.clone()), D("_", af.clone())))); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); + + let af = Arc::new(RefCell::new(Vec::new())); + let mut fut = Box::pin(b.foobar( + D("x", af.clone()), + (D("a", af.clone()), D("_", af.clone()), D("_c", af.clone())), + D("_", af.clone()), + D("_y", af.clone()), + )); + let _ = fut.as_mut().poll(&mut cx); + assert_eq!(*af.borrow(), &[ + Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), + ]); } From 92e72df2c1402d3d8fceac81e650b633c555a523 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Mar 2019 17:08:34 +0100 Subject: [PATCH 06/10] Do not specify type in generated let bindings. This avoids issues with `impl_trait_in_bindings` as the type from the argument is normally used as the let binding, but `impl Trait` is unstable in binding position. --- src/libsyntax/parse/parser.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 26ede5a1057db..172d5c38c778b 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -8858,15 +8858,16 @@ impl<'a> Parser<'a> { }), }; - // Construct a `let : = __argN;` statement to insert at the top of the + // Construct a `let = __argN;` statement to insert at the top of the // async closure. let local = P(Local { pat: input.pat.clone(), - ty: Some(P(Ty { - id, - node: input.ty.node.clone(), - span: input.ty.span, - })), + // 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 + // be valid in this binding. At the time of writing (13 Mar 19), + // `impl_trait_in_bindings` is not stable. + ty: None, init: Some(P(Expr { id, node: ExprKind::Path(None, ast::Path { From 9d7da824d6a691d58d486e22cc789c572e3d0bf0 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Mar 2019 17:10:27 +0100 Subject: [PATCH 07/10] Introduce `ArgSource` for diagnostics. This commit introduces an `ArgSource` enum that is lowered into the HIR so that diagnostics can correctly refer to the argument pattern's original name rather than the generated pattern. --- src/librustc/hir/intravisit.rs | 10 ++++++++++ src/librustc/hir/lowering.rs | 9 +++++++++ src/librustc/hir/mod.rs | 20 +++++++++++++++++++ .../nice_region_error/different_lifetimes.rs | 15 ++++++-------- .../nice_region_error/named_anon_conflict.rs | 9 ++++----- src/librustc/lint/context.rs | 3 +++ src/librustc/middle/resolve_lifetime.rs | 2 +- src/librustc_privacy/lib.rs | 20 +++++++++++++++++++ src/librustc_typeck/check/mod.rs | 10 ++++++++++ src/librustc_typeck/check/writeback.rs | 10 ++++++++++ src/libsyntax/ast.rs | 11 ++++++++++ src/libsyntax/ext/build.rs | 3 ++- src/libsyntax/mut_visit.rs | 14 ++++++++++++- src/libsyntax/parse/parser.rs | 10 ++++++---- src/libsyntax/visit.rs | 3 +++ 15 files changed, 128 insertions(+), 21 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index b653093c1f8a4..a0c9e5983a1d7 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -262,6 +262,9 @@ pub trait Visitor<'v> : Sized { fn visit_pat(&mut self, p: &'v Pat) { walk_pat(self, p) } + fn visit_argument_source(&mut self, s: &'v ArgSource) { + walk_argument_source(self, s) + } fn visit_anon_const(&mut self, c: &'v AnonConst) { walk_anon_const(self, c) } @@ -399,10 +402,17 @@ pub fn walk_body<'v, V: Visitor<'v>>(visitor: &mut V, body: &'v Body) { for argument in &body.arguments { visitor.visit_id(argument.hir_id); visitor.visit_pat(&argument.pat); + visitor.visit_argument_source(&argument.source); } visitor.visit_expr(&body.value); } +pub fn walk_argument_source<'v, V: Visitor<'v>>(visitor: &mut V, source: &'v ArgSource) { + if let ArgSource::AsyncFn(pat) = source { + visitor.visit_pat(pat); + } +} + pub fn walk_local<'v, V: Visitor<'v>>(visitor: &mut V, local: &'v Local) { // Intentionally visiting the expr first - the initialization expr // dominates the local's definition. diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 5afac999fc0e6..28899af629bee 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -465,6 +465,7 @@ impl<'a> LoweringContext<'a> { // Don't visit the original pattern for async functions as it will be // replaced. for arg in &fd.inputs { + if let ArgSource::AsyncFn(pat) = &arg.source { self.visit_pat(pat); } self.visit_ty(&arg.ty) } self.visit_fn_ret_ty(&fd.output); @@ -2271,6 +2272,14 @@ impl<'a> LoweringContext<'a> { hir::Arg { hir_id, pat: self.lower_pat(&arg.pat), + source: self.lower_arg_source(&arg.source), + } + } + + fn lower_arg_source(&mut self, source: &ArgSource) -> hir::ArgSource { + match source { + ArgSource::Normal => hir::ArgSource::Normal, + ArgSource::AsyncFn(pat) => hir::ArgSource::AsyncFn(self.lower_pat(pat)), } } diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index e873663a613b4..1ebaa60fc885a 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1894,6 +1894,26 @@ pub struct InlineAsm { pub struct Arg { pub pat: P, pub hir_id: HirId, + pub source: ArgSource, +} + +impl Arg { + /// Returns the pattern representing the original binding for this argument. + pub fn original_pat(&self) -> &P { + match &self.source { + ArgSource::Normal => &self.pat, + ArgSource::AsyncFn(pat) => &pat, + } + } +} + +/// Represents the source of an argument in a function header. +#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] +pub enum ArgSource { + /// Argument as specified by the user. + Normal, + /// Generated argument from `async fn` lowering, contains the original binding pattern. + AsyncFn(P), } /// Represents the header (not the body) of a function declaration. diff --git a/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs b/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs index 86d7a19bc8309..944cc8a8b1999 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs @@ -86,19 +86,16 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { let sub_is_ret_type = self.is_return_type_anon(scope_def_id_sub, bregion_sub, ty_fndecl_sub); - let span_label_var1 = if let Some(simple_ident) = anon_arg_sup.pat.simple_ident() { - format!(" from `{}`", simple_ident) - } else { - String::new() + let span_label_var1 = match anon_arg_sup.original_pat().simple_ident() { + Some(simple_ident) => format!(" from `{}`", simple_ident), + None => String::new(), }; - let span_label_var2 = if let Some(simple_ident) = anon_arg_sub.pat.simple_ident() { - format!(" into `{}`", simple_ident) - } else { - String::new() + let span_label_var2 = match anon_arg_sub.original_pat().simple_ident() { + Some(simple_ident) => format!(" into `{}`", simple_ident), + None => String::new(), }; - let (span_1, span_2, main_label, span_label) = match (sup_is_ret_type, sub_is_ret_type) { (None, None) => { let (main_label_1, span_label_1) = if ty_sup.hir_id == ty_sub.hir_id { diff --git a/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs index 7403a5d7dbb09..2d7587b11b6f8 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs @@ -95,13 +95,12 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { } } - let (error_var, span_label_var) = if let Some(simple_ident) = arg.pat.simple_ident() { - ( + let (error_var, span_label_var) = match arg.original_pat().simple_ident() { + Some(simple_ident) => ( format!("the type of `{}`", simple_ident), format!("the type of `{}`", simple_ident), - ) - } else { - ("parameter type".to_owned(), "type".to_owned()) + ), + None => ("parameter type".to_owned(), "type".to_owned()), }; let mut diag = struct_span_err!( diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 709e5c4cce4ef..d7f97f7a58eac 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -1337,6 +1337,9 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> 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); + } self.visit_ty(&a.arg.ty); // ..and the statement. diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 3306bcae2123d..814776c21bd2a 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -2421,7 +2421,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { let help_name = if let Some(body) = parent { let arg = &self.tcx.hir().body(body).arguments[index]; - format!("`{}`", self.tcx.hir().hir_to_pretty_string(arg.pat.hir_id)) + format!("`{}`", self.tcx.hir().hir_to_pretty_string(arg.original_pat().hir_id)) } else { format!("argument {}", index + 1) }; diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index edb3efb78a39c..57e17eb6878e1 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -948,6 +948,16 @@ impl<'a, 'tcx> Visitor<'tcx> for NamePrivacyVisitor<'a, 'tcx> { intravisit::walk_pat(self, pat); } + + fn visit_argument_source(&mut self, s: &'tcx hir::ArgSource) { + match s { + // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has + // a `NodeId` w/out a type, as it is only used for getting the name of the original + // pattern for diagnostics where only an `hir::Arg` is present. + hir::ArgSource::AsyncFn(..) => {}, + _ => intravisit::walk_argument_source(self, s), + } + } } //////////////////////////////////////////////////////////////////////////////////////////// @@ -1133,6 +1143,16 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> { intravisit::walk_pat(self, pattern); } + fn visit_argument_source(&mut self, s: &'tcx hir::ArgSource) { + match s { + // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has + // a `NodeId` w/out a type, as it is only used for getting the name of the original + // pattern for diagnostics where only an `hir::Arg` is present. + hir::ArgSource::AsyncFn(..) => {}, + _ => intravisit::walk_argument_source(self, s), + } + } + fn visit_local(&mut self, local: &'tcx hir::Local) { if let Some(ref init) = local.init { if self.check_expr_pat_type(init.hir_id, init.span) { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 15190f569655f..79477b6fea87d 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1005,6 +1005,16 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for GatherLocalsVisitor<'a, 'gcx, 'tcx> { // Don't descend into the bodies of nested closures fn visit_fn(&mut self, _: intravisit::FnKind<'gcx>, _: &'gcx hir::FnDecl, _: hir::BodyId, _: Span, _: hir::HirId) { } + + fn visit_argument_source(&mut self, s: &'gcx hir::ArgSource) { + match s { + // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has + // a `NodeId` w/out a type, as it is only used for getting the name of the original + // pattern for diagnostics where only an `hir::Arg` is present. + hir::ArgSource::AsyncFn(..) => {}, + _ => intravisit::walk_argument_source(self, s), + } + } } /// When `check_fn` is invoked on a generator (i.e., a body that diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index d9df4672f1478..efff08f669690 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -297,6 +297,16 @@ impl<'cx, 'gcx, 'tcx> Visitor<'gcx> for WritebackCx<'cx, 'gcx, 'tcx> { let ty = self.resolve(&ty, &hir_ty.span); self.write_ty_to_tables(hir_ty.hir_id, ty); } + + fn visit_argument_source(&mut self, s: &'gcx hir::ArgSource) { + match s { + // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has + // a `NodeId` w/out a type, as it is only used for getting the name of the original + // pattern for diagnostics where only an `hir::Arg` is present. + hir::ArgSource::AsyncFn(..) => {}, + _ => intravisit::walk_argument_source(self, s), + } + } } impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index e6669e0d6edf2..81d8cfd862279 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1736,6 +1736,16 @@ pub struct Arg { pub ty: P, pub pat: P, pub id: NodeId, + pub source: ArgSource, +} + +/// The source of an argument in a function header. +#[derive(Clone, RustcEncodable, RustcDecodable, Debug)] +pub enum ArgSource { + /// Argument as written by the user. + Normal, + /// Argument from `async fn` lowering, contains the original binding pattern. + AsyncFn(P), } /// Alternative representation for `Arg`s describing `self` parameter of methods. @@ -1795,6 +1805,7 @@ impl Arg { }), ty, id: DUMMY_NODE_ID, + source: ArgSource::Normal, }; match eself.node { SelfKind::Explicit(ty, mutbl) => arg(mutbl, ty), diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 029c45eaa7247..40dd187ed28a7 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -979,7 +979,8 @@ impl<'a> AstBuilder for ExtCtxt<'a> { ast::Arg { ty, pat: arg_pat, - id: ast::DUMMY_NODE_ID + id: ast::DUMMY_NODE_ID, + source: ast::ArgSource::Normal, } } diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index bb9116e678efc..d3441a2039b17 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -235,6 +235,10 @@ pub trait MutVisitor: Sized { noop_visit_arg(a, self); } + fn visit_arg_source(&mut self, a: &mut ArgSource) { + noop_visit_arg_source(a, self); + } + fn visit_generics(&mut self, generics: &mut Generics) { noop_visit_generics(generics, self); } @@ -564,10 +568,18 @@ pub fn noop_visit_meta_item(mi: &mut MetaItem, vis: &mut T) { vis.visit_span(span); } -pub fn noop_visit_arg(Arg { id, pat, ty }: &mut Arg, vis: &mut T) { +pub fn noop_visit_arg(Arg { id, pat, ty, source }: &mut Arg, vis: &mut T) { vis.visit_id(id); vis.visit_pat(pat); vis.visit_ty(ty); + vis.visit_arg_source(source); +} + +pub fn noop_visit_arg_source(source: &mut ArgSource, vis: &mut T) { + match source { + ArgSource::Normal => {}, + ArgSource::AsyncFn(pat) => vis.visit_pat(pat), + } } pub fn noop_visit_tt(tt: &mut TokenTree, vis: &mut T) { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 172d5c38c778b..d7330ff55adaf 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1,7 +1,7 @@ use crate::ast::{AngleBracketedArgs, AsyncArgument, ParenthesizedArgs, AttrStyle, BareFnTy}; use crate::ast::{GenericBound, TraitBoundModifier}; use crate::ast::Unsafety; -use crate::ast::{Mod, AnonConst, Arg, Arm, Guard, Attribute, BindingMode, TraitItemKind}; +use crate::ast::{Mod, AnonConst, Arg, ArgSource, Arm, Guard, Attribute, BindingMode, TraitItemKind}; use crate::ast::Block; use crate::ast::{BlockCheckMode, CaptureBy, Movability}; use crate::ast::{Constness, Crate}; @@ -550,7 +550,7 @@ fn dummy_arg(span: Span) -> Arg { span, id: ast::DUMMY_NODE_ID }; - Arg { ty: P(ty), pat: pat, id: ast::DUMMY_NODE_ID } + Arg { ty: P(ty), pat: pat, id: ast::DUMMY_NODE_ID, source: ast::ArgSource::Normal } } #[derive(Copy, Clone, Debug)] @@ -2126,7 +2126,7 @@ impl<'a> Parser<'a> { } }; - Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) + Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID, source: ast::ArgSource::Normal }) } /// Parses a single function argument. @@ -2149,7 +2149,8 @@ impl<'a> Parser<'a> { Ok(Arg { ty: t, pat, - id: ast::DUMMY_NODE_ID + id: ast::DUMMY_NODE_ID, + source: ast::ArgSource::Normal, }) } @@ -8856,6 +8857,7 @@ impl<'a> Parser<'a> { ), span, }), + source: ArgSource::AsyncFn(input.pat.clone()), }; // Construct a `let = __argN;` statement to insert at the top of the diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index fe74cbd649612..fc99d10b0b6c6 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -544,6 +544,9 @@ pub fn walk_fn_ret_ty<'a, V: Visitor<'a>>(visitor: &mut V, ret_ty: &'a FunctionR pub fn walk_fn_decl<'a, V: Visitor<'a>>(visitor: &mut V, function_declaration: &'a FnDecl) { for argument in &function_declaration.inputs { visitor.visit_pat(&argument.pat); + if let ArgSource::AsyncFn(pat) = &argument.source { + visitor.visit_pat(pat); + } visitor.visit_ty(&argument.ty) } visitor.visit_fn_ret_ty(&function_declaration.output) From 44ddbc55655268f04d1130620f3f359aa514a979 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 21 Apr 2019 00:04:10 +0100 Subject: [PATCH 08/10] Correct lowering order to avoid ICE after rebase. This commit changes the order that arguments and bodies of async functions are lowered so that when the body attempts to `lower_def` of a upvar then the id has already been assigned by lowering the argument first. --- src/librustc/hir/lowering.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 28899af629bee..b04e2e29bf2e5 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -809,12 +809,10 @@ impl<'a> LoweringContext<'a> { }) } - fn record_body(&mut self, value: hir::Expr, decl: Option<&FnDecl>) -> hir::BodyId { + fn record_body(&mut self, value: hir::Expr, arguments: HirVec) -> hir::BodyId { let body = hir::Body { - arguments: decl.map_or(hir_vec![], |decl| { - decl.inputs.iter().map(|x| self.lower_arg(x)).collect() - }), is_generator: self.is_generator, + arguments, value, }; let id = body.id(); @@ -1137,11 +1135,10 @@ impl<'a> LoweringContext<'a> { capture_clause: CaptureBy, closure_node_id: NodeId, ret_ty: Option<&Ty>, + span: Span, body: impl FnOnce(&mut LoweringContext<'_>) -> hir::Expr, ) -> hir::ExprKind { let prev_is_generator = mem::replace(&mut self.is_generator, true); - let body_expr = body(self); - let span = body_expr.span; let output = match ret_ty { Some(ty) => FunctionRetTy::Ty(P(ty.clone())), None => FunctionRetTy::Default(span), @@ -1151,7 +1148,11 @@ impl<'a> LoweringContext<'a> { output, c_variadic: false }; - let body_id = self.record_body(body_expr, Some(&decl)); + // Lower the arguments before the body otherwise the body will call `lower_def` expecting + // the argument to have been assigned an id already. + let arguments = self.lower_args(Some(&decl)); + let body_expr = body(self); + let body_id = self.record_body(body_expr, arguments); self.is_generator = prev_is_generator; let capture_clause = self.lower_capture_clause(capture_clause); @@ -1182,8 +1183,9 @@ impl<'a> LoweringContext<'a> { F: FnOnce(&mut LoweringContext<'_>) -> hir::Expr, { let prev = mem::replace(&mut self.is_generator, false); + let arguments = self.lower_args(decl); let result = f(self); - let r = self.record_body(result, decl); + let r = self.record_body(result, arguments); self.is_generator = prev; return r; } @@ -2267,6 +2269,10 @@ impl<'a> LoweringContext<'a> { } } + fn lower_args(&mut self, decl: Option<&FnDecl>) -> HirVec { + decl.map_or(hir_vec![], |decl| decl.inputs.iter().map(|x| self.lower_arg(x)).collect()) + } + fn lower_arg(&mut self, arg: &Arg) -> hir::Arg { let LoweredNodeId { node_id: _, hir_id } = self.lower_node_id(arg.id); hir::Arg { @@ -3045,7 +3051,7 @@ impl<'a> LoweringContext<'a> { } let async_expr = this.make_async_expr( - CaptureBy::Value, *closure_id, None, + CaptureBy::Value, *closure_id, None, body.span, |this| { let body = this.lower_block(&body, false); this.expr_block(body, ThinVec::new()) @@ -4190,7 +4196,7 @@ impl<'a> LoweringContext<'a> { hir::MatchSource::Normal, ), ExprKind::Async(capture_clause, closure_node_id, ref block) => { - self.make_async_expr(capture_clause, closure_node_id, None, |this| { + self.make_async_expr(capture_clause, closure_node_id, None, block.span, |this| { this.with_new_scopes(|this| { let block = this.lower_block(block, false); this.expr_block(block, ThinVec::new()) @@ -4236,7 +4242,7 @@ impl<'a> LoweringContext<'a> { Some(&**ty) } else { None }; let async_body = this.make_async_expr( - capture_clause, *closure_id, async_ret_ty, + capture_clause, *closure_id, async_ret_ty, body.span, |this| { this.with_new_scopes(|this| this.lower_expr(body)) }); From 09c707f0ca8198bcf81ceab2f68298f661b37b6f Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 21 Apr 2019 14:33:28 +0100 Subject: [PATCH 09/10] Display original pattern in rustdoc. This commit displays the original pattern in generated documentation for async functions rather than the synthesized pattern. --- src/librustdoc/clean/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 80e796b0af732..96e1138fb34e1 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2017,7 +2017,7 @@ impl<'a> Clean for (&'a [hir::Ty], hir::BodyId) { Arguments { values: self.0.iter().enumerate().map(|(i, ty)| { Argument { - name: name_from_pat(&body.arguments[i].pat), + name: name_from_pat(&body.arguments[i].original_pat()), type_: ty.clean(cx), } }).collect() From 119e67ac6b7d72c2b314777ba36b2191cbfa7309 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 23 Apr 2019 18:44:41 +0100 Subject: [PATCH 10/10] Reduce noise and document test case. This commit introduces a `assert_drop_order_after_poll` helper function to the test case for this case to reduce repetitive noise and documents what each function aims to test. --- src/test/run-pass/issue-54716.rs | 174 +++++++++++++++---------------- 1 file changed, 83 insertions(+), 91 deletions(-) diff --git a/src/test/run-pass/issue-54716.rs b/src/test/run-pass/issue-54716.rs index d075d2d619106..ea4f5e076b005 100644 --- a/src/test/run-pass/issue-54716.rs +++ b/src/test/run-pass/issue-54716.rs @@ -12,6 +12,7 @@ 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; @@ -26,7 +27,9 @@ enum DropOrder { Val(&'static str), } -struct D(&'static str, Arc>>); +type DropOrderListPtr = Rc>>; + +struct D(&'static str, DropOrderListPtr); impl Drop for D { fn drop(&mut self) { @@ -34,18 +37,24 @@ impl Drop for D { } } +/// 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); } @@ -53,18 +62,24 @@ async fn foobar(x: D, (a, _, _c): (D, D, D), _: D, _y: D) { 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); } @@ -73,120 +88,97 @@ impl Foo { 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 main() { +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); - use DropOrder::*; - - // Currently, 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 - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(foo(D("x", af.clone()), D("_y", af.clone()))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("_y"), Val("x")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(bar(D("x", af.clone()), D("_", af.clone()))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(baz((D("x", af.clone()), D("_", af.clone())))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(foobar( - D("x", af.clone()), - (D("a", af.clone()), D("_", af.clone()), D("_c", af.clone())), - D("_", af.clone()), - D("_y", af.clone()), - )); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[ - Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), - ]); - - // Methods w/out self - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(Foo::foo(D("x", af.clone()), D("_y", af.clone()))); + 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!(*af.borrow(), &[Function, Val("_y"), Val("x")]); - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(Foo::bar(D("x", af.clone()), D("_", af.clone()))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(Foo::baz((D("x", af.clone()), D("_", af.clone())))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(Foo::foobar( - D("x", af.clone()), - (D("a", af.clone()), D("_", af.clone()), D("_c", af.clone())), - D("_", af.clone()), - D("_y", af.clone()), - )); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[ - Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), - ]); + assert_eq!(*actual_order.borrow(), expected_order); +} - // Methods +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()); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(b.foo(D("x", af.clone()), D("_y", af.clone()))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("_y"), Val("x")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(b.bar(D("x", af.clone()), D("_", af.clone()))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(b.baz((D("x", af.clone()), D("_", af.clone())))); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[Function, Val("x"), Val("_")]); - - let af = Arc::new(RefCell::new(Vec::new())); - let mut fut = Box::pin(b.foobar( - D("x", af.clone()), - (D("a", af.clone()), D("_", af.clone()), D("_c", af.clone())), - D("_", af.clone()), - D("_y", af.clone()), - )); - let _ = fut.as_mut().poll(&mut cx); - assert_eq!(*af.borrow(), &[ - Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), - ]); + 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("_")]); }