Rollup merge of rust-lang#109354 - Swatinem:rm-closureid, r=compiler-errors

Remove the `NodeId` of `ast::ExprKind::Async`

This is a followup to rust-lang#104833 (review).

In my original attempt, I was using `LoweringContext::expr`, which was not correct as it creates a fresh `DefId`.
It now uses the correct `DefId` for the wrapping `Expr`, and also makes forwarding `#[track_caller]` attributes more explicit.

Author: GuillaumeGomez

compiler/rustc_ast/src/ast.rs

@@ -1426,13 +1426,9 @@ pub enum ExprKind {
     Block(P<Block>, Option<Label>),
     /// An async block (`async move { ... }`).
     ///
-    /// The `NodeId` is the `NodeId` for the closure that results from
-    /// desugaring an async block, just like the NodeId field in the
-    /// `Async::Yes` variant. This is necessary in order to create a def for the
-    /// closure which can be used as a parent of any child defs. Defs
-    /// created during lowering cannot be made the parent of any other
-    /// preexisting defs.
-    Async(CaptureBy, NodeId, P<Block>),
+    /// The async block used to have a `NodeId`, which was removed in favor of
+    /// using the parent `NodeId` of the parent `Expr`.
+    Async(CaptureBy, P<Block>),
     /// An await expression (`my_future.await`).
     Await(P<Expr>),
 

compiler/rustc_ast/src/mut_visit.rs

@@ -1407,8 +1407,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
             vis.visit_block(blk);
             visit_opt(label, |label| vis.visit_label(label));
         }
-        ExprKind::Async(_capture_by, node_id, body) => {
-            vis.visit_id(node_id);
+        ExprKind::Async(_capture_by, body) => {
             vis.visit_block(body);
         }
         ExprKind::Await(expr) => vis.visit_expr(expr),

compiler/rustc_ast/src/visit.rs

@@ -860,7 +860,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
             walk_list!(visitor, visit_label, opt_label);
             visitor.visit_block(block);
         }
-        ExprKind::Async(_, _, body) => {
+        ExprKind::Async(_, body) => {
             visitor.visit_block(body);
         }
         ExprKind::Await(expr) => visitor.visit_expr(expr),

compiler/rustc_ast_lowering/src/expr.rs

@@ -63,20 +63,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ExprKind::ForLoop(pat, head, body, opt_label) => {
                     return self.lower_expr_for(e, pat, head, body, *opt_label);
                 }
-                // Similarly, async blocks do not use `e.id` but rather `closure_node_id`.
-                ExprKind::Async(capture_clause, closure_node_id, block) => {
-                    let hir_id = self.lower_node_id(*closure_node_id);
-                    self.lower_attrs(hir_id, &e.attrs);
-                    return self.make_async_expr(
-                        *capture_clause,
-                        hir_id,
-                        *closure_node_id,
-                        None,
-                        e.span,
-                        hir::AsyncGeneratorKind::Block,
-                        |this| this.with_new_scopes(|this| this.lower_block_expr(block)),
-                    );
-                }
                 _ => (),
             }
 
@@ -187,6 +173,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     self.arena.alloc_from_iter(arms.iter().map(|x| self.lower_arm(x))),
                     hir::MatchSource::Normal,
                 ),
+                ExprKind::Async(capture_clause, block) => self.make_async_expr(
+                    *capture_clause,
+                    e.id,
+                    None,
+                    e.span,
+                    hir::AsyncGeneratorKind::Block,
+                    |this| this.with_new_scopes(|this| this.lower_block_expr(block)),
+                ),
                 ExprKind::Await(expr) => {
                     let dot_await_span = if expr.span.hi() < e.span.hi() {
                         let span_with_whitespace = self
@@ -320,7 +314,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ),
                 ExprKind::Try(sub_expr) => self.lower_expr_try(e.span, sub_expr),
 
-                ExprKind::Paren(_) | ExprKind::ForLoop(..) | ExprKind::Async(..) => {
+                ExprKind::Paren(_) | ExprKind::ForLoop(..) => {
                     unreachable!("already handled")
                 }
 
@@ -591,13 +585,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
     pub(super) fn make_async_expr(
         &mut self,
         capture_clause: CaptureBy,
-        outer_hir_id: hir::HirId,
         closure_node_id: NodeId,
         ret_ty: Option<hir::FnRetTy<'hir>>,
         span: Span,
         async_gen_kind: hir::AsyncGeneratorKind,
         body: impl FnOnce(&mut Self) -> hir::Expr<'hir>,
-    ) -> hir::Expr<'hir> {
+    ) -> hir::ExprKind<'hir> {
         let output = ret_ty.unwrap_or_else(|| hir::FnRetTy::DefaultReturn(self.lower_span(span)));
 
         // Resume argument type: `ResumeTy`
@@ -644,32 +637,36 @@ impl<'hir> LoweringContext<'_, 'hir> {
         });
 
         // `static |_task_context| -> <ret_ty> { body }`:
