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.

3 files changed, 74 insertions, 45 deletions

diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
index 1d6703077ac..f548e2848a7 100644
--- a/compiler/rustc_expand/src/expand.rs
+++ b/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!(),
                 })
             }
diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
index b79f00a8a36..74958c49849 100644
--- a/compiler/rustc_span/src/source_map.rs
+++ b/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)]
diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
index 7b9629e534b..ac4bb652244 100644
--- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
+++ b/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