Rollup merge of rust-lang#127058 - compiler-errors:tighten-async-span…

…s, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ``@estebank`` or ``@oli-obk`` or someone else on wg-diag or compiler i dont really care lol

compiler/rustc_ast/src/ast.rs

@@ -1454,7 +1454,10 @@ pub enum ExprKind {
     Block(P<Block>, Option<Label>),
     /// An `async` block (`async move { ... }`),
     /// or a `gen` block (`gen move { ... }`)
-    Gen(CaptureBy, P<Block>, GenBlockKind),
+    ///
+    /// The span is the "decl", which is the header before the body `{ }`
+    /// including the `asyng`/`gen` keywords and possibly `move`.
+    Gen(CaptureBy, P<Block>, GenBlockKind, Span),
     /// An await expression (`my_future.await`). Span is of await keyword.
     Await(P<Expr>, Span),
 

compiler/rustc_ast/src/mut_visit.rs

@@ -1528,8 +1528,9 @@ pub fn noop_visit_expr<T: MutVisitor>(
             visit_opt(label, |label| vis.visit_label(label));
             vis.visit_block(blk);
         }
-        ExprKind::Gen(_capture_by, body, _kind) => {
+        ExprKind::Gen(_capture_by, body, _kind, decl_span) => {
             vis.visit_block(body);
+            vis.visit_span(decl_span);
         }
         ExprKind::Await(expr, await_kw_span) => {
             vis.visit_expr(expr);

compiler/rustc_ast/src/visit.rs

@@ -1122,7 +1122,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V
             visit_opt!(visitor, visit_label, opt_label);
             try_visit!(visitor.visit_block(block));
         }
-        ExprKind::Gen(_capt, body, _kind) => try_visit!(visitor.visit_block(body)),
+        ExprKind::Gen(_capt, body, _kind, _decl_span) => try_visit!(visitor.visit_block(body)),
         ExprKind::Await(expr, _span) => try_visit!(visitor.visit_expr(expr)),
         ExprKind::Assign(lhs, rhs, _span) => {
             try_visit!(visitor.visit_expr(lhs));

compiler/rustc_ast_lowering/src/expr.rs

@@ -226,7 +226,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                         *fn_arg_span,
                     ),
                 },
-                ExprKind::Gen(capture_clause, block, genblock_kind) => {
+                ExprKind::Gen(capture_clause, block, genblock_kind, decl_span) => {
                     let desugaring_kind = match genblock_kind {
                         GenBlockKind::Async => hir::CoroutineDesugaring::Async,
                         GenBlockKind::Gen => hir::CoroutineDesugaring::Gen,
@@ -236,6 +236,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                         *capture_clause,
                         e.id,
                         None,
+                        *decl_span,
                         e.span,
                         desugaring_kind,
                         hir::CoroutineSource::Block,
@@ -615,6 +616,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         capture_clause: CaptureBy,
         closure_node_id: NodeId,
         return_ty: Option<hir::FnRetTy<'hir>>,
+        fn_decl_span: Span,
         span: Span,
         desugaring_kind: hir::CoroutineDesugaring,
         coroutine_source: hir::CoroutineSource,
@@ -691,7 +693,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             bound_generic_params: &[],
             fn_decl,
             body,
-            fn_decl_span: self.lower_span(span),
+            fn_decl_span: self.lower_span(fn_decl_span),
             fn_arg_span: None,
             kind: hir::ClosureKind::Coroutine(coroutine_kind),
             constness: hir::Constness::NotConst,
@@ -1082,6 +1084,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
                     &inner_decl,
                     |this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
+                    fn_decl_span,
                     body.span,
                     coroutine_kind,
                     hir::CoroutineSource::Closure,

compiler/rustc_ast_lowering/src/item.rs

@@ -212,6 +212,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     // declaration (decl), not the return types.
                     let coroutine_kind = header.coroutine_kind;
                     let body_id = this.lower_maybe_coroutine_body(
+                        *fn_sig_span,
                         span,
                         hir_id,
                         decl,
@@ -819,6 +820,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             }
             AssocItemKind::Fn(box Fn { sig, generics, body: Some(body), .. }) => {
                 let body_id = self.lower_maybe_coroutine_body(
+                    sig.span,
                     i.span,
                     hir_id,
                     &sig.decl,
@@ -942,6 +944,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             ),
             AssocItemKind::Fn(box Fn { sig, generics, body, .. }) => {
                 let body_id = self.lower_maybe_coroutine_body(
+                    sig.span,
                     i.span,
                     hir_id,
                     &sig.decl,
@@ -1140,6 +1143,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
     /// `gen {}` block as appropriate.
     fn lower_maybe_coroutine_body(
         &mut self,
+        fn_decl_span: Span,
         span: Span,
         fn_id: hir::HirId,
         decl: &FnDecl,
@@ -1153,6 +1157,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
                 decl,
                 |this| this.lower_block_expr(body),
+                fn_decl_span,
                 body.span,
                 coroutine_kind,
                 hir::CoroutineSource::Fn,
@@ -1174,6 +1179,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         &mut self,
         decl: &FnDecl,
         lower_body: impl FnOnce(&mut LoweringContext<'_, 'hir>) -> hir::Expr<'hir>,
+        fn_decl_span: Span,
         body_span: Span,
         coroutine_kind: CoroutineKind,
         coroutine_source: hir::CoroutineSource,
@@ -1344,13 +1350,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
         };
         let closure_id = coroutine_kind.closure_id();
 
-        let span = if let FnRetTy::Default(span) = decl.output
-            && matches!(coroutine_source, rustc_hir::CoroutineSource::Closure)
-        {
-            body_span.with_lo(span.lo())
-        } else {
-            body_span
-        };
         let coroutine_expr = self.make_desugared_coroutine_expr(
             // The default capture mode here is by-ref. Later on during upvar analysis,
             // we will force the captured arguments to by-move, but for async closures,
@@ -1359,7 +1358,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
             CaptureBy::Ref,
             closure_id,
             None,
-            span,
+            fn_decl_span,
+            body_span,
             desugaring_kind,
             coroutine_source,
             mkbody,

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

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

compiler/rustc_builtin_macros/src/assert/context.rs

@@ -298,7 +298,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::Gen(_, _, _)
+            | ExprKind::Gen(_, _, _, _)
             | ExprKind::Await(_, _)
             | ExprKind::Block(_, _)
             | ExprKind::Break(_, _)

compiler/rustc_parse/src/parser/expr.rs

@@ -3432,8 +3432,9 @@ impl<'a> Parser<'a> {
             }
         }
         let capture_clause = self.parse_capture_clause()?;
+        let decl_span = lo.to(self.prev_token.span);
         let (attrs, body) = self.parse_inner_attrs_and_block()?;
-        let kind = ExprKind::Gen(capture_clause, body, kind);
+        let kind = ExprKind::Gen(capture_clause, body, kind, decl_span);
         Ok(self.mk_expr_with_attrs(lo.to(self.prev_token.span), kind, attrs))
     }
 
@@ -4022,7 +4023,7 @@ impl MutVisitor for CondChecker<'_> {
             | ExprKind::Match(_, _, _)
             | ExprKind::Closure(_)
             | ExprKind::Block(_, _)
-            | ExprKind::Gen(_, _, _)
+            | ExprKind::Gen(_, _, _, _)
             | ExprKind::TryBlock(_)
             | ExprKind::Underscore
             | ExprKind::Path(_, _)

compiler/rustc_resolve/src/def_collector.rs

@@ -334,7 +334,7 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
                     None => closure_def,
                 }
             }
-            ExprKind::Gen(_, _, _) => {
+            ExprKind::Gen(_, _, _, _) => {
                 self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span)
             }
             ExprKind::ConstBlock(ref constant) => {

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

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

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

@@ -226,7 +226,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
                 && eq_fn_decl(lf, rf)
                 && eq_expr(le, re)
         },
-        (Gen(lc, lb, lk), Gen(rc, rb, rk)) => lc == rc && eq_block(lb, rb) && lk == rk,
+        (Gen(lc, lb, lk, _), Gen(rc, rb, rk, _)) => lc == rc && eq_block(lb, rb) && lk == rk,
         (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

@@ -372,7 +372,7 @@ pub(crate) fn format_expr(
                 ))
             }
         }
-        ast::ExprKind::Gen(capture_by, ref block, ref kind) => {
+        ast::ExprKind::Gen(capture_by, ref block, ref kind, _) => {
             let mover = if matches!(capture_by, ast::CaptureBy::Value { .. }) {
                 "move "
             } else {

tests/ui/async-await/async-block-control-flow-static-semantics.stderr

@@ -1,20 +1,18 @@
 error[E0267]: `break` inside `async` block
   --> $DIR/async-block-control-flow-static-semantics.rs:32:9
    |
-LL | /     async {
-LL | |         break 0u8;
-   | |         ^^^^^^^^^ cannot `break` inside `async` block
-LL | |     };
-   | |_____- enclosing `async` block
+LL |     async {
+   |     ----- enclosing `async` block
+LL |         break 0u8;
+   |         ^^^^^^^^^ cannot `break` inside `async` block
 
 error[E0267]: `break` inside `async` block
   --> $DIR/async-block-control-flow-static-semantics.rs:39:13
    |
-LL | /         async {
-LL | |             break 0u8;
-   | |             ^^^^^^^^^ cannot `break` inside `async` block
-LL | |         };
-   | |_________- enclosing `async` block
+LL |         async {
+   |         ----- enclosing `async` block
+LL |             break 0u8;
+   |             ^^^^^^^^^ cannot `break` inside `async` block
 
 error[E0308]: mismatched types
   --> $DIR/async-block-control-flow-static-semantics.rs:21:58
@@ -29,13 +27,13 @@ LL | |
 LL | | }
    | |_^ expected `u8`, found `()`
 
-error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 25:6}` to be a future that resolves to `()`, but it resolves to `u8`
+error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to be a future that resolves to `()`, but it resolves to `u8`
   --> $DIR/async-block-control-flow-static-semantics.rs:26:39
    |
 LL |     let _: &dyn Future<Output = ()> = &block;
    |                                       ^^^^^^ expected `()`, found `u8`
    |
-   = note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 25:6}` to `&dyn Future<Output = ()>`
+   = note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:23:17: 23:22}` to `&dyn Future<Output = ()>`
 
 error[E0308]: mismatched types
   --> $DIR/async-block-control-flow-static-semantics.rs:12:43
@@ -45,13 +43,13 @@ LL | fn return_targets_async_block_not_fn() -> u8 {
    |    |
    |    implicitly returns `()` as its body has no tail or `return` expression
 
-error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 16:6}` to be a future that resolves to `()`, but it resolves to `u8`
+error[E0271]: expected `{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to be a future that resolves to `()`, but it resolves to `u8`
   --> $DIR/async-block-control-flow-static-semantics.rs:17:39
    |
 LL |     let _: &dyn Future<Output = ()> = &block;
    |                                       ^^^^^^ expected `()`, found `u8`
    |
-   = note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 16:6}` to `&dyn Future<Output = ()>`
+   = note: required for the cast from `&{async block@$DIR/async-block-control-flow-static-semantics.rs:14:17: 14:22}` to `&dyn Future<Output = ()>`
 
 error[E0308]: mismatched types
   --> $DIR/async-block-control-flow-static-semantics.rs:49:44

tests/ui/async-await/async-borrowck-escaping-block-error.stderr

@@ -2,9 +2,8 @@ error[E0373]: async block may outlive the current function, but it borrows `x`,
   --> $DIR/async-borrowck-escaping-block-error.rs:6:14
    |
 LL |     Box::new(async { x } )
-   |              ^^^^^^^^-^^
-   |              |       |
-   |              |       `x` is borrowed here
+   |              ^^^^^   - `x` is borrowed here
+   |              |
    |              may outlive borrowed value `x`
    |
 note: async block is returned here
@@ -21,9 +20,8 @@ error[E0373]: async block may outlive the current function, but it borrows `x`,
   --> $DIR/async-borrowck-escaping-block-error.rs:11:5
    |
 LL |     async { *x }
-   |     ^^^^^^^^--^^
-   |     |       |
-   |     |       `x` is borrowed here
+   |     ^^^^^   -- `x` is borrowed here
+   |     |
    |     may outlive borrowed value `x`
    |
 note: async block is returned here

tests/ui/async-await/async-closures/wrong-fn-kind.stderr

@@ -22,20 +22,18 @@ LL | fn needs_async_fn(_: impl async Fn()) {}
 error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure
   --> $DIR/wrong-fn-kind.rs:9:20
    |
-LL |   fn needs_async_fn(_: impl async Fn()) {}
-   |                        --------------- change this to accept `FnMut` instead of `Fn`
+LL | fn needs_async_fn(_: impl async Fn()) {}
+   |                      --------------- change this to accept `FnMut` instead of `Fn`
 ...
-LL |       needs_async_fn(async || {
-   |       -------------- ^-------
-   |       |              |
-   |  _____|______________in this closure
-   | |     |
-   | |     expects `Fn` instead of `FnMut`
-LL | |
-LL | |         x += 1;
-   | |         - mutable borrow occurs due to use of `x` in closure
-LL | |     });
-   | |_____^ cannot borrow as mutable
+LL |     needs_async_fn(async || {
+   |     -------------- ^^^^^^^^
+   |     |              |
+   |     |              cannot borrow as mutable
+   |     |              in this closure
+   |     expects `Fn` instead of `FnMut`
+LL |
+LL |         x += 1;
+   |         - mutable borrow occurs due to use of `x` in closure
 
 error: aborting due to 2 previous errors
 

tests/ui/async-await/async-is-unwindsafe.stderr

@@ -1,20 +1,19 @@
 error[E0277]: the type `&mut Context<'_>` may not be safely transferred across an unwind boundary
   --> $DIR/async-is-unwindsafe.rs:12:5
    |
-LL |        is_unwindsafe(async {
-   |  ______^             -
-   | | ___________________|
-LL | ||
-LL | ||         use std::ptr::null;
-LL | ||         use std::task::{Context, RawWaker, RawWakerVTable, Waker};
-...  ||
-LL | ||         drop(cx_ref);
-LL | ||     });
-   | ||_____-^ `&mut Context<'_>` may not be safely transferred across an unwind boundary
-   |  |_____|
-   |        within this `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}`
+LL |       is_unwindsafe(async {
+   |       ^             ----- within this `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}`
+   |  _____|
+   | |
+LL | |
+LL | |         use std::ptr::null;
+LL | |         use std::task::{Context, RawWaker, RawWakerVTable, Waker};
+...  |
+LL | |         drop(cx_ref);
+LL | |     });
+   | |______^ `&mut Context<'_>` may not be safely transferred across an unwind boundary
    |
-   = help: within `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}`, the trait `UnwindSafe` is not implemented for `&mut Context<'_>`, which is required by `{async block@$DIR/async-is-unwindsafe.rs:12:19: 29:6}: UnwindSafe`
+   = help: within `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}`, the trait `UnwindSafe` is not implemented for `&mut Context<'_>`, which is required by `{async block@$DIR/async-is-unwindsafe.rs:12:19: 12:24}: UnwindSafe`
    = note: `UnwindSafe` is implemented for `&Context<'_>`, but not for `&mut Context<'_>`
 note: future does not implement `UnwindSafe` as this value is used across an await
   --> $DIR/async-is-unwindsafe.rs:25:18

tests/ui/async-await/coroutine-desc.stderr

@@ -8,8 +8,8 @@ LL |     fun(async {}, async {});
    |     |   expected all arguments to be this `async` block type because they need to match the type of this parameter
    |     arguments to this function are incorrect
    |
-   = note: expected `async` block `{async block@$DIR/coroutine-desc.rs:10:9: 10:17}`
-              found `async` block `{async block@$DIR/coroutine-desc.rs:10:19: 10:27}`
+   = note: expected `async` block `{async block@$DIR/coroutine-desc.rs:10:9: 10:14}`
+              found `async` block `{async block@$DIR/coroutine-desc.rs:10:19: 10:24}`
    = note: no two async blocks, even if identical, have the same type
    = help: consider pinning your async block and casting it to a trait object
 note: function defined here

tests/ui/async-await/coroutine-not-future.stderr

@@ -26,11 +26,11 @@ note: required by a bound in `takes_coroutine`
 LL | fn takes_coroutine<ResumeTy>(_g: impl Coroutine<ResumeTy, Yield = (), Return = ()>) {}
    |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `takes_coroutine`
 
-error[E0277]: the trait bound `{async block@$DIR/coroutine-not-future.rs:39:21: 39:29}: Coroutine<_>` is not satisfied
+error[E0277]: the trait bound `{async block@$DIR/coroutine-not-future.rs:39:21: 39:26}: Coroutine<_>` is not satisfied
   --> $DIR/coroutine-not-future.rs:39:21
    |
 LL |     takes_coroutine(async {});
-   |     --------------- ^^^^^^^^ the trait `Coroutine<_>` is not implemented for `{async block@$DIR/coroutine-not-future.rs:39:21: 39:29}`
+   |     --------------- ^^^^^^^^ the trait `Coroutine<_>` is not implemented for `{async block@$DIR/coroutine-not-future.rs:39:21: 39:26}`
    |     |
    |     required by a bound introduced by this call
    |