-        let generator_kind = {
-            let c = self.arena.alloc(hir::Closure {
-                def_id: self.local_def_id(closure_node_id),
-                binder: hir::ClosureBinder::Default,
-                capture_clause,
-                bound_generic_params: &[],
-                fn_decl,
-                body,
-                fn_decl_span: self.lower_span(span),
-                fn_arg_span: None,
-                movability: Some(hir::Movability::Static),
-                constness: hir::Constness::NotConst,
-            });
-
-            hir::ExprKind::Closure(c)
-        };
+        hir::ExprKind::Closure(self.arena.alloc(hir::Closure {
+            def_id: self.local_def_id(closure_node_id),
+            binder: hir::ClosureBinder::Default,
+            capture_clause,
+            bound_generic_params: &[],
+            fn_decl,
+            body,
+            fn_decl_span: self.lower_span(span),
+            fn_arg_span: None,
+            movability: Some(hir::Movability::Static),
+            constness: hir::Constness::NotConst,
+        }))
+    }
 
-        let hir_id = self.lower_node_id(closure_node_id);
+    /// Forwards a possible `#[track_caller]` annotation from `outer_hir_id` to
+    /// `inner_hir_id` in case the `closure_track_caller` feature is enabled.
+    pub(super) fn maybe_forward_track_caller(
+        &mut self,
+        span: Span,
+        outer_hir_id: hir::HirId,
+        inner_hir_id: hir::HirId,
+    ) {
         if self.tcx.features().closure_track_caller
             && let Some(attrs) = self.attrs.get(&outer_hir_id.local_id)
             && attrs.into_iter().any(|attr| attr.has_name(sym::track_caller))
         {
             let unstable_span =
                 self.mark_span_with_reason(DesugaringKind::Async, span, self.allow_gen_future.clone());
             self.lower_attrs(
-                hir_id,
+                inner_hir_id,
                 &[Attribute {
                     kind: AttrKind::Normal(ptr::P(NormalAttr {
                         item: AttrItem {
@@ -685,8 +682,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 }],
             );
         }
-
-        hir::Expr { hir_id, kind: generator_kind, span: self.lower_span(span) }
     }
 
     /// Desugar `<expr>.await` into:
@@ -1001,15 +996,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     None
                 };
 
-                this.make_async_expr(
+                let async_body = this.make_async_expr(
                     capture_clause,
-                    closure_hir_id,
                     inner_closure_id,
                     async_ret_ty,
                     body.span,
                     hir::AsyncGeneratorKind::Closure,
                     |this| this.with_new_scopes(|this| this.lower_expr_mut(body)),
-                )
+                );
+                let hir_id = this.lower_node_id(inner_closure_id);
+                this.maybe_forward_track_caller(body.span, closure_hir_id, hir_id);
+                hir::Expr { hir_id, kind: async_body, span: this.lower_span(body.span) }
             });
             body_id
         });

compiler/rustc_ast_lowering/src/item.rs

@@ -1146,7 +1146,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
             let async_expr = this.make_async_expr(
                 CaptureBy::Value,
-                fn_id,
                 closure_id,
                 None,
                 body.span,
@@ -1180,7 +1179,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 },
             );
 
-            (this.arena.alloc_from_iter(parameters), async_expr)
+            let hir_id = this.lower_node_id(closure_id);
+            this.maybe_forward_track_caller(body.span, fn_id, hir_id);
+            let expr = hir::Expr { hir_id, kind: async_expr, span: this.lower_span(body.span) };
+
+            (this.arena.alloc_from_iter(parameters), expr)
         })
     }
 

compiler/rustc_ast_pretty/src/pprust/state/expr.rs

@@ -439,7 +439,7 @@ impl<'a> State<'a> {
                 self.ibox(0);
                 self.print_block_with_attrs(blk, attrs);
             }
