Auto merge of #89860 - camsteffen:macro-semi, r=petrochenkov

Remove trailing semicolon from macro call span

Macro call site spans are now less surprising/more consistent since they no longer contain a semicolon after the macro call.

The downside is that we need to do a little guesswork to get the semicolon in diagnostics. But this should not be noticeable since it is rare for the semicolon to not immediately follow the macro call.

compiler/rustc_expand/src/expand.rs

@@ -1024,12 +1024,10 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         placeholder(fragment_kind, NodeId::placeholder_from_expn_id(expn_id), vis)
     }
 
-    fn collect_bang(
-        &mut self,
-        mac: ast::MacCall,
-        span: Span,
-        kind: AstFragmentKind,
-    ) -> AstFragment {
+    fn collect_bang(&mut self, mac: ast::MacCall, kind: AstFragmentKind) -> AstFragment {
+        // cache the macro call span so that it can be
+        // easily adjusted for incremental compilation
+        let span = mac.span();
         self.collect(kind, InvocationKind::Bang { mac, span })
     }
 
@@ -1087,25 +1085,19 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 let MacCallStmt { mac, style, attrs, .. } = mac.into_inner();
                 Ok((style == MacStmtStyle::Semicolon, mac, attrs.into()))
             }
-            StmtKind::Item(ref item) if matches!(item.kind, ItemKind::MacCall(..)) => {
-                match stmt.kind {
-                    StmtKind::Item(item) => match item.into_inner() {
-                        ast::Item { kind: ItemKind::MacCall(mac), attrs, .. } => {
-                            Ok((mac.args.need_semicolon(), mac, attrs))
-                        }
-                        _ => unreachable!(),
-                    },
+            StmtKind::Item(item) if matches!(item.kind, ItemKind::MacCall(..)) => {
+                match item.into_inner() {
+                    ast::Item { kind: ItemKind::MacCall(mac), attrs, .. } => {
+                        Ok((mac.args.need_semicolon(), mac, attrs))
+                    }
                     _ => unreachable!(),
                 }
             }
-            StmtKind::Semi(ref expr) if matches!(expr.kind, ast::ExprKind::MacCall(..)) => {
-                match stmt.kind {
-                    StmtKind::Semi(expr) => match expr.into_inner() {
-                        ast::Expr { kind: ast::ExprKind::MacCall(mac), attrs, .. } => {
-                            Ok((mac.args.need_semicolon(), mac, attrs.into()))
-                        }
-                        _ => unreachable!(),
-                    },
+            StmtKind::Semi(expr) if matches!(expr.kind, ast::ExprKind::MacCall(..)) => {
+                match expr.into_inner() {
+                    ast::Expr { kind: ast::ExprKind::MacCall(mac), attrs, .. } => {
+                        Ok((mac.args.need_semicolon(), mac, attrs.into()))
+                    }
                     _ => unreachable!(),
                 }
             }
@@ -1222,7 +1214,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
 
             if let ast::ExprKind::MacCall(mac) = expr.kind {
                 self.check_attributes(&expr.attrs, &mac);
-                self.collect_bang(mac, expr.span, AstFragmentKind::Expr).make_expr().into_inner()
+                self.collect_bang(mac, AstFragmentKind::Expr).make_expr().into_inner()
             } else {
                 assign_id!(self, &mut expr.id, || {
                     ensure_sufficient_stack(|| noop_visit_expr(&mut expr, self));
@@ -1318,7 +1310,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
 
             if let ast::ExprKind::MacCall(mac) = expr.kind {
                 self.check_attributes(&expr.attrs, &mac);
-                self.collect_bang(mac, expr.span, AstFragmentKind::OptExpr)
+                self.collect_bang(mac, AstFragmentKind::OptExpr)
                     .make_opt_expr()
                     .map(|expr| expr.into_inner())
             } else {
@@ -1339,9 +1331,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         }
 
         visit_clobber(pat, |mut pat| match mem::replace(&mut pat.kind, PatKind::Wild) {
-            PatKind::MacCall(mac) => {
-                self.collect_bang(mac, pat.span, AstFragmentKind::Pat).make_pat()
-            }
+            PatKind::MacCall(mac) => self.collect_bang(mac, AstFragmentKind::Pat).make_pat(),
             _ => unreachable!(),
         });
     }
@@ -1360,12 +1350,10 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
                     .make_stmts();
             }
 
-            let span = stmt.span;
             match self.take_stmt_bang(stmt) {
                 Ok((add_semicolon, mac, attrs)) => {
                     self.check_attributes(&attrs, &mac);
-                    let mut stmts =
-                        self.collect_bang(mac, span, AstFragmentKind::Stmts).make_stmts();
+                    let mut stmts = self.collect_bang(mac, AstFragmentKind::Stmts).make_stmts();
 
                     // If this is a macro invocation with a semicolon, then apply that
                     // semicolon to the final statement produced by expansion.
@@ -1433,7 +1421,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
                 item.attrs = attrs;
                 item.and_then(|item| match item.kind {
                     ItemKind::MacCall(mac) => {
-                        self.collect_bang(mac, span, AstFragmentKind::Items).make_items()
+                        self.collect_bang(mac, AstFragmentKind::Items).make_items()
                     }
                     _ => unreachable!(),
                 })
@@ -1542,9 +1530,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
             ast::AssocItemKind::MacCall(ref mac) => {
                 self.check_attributes(&item.attrs, &mac);
                 item.and_then(|item| match item.kind {
-                    ast::AssocItemKind::MacCall(mac) => self
-                        .collect_bang(mac, item.span, AstFragmentKind::TraitItems)
-                        .make_trait_items(),
+                    ast::AssocItemKind::MacCall(mac) => {
+                        self.collect_bang(mac, AstFragmentKind::TraitItems).make_trait_items()
+                    }
                     _ => unreachable!(),
                 })
             }
@@ -1567,9 +1555,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
             ast::AssocItemKind::MacCall(ref mac) => {
                 self.check_attributes(&item.attrs, &mac);
                 item.and_then(|item| match item.kind {
-                    ast::AssocItemKind::MacCall(mac) => self
-                        .collect_bang(mac, item.span, AstFragmentKind::ImplItems)
-                        .make_impl_items(),
+                    ast::AssocItemKind::MacCall(mac) => {
+                        self.collect_bang(mac, AstFragmentKind::ImplItems).make_impl_items()
+                    }
                     _ => unreachable!(),
                 })
             }
@@ -1586,9 +1574,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         };
 
         visit_clobber(ty, |mut ty| match mem::replace(&mut ty.kind, ast::TyKind::Err) {
-            ast::TyKind::MacCall(mac) => {
-                self.collect_bang(mac, ty.span, AstFragmentKind::Ty).make_ty()
-            }
+            ast::TyKind::MacCall(mac) => self.collect_bang(mac, AstFragmentKind::Ty).make_ty(),
             _ => unreachable!(),
         });
     }
@@ -1613,9 +1599,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
             ast::ForeignItemKind::MacCall(ref mac) => {
                 self.check_attributes(&foreign_item.attrs, &mac);
                 foreign_item.and_then(|item| match item.kind {
-                    ast::ForeignItemKind::MacCall(mac) => self
-                        .collect_bang(mac, item.span, AstFragmentKind::ForeignItems)
-                        .make_foreign_items(),
+                    ast::ForeignItemKind::MacCall(mac) => {
+                        self.collect_bang(mac, AstFragmentKind::ForeignItems).make_foreign_items()
+                    }
                     _ => unreachable!(),
                 })
             }

compiler/rustc_span/src/source_map.rs

@@ -653,6 +653,18 @@ impl SourceMap {
         })
     }
 
+    /// Extends the given `Span` while the next character matches the predicate
+    pub fn span_extend_while(
+        &self,
+        span: Span,
+        f: impl Fn(char) -> bool,
+    ) -> Result<Span, SpanSnippetError> {
+        self.span_to_source(span, |s, _start, end| {
+            let n = s[end..].char_indices().find(|&(_, c)| !f(c)).map_or(s.len() - end, |(i, _)| i);
+            Ok(span.with_hi(span.hi() + BytePos(n as u32)))
+        })
+    }
+
     /// Extends the given `Span` to just after the next occurrence of `c`.
     pub fn span_extend_to_next_char(&self, sp: Span, c: char, accept_newlines: bool) -> Span {
         if let Ok(next_source) = self.span_to_next_source(sp) {
@@ -1013,6 +1025,32 @@ impl SourceMap {
         let source_file = &self.files()[source_file_index];
         source_file.is_imported()
     }
+
+    /// Gets the span of a statement. If the statement is a macro expansion, the
+    /// span in the context of the block span is found. The trailing semicolon is included
+    /// on a best-effort basis.
+    pub fn stmt_span(&self, stmt_span: Span, block_span: Span) -> Span {
+        if !stmt_span.from_expansion() {
+            return stmt_span;
+        }
+        let mac_call = original_sp(stmt_span, block_span);
+        self.mac_call_stmt_semi_span(mac_call).map_or(mac_call, |s| mac_call.with_hi(s.hi()))
+    }
+
+    /// Tries to find the span of the semicolon of a macro call statement.
+    /// The input must be the *call site* span of a statement from macro expansion.
+    ///
+    ///           v output
+    ///     mac!();
+    ///     ^^^^^^ input
+    pub fn mac_call_stmt_semi_span(&self, mac_call: Span) -> Option<Span> {
+        let span = self.span_extend_while(mac_call, char::is_whitespace).ok()?;
+        let span = span.shrink_to_hi().with_hi(BytePos(span.hi().0.checked_add(1)?));
+        if self.span_to_snippet(span).as_deref() != Ok(";") {
+            return None;
+        }
+        Some(span)
+    }
 }
 
 #[derive(Clone)]

compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs

@@ -1171,8 +1171,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         {
             return None;
         }
-        let original_span = original_sp(last_stmt.span, blk.span);
-        Some((original_span.with_lo(original_span.hi() - BytePos(1)), needs_box))
+        let span = if last_stmt.span.from_expansion() {
+            let mac_call = original_sp(last_stmt.span, blk.span);
+            self.tcx.sess.source_map().mac_call_stmt_semi_span(mac_call)?
+        } else {
+            last_stmt.span.with_lo(last_stmt.span.hi() - BytePos(1))
+        };
+        Some((span, needs_box))
     }
 
     // Instantiates the given path, which must refer to an item with the given

src/test/mir-opt/unreachable_asm.main.UnreachablePropagation.diff

@@ -59,7 +59,7 @@
           StorageDead(_6);                 // scope 2 at $DIR/unreachable_asm.rs:18:9: 18:10
           StorageDead(_5);                 // scope 2 at $DIR/unreachable_asm.rs:18:9: 18:10
           StorageLive(_7);                 // scope 2 at $DIR/unreachable_asm.rs:21:9: 21:37
-          llvm_asm!(LlvmInlineAsmInner { asm: "NOP", asm_str_style: Cooked, outputs: [], inputs: [], clobbers: [], volatile: true, alignstack: false, dialect: Att } : [] : []); // scope 3 at $DIR/unreachable_asm.rs:21:18: 21:35
+          llvm_asm!(LlvmInlineAsmInner { asm: "NOP", asm_str_style: Cooked, outputs: [], inputs: [], clobbers: [], volatile: true, alignstack: false, dialect: Att } : [] : []); // scope 3 at $DIR/unreachable_asm.rs:21:18: 21:34
           _7 = const ();                   // scope 3 at $DIR/unreachable_asm.rs:21:9: 21:37
           StorageDead(_7);                 // scope 2 at $DIR/unreachable_asm.rs:21:36: 21:37
           StorageLive(_8);                 // scope 2 at $DIR/unreachable_asm.rs:22:9: 22:21

src/test/mir-opt/unreachable_asm_2.main.UnreachablePropagation.diff

@@ -49,7 +49,7 @@
 
       bb3: {
           StorageLive(_7);                 // scope 2 at $DIR/unreachable_asm_2.rs:16:13: 16:41
-          llvm_asm!(LlvmInlineAsmInner { asm: "NOP", asm_str_style: Cooked, outputs: [], inputs: [], clobbers: [], volatile: true, alignstack: false, dialect: Att } : [] : []); // scope 3 at $DIR/unreachable_asm_2.rs:16:22: 16:39
+          llvm_asm!(LlvmInlineAsmInner { asm: "NOP", asm_str_style: Cooked, outputs: [], inputs: [], clobbers: [], volatile: true, alignstack: false, dialect: Att } : [] : []); // scope 3 at $DIR/unreachable_asm_2.rs:16:22: 16:38
           _7 = const ();                   // scope 3 at $DIR/unreachable_asm_2.rs:16:13: 16:41
           StorageDead(_7);                 // scope 2 at $DIR/unreachable_asm_2.rs:16:40: 16:41
           _4 = const 21_i32;               // scope 2 at $DIR/unreachable_asm_2.rs:17:13: 17:20
@@ -60,7 +60,7 @@
 
       bb4: {
           StorageLive(_8);                 // scope 2 at $DIR/unreachable_asm_2.rs:20:13: 20:41
-          llvm_asm!(LlvmInlineAsmInner { asm: "NOP", asm_str_style: Cooked, outputs: [], inputs: [], clobbers: [], volatile: true, alignstack: false, dialect: Att } : [] : []); // scope 4 at $DIR/unreachable_asm_2.rs:20:22: 20:39
+          llvm_asm!(LlvmInlineAsmInner { asm: "NOP", asm_str_style: Cooked, outputs: [], inputs: [], clobbers: [], volatile: true, alignstack: false, dialect: Att } : [] : []); // scope 4 at $DIR/unreachable_asm_2.rs:20:22: 20:38
           _8 = const ();                   // scope 4 at $DIR/unreachable_asm_2.rs:20:13: 20:41
           StorageDead(_8);                 // scope 2 at $DIR/unreachable_asm_2.rs:20:40: 20:41
           _4 = const 42_i32;               // scope 2 at $DIR/unreachable_asm_2.rs:21:13: 21:20

src/test/rustdoc-ui/intra-doc/warning.stderr

@@ -88,7 +88,7 @@ LL |         #[doc = $f]
    |         ^^^^^^^^^^^
 ...
 LL | f!("Foo\nbar [BarF] bar\nbaz");
-   | ------------------------------- in this macro invocation
+   | ------------------------------ in this macro invocation
    |
    = note: the link appears in this line:
 

src/test/ui-fulldeps/internal-lints/lint_pass_impl_without_macro.stderr

@@ -18,7 +18,7 @@ LL |         impl LintPass for Custom {
    |              ^^^^^^^^
 ...
 LL | custom_lint_pass_macro!();
-   | -------------------------- in this macro invocation
+   | ------------------------- in this macro invocation
    |
    = help: try using `declare_lint_pass!` or `impl_lint_pass!` instead
    = note: this error originates in the macro `custom_lint_pass_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

src/test/ui-fulldeps/issue-76270-panic-in-libproc-macro.stderr

@@ -2,7 +2,7 @@ error: proc macro panicked
   --> $DIR/issue-76270-panic-in-libproc-macro.rs:15:1
    |
 LL | proc_macro_panic::panic_in_libproc_macro!();
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: message: `""` is not a valid identifier
 

src/test/ui/annotate-snippet/multispan.stderr

@@ -2,41 +2,41 @@ error: hello to you, too!
   --> $DIR/multispan.rs:15:5
    |
 LL |     hello!(hi);
-   |     ^^^^^^^^^^^
+   |     ^^^^^^^^^^
    |
 error: hello to you, too!
   --> $DIR/multispan.rs:18:5
    |
 LL |     hello!(hi hi);
-   |     ^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^
    |
 error: hello to you, too!
   --> $DIR/multispan.rs:21:5
    |
 LL |     hello!(hi hi hi);
-   |     ^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^
    |
 error: hello to you, too!
   --> $DIR/multispan.rs:24:5
    |
 LL |     hello!(hi hey hi yo hi beep beep hi hi);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 error: hello to you, too!
   --> $DIR/multispan.rs:25:5
    |
 LL |     hello!(hi there, hi how are you? hi... hi.);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 error: hello to you, too!
   --> $DIR/multispan.rs:26:5
    |
 LL |     hello!(whoah. hi di hi di ho);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 error: hello to you, too!
   --> $DIR/multispan.rs:27:5
    |
 LL |     hello!(hi good hi and good bye);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |

src/test/ui/asm/aarch64/interpolated-idents.stderr

@@ -7,7 +7,7 @@ LL |               $options($pure, $nomem, $readonly, $preserves_flags, $noretur
 LL | /     m!(in out lateout inout inlateout const sym
 LL | |        pure nomem readonly preserves_flags
 LL | |        noreturn nostack options);
-   | |_________________________________- in this macro invocation
+   | |________________________________- in this macro invocation
    |
    = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
 
@@ -20,7 +20,7 @@ LL |               $options($pure, $nomem, $readonly, $preserves_flags, $noretur
 LL | /     m!(in out lateout inout inlateout const sym
 LL | |        pure nomem readonly preserves_flags
 LL | |        noreturn nostack options);
-   | |_________________________________- in this macro invocation
+   | |________________________________- in this macro invocation
    |
    = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
 
@@ -38,12 +38,12 @@ LL |       m!(in out lateout inout inlateout const sym
    | |
 LL | |        pure nomem readonly preserves_flags
 LL | |        noreturn nostack options);
-   | |                                 -
-   | |_________________________________|
-   | |_________________________________in this macro invocation
-   | |_________________________________in this macro invocation
-   | |_________________________________in this macro invocation
-   |                                   in this macro invocation
+   | |                                -
+   | |________________________________|
+   | |________________________________in this macro invocation
+   | |________________________________in this macro invocation
+   | |________________________________in this macro invocation
+   |                                  in this macro invocation
    |
    = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
 

src/test/ui/asm/aarch64/parse-error.stderr

@@ -2,7 +2,7 @@ error: requires at least a template string argument
   --> $DIR/parse-error.rs:9:9
    |
 LL |         asm!();
-   |         ^^^^^^^
+   |         ^^^^^^
 
 error: asm template must be a string literal
   --> $DIR/parse-error.rs:11:14
@@ -236,7 +236,7 @@ error: requires at least a template string argument
   --> $DIR/parse-error.rs:90:1
    |
 LL | global_asm!();
-   | ^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^
 
 error: asm template must be a string literal
   --> $DIR/parse-error.rs:92:13

src/test/ui/asm/aarch64/type-check-2.stderr

@@ -77,7 +77,7 @@ error[E0381]: use of possibly-uninitialized variable: `y`
   --> $DIR/type-check-2.rs:20:9
    |
 LL |         asm!("{}", inout(reg) y);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ use of possibly-uninitialized `y`
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^ use of possibly-uninitialized `y`
 
 error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable
   --> $DIR/type-check-2.rs:28:29