Auto merge of #87381 - Aaron1011:note-semi-trailing-macro, r=petrochenkov

Display an extra note for trailing semicolon lint with trailing macro

Currently, we parse macros at the end of a block
(e.g. `fn foo() { my_macro!() }`) as expressions, rather than
statements. This means that a macro invoked in this position
cannot expand to items or semicolon-terminated expressions.

In the future, we might want to start parsing these kinds of macros
as statements. This would make expansion more 'token-based'
(i.e. macro expansion behaves (almost) as if you just textually
replaced the macro invocation with its output). However,
this is a breaking change (see PR #78991), so it will require
further discussion.

Since the current behavior will not be changing any time soon,
we need to address the interaction with the
`SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint. Since we are parsing
the result of macro expansion as an expression, we will emit a lint
if there's a trailing semicolon in the macro output. However, this
results in a somewhat confusing message for users, since it visually
looks like there should be no problem with having a semicolon
at the end of a block
(e.g. `fn foo() { my_macro!() }` => `fn foo() { produced_expr; }`)

To help reduce confusion, this commit adds a note explaining
that the macro is being interpreted as an expression. Additionally,
we suggest adding a semicolon after the macro *invocation* - this
will cause us to parse the macro call as a statement. We do *not*
use a structured suggestion for this, since the user may actually
want to remove the semicolon from the macro definition (allowing
the block to evaluate to the expression produced by the macro).

5 files changed, 39 insertions, 5 deletions

diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs
index 3d5bc770c4f..b454737fb80 100644
--- a/compiler/rustc_expand/src/base.rs
+++ b/compiler/rustc_expand/src/base.rs
@@ -929,6 +929,7 @@ pub struct ExpansionData {
     pub prior_type_ascription: Option<(Span, bool)>,
     /// Some parent node that is close to this macro call
     pub lint_node_id: NodeId,
+    pub is_trailing_mac: bool,
 }
 
 type OnExternModLoaded<'a> =
@@ -979,6 +980,7 @@ impl<'a> ExtCtxt<'a> {
                 dir_ownership: DirOwnership::Owned { relative: None },
                 prior_type_ascription: None,
                 lint_node_id: ast::CRATE_NODE_ID,
+                is_trailing_mac: false,
             },
             force_mode: false,
             expansions: FxHashMap::default(),
diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
index c72b1b33dbc..09beda33483 100644
--- a/compiler/rustc_expand/src/expand.rs
+++ b/compiler/rustc_expand/src/expand.rs
@@ -1328,14 +1328,30 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
             return placeholder;
         }
 
+        // The only way that we can end up with a `MacCall` expression statement,
+        // (as opposed to a `StmtKind::MacCall`) is if we have a macro as the
+        // traiing expression in a block (e.g. `fn foo() { my_macro!() }`).
+        // Record this information, so that we can report a more specific
+        // `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint if needed.
+        // See #78991 for an investigation of treating macros in this position
+        // as statements, rather than expressions, during parsing.
+        if let StmtKind::Expr(expr) = &stmt.kind {
+            if matches!(**expr, ast::Expr { kind: ast::ExprKind::MacCall(..), .. }) {
+                self.cx.current_expansion.is_trailing_mac = true;
+            }
+        }
+
         // The placeholder expander gives ids to statements, so we avoid folding the id here.
         // We don't use `assign_id!` - it will be called when we visit statement's contents
         // (e.g. an expression, item, or local)
         let ast::Stmt { id, kind, span } = stmt;
-        noop_flat_map_stmt_kind(kind, self)
+        let res = noop_flat_map_stmt_kind(kind, self)
             .into_iter()
             .map(|kind| ast::Stmt { id, kind, span })
-            .collect()
+            .collect();
+
+        self.cx.current_expansion.is_trailing_mac = false;
+        res
     }
 
     fn visit_block(&mut self, block: &mut P<Block>) {
diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs
index 7f985af364d..b97593b92b3 100644
--- a/compiler/rustc_expand/src/mbe/macro_rules.rs
+++ b/compiler/rustc_expand/src/mbe/macro_rules.rs
@@ -43,6 +43,7 @@ crate struct ParserAnyMacro<'a> {
     /// The ident of the macro we're parsing
     macro_ident: Ident,
     lint_node_id: NodeId,
+    is_trailing_mac: bool,
     arm_span: Span,
 }
 
@@ -116,8 +117,14 @@ fn emit_frag_parse_err(
 
 impl<'a> ParserAnyMacro<'a> {
     crate fn make(mut self: Box<ParserAnyMacro<'a>>, kind: AstFragmentKind) -> AstFragment {
-        let ParserAnyMacro { site_span, macro_ident, ref mut parser, lint_node_id, arm_span } =
-            *self;
+        let ParserAnyMacro {
+            site_span,
+            macro_ident,
+            ref mut parser,
+            lint_node_id,
+            arm_span,
+            is_trailing_mac,
+        } = *self;
         let snapshot = &mut parser.clone();
         let fragment = match parse_ast_fragment(parser, kind) {
             Ok(f) => f,
@@ -131,11 +138,12 @@ impl<'a> ParserAnyMacro<'a> {
         // `macro_rules! m { () => { panic!(); } }` isn't parsed by `.parse_expr()`,
         // but `m!()` is allowed in expression positions (cf. issue #34706).
         if kind == AstFragmentKind::Expr && parser.token == token::Semi {
-            parser.sess.buffer_lint(
+            parser.sess.buffer_lint_with_diagnostic(
                 SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
                 parser.token.span,
                 lint_node_id,
                 "trailing semicolon in macro used in expression position",
+                BuiltinLintDiagnostics::TrailingMacro(is_trailing_mac, macro_ident),
             );
             parser.bump();
         }
@@ -301,6 +309,7 @@ fn generic_extension<'cx>(
                     site_span: sp,
                     macro_ident: name,
                     lint_node_id: cx.current_expansion.lint_node_id,
+                    is_trailing_mac: cx.current_expansion.is_trailing_mac,
                     arm_span,
                 });
             }
diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs
index 31ce77375e5..ad8a41a56cc 100644
--- a/compiler/rustc_lint/src/context.rs
+++ b/compiler/rustc_lint/src/context.rs
@@ -744,6 +744,12 @@ pub trait LintContext: Sized {
                         &format!("the built-in attribute `{attr_name}` will be ignored, since it's applied to the macro invocation `{macro_name}`")
                     );
                 }
+                BuiltinLintDiagnostics::TrailingMacro(is_trailing, name) => {
+                    if is_trailing {
+                        db.note("macro invocations at the end of a block are treated as expressions");
+                        db.note(&format!("to ignore the value produced by the macro, add a semicolon after the invocation of `{name}`"));
+                    }
+                }
             }
             // Rewrap `db`, and pass control to the user.
             decorate(LintDiagnosticBuilder::new(db));
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
index ec17e7a6b03..6c38b8f5bc0 100644
--- a/compiler/rustc_lint_defs/src/lib.rs
+++ b/compiler/rustc_lint_defs/src/lib.rs
@@ -303,6 +303,7 @@ pub enum BuiltinLintDiagnostics {
     ProcMacroBackCompat(String),
     OrPatternsBackCompat(Span, String),
     ReservedPrefix(Span),
+    TrailingMacro(bool, Ident),
 }
 
 /// Lints that are buffered up early on in the `Session` before the