-            ast::ExprKind::Async(capture_clause, _, blk) => {
+            ast::ExprKind::Async(capture_clause, blk) => {
                 self.word_nbsp("async");
                 self.print_capture_clause(*capture_clause);
                 // cbox/ibox in analogy to the `ExprKind::Block` arm above

compiler/rustc_builtin_macros/src/assert/context.rs

@@ -287,7 +287,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
             // sync with the `rfc-2011-nicer-assert-messages/all-expr-kinds.rs` test.
             ExprKind::Assign(_, _, _)
             | ExprKind::AssignOp(_, _, _)
-            | ExprKind::Async(_, _, _)
+            | ExprKind::Async(_, _)
             | ExprKind::Await(_)
             | ExprKind::Block(_, _)
             | ExprKind::Break(_, _)

compiler/rustc_lint/src/early.rs

@@ -224,8 +224,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
             ast::ExprKind::Closure(box ast::Closure {
                 asyncness: ast::Async::Yes { closure_id, .. },
                 ..
-            })
-            | ast::ExprKind::Async(_, closure_id, ..) => self.check_id(closure_id),
+            }) => self.check_id(closure_id),
             _ => {}
         }
     }

compiler/rustc_parse/src/parser/expr.rs

@@ -2911,7 +2911,7 @@ impl<'a> Parser<'a> {
         self.expect_keyword(kw::Async)?;
         let capture_clause = self.parse_capture_clause()?;
         let (attrs, body) = self.parse_inner_attrs_and_block()?;
-        let kind = ExprKind::Async(capture_clause, DUMMY_NODE_ID, body);
+        let kind = ExprKind::Async(capture_clause, body);
         Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
     }
 

compiler/rustc_resolve/src/def_collector.rs

@@ -260,9 +260,7 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
                     Async::No => closure_def,
                 }
             }
-            ExprKind::Async(_, async_id, _) => {
-                self.create_def(async_id, DefPathData::ClosureExpr, expr.span)
-            }
+            ExprKind::Async(_, _) => self.create_def(expr.id, DefPathData::ClosureExpr, expr.span),
             _ => self.parent_def,
         };
 

src/tools/clippy/clippy_lints/src/redundant_async_block.rs

@@ -42,7 +42,7 @@ impl EarlyLintPass for RedundantAsyncBlock {
         if expr.span.from_expansion() {
             return;
         }
-        if let ExprKind::Async(_, _, block) = &expr.kind && block.stmts.len() == 1 &&
+        if let ExprKind::Async(_, block) = &expr.kind && block.stmts.len() == 1 &&
             let Some(Stmt { kind: StmtKind::Expr(last), .. }) = block.stmts.last() &&
             let ExprKind::Await(future) = &last.kind &&
             !future.span.from_expansion() &&

src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs

@@ -578,7 +578,7 @@ fn ident_difference_expr_with_base_location(
         | (Assign(_, _, _), Assign(_, _, _))
         | (TryBlock(_), TryBlock(_))
         | (Await(_), Await(_))
-        | (Async(_, _, _), Async(_, _, _))
+        | (Async(_, _), Async(_, _))
         | (Block(_, _), Block(_, _))
         | (Closure(_), Closure(_))
         | (Match(_, _), Match(_, _))

src/tools/clippy/clippy_utils/src/ast_utils.rs

@@ -209,7 +209,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
                 && eq_fn_decl(lf, rf)
                 && eq_expr(le, re)
         },
-        (Async(lc, _, lb), Async(rc, _, rb)) => lc == rc && eq_block(lb, rb),
+        (Async(lc, lb), Async(rc, rb)) => lc == rc && eq_block(lb, rb),
         (Range(lf, lt, ll), Range(rf, rt, rl)) => ll == rl && eq_expr_opt(lf, rf) && eq_expr_opt(lt, rt),
         (AddrOf(lbk, lm, le), AddrOf(rbk, rm, re)) => lbk == rbk && lm == rm && eq_expr(le, re),
         (Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp),

src/tools/rustfmt/src/expr.rs

@@ -366,7 +366,7 @@ pub(crate) fn format_expr(
                 ))
             }
         }
-        ast::ExprKind::Async(capture_by, _node_id, ref block) => {
+        ast::ExprKind::Async(capture_by, ref block) => {
             let mover = if capture_by == ast::CaptureBy::Value {
                 "move "
             } else {