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).

Author: bors

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(),

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>) {

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,
                 });
             }

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));

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

src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr

@@ -14,6 +14,8 @@ LL | #![warn(semicolon_in_expressions_from_macros)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #79813 <https://github.com/rust-lang/rust/issues/79813>
+   = note: macro invocations at the end of a block are treated as expressions
+   = note: to ignore the value produced by the macro, add a semicolon after the invocation of `foo`
    = note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: trailing semicolon in macro used in expression position