Add MACRO_TRAILING_SEMICOLON lint

cc rust-lang#79813

This PR adds an allow-by-default future-compatibility lint
`MACRO_TRAILING_SEMICOLON`. It fires when a trailing semicolon in a
macro body is ignored due to the macro being used in expression
position:

```rust
macro_rules! foo {
    () => {
        true; // WARN
    }
}

fn main() {
    let val = match true {
        true => false,
        _ => foo!()
    };
}
```

The lint takes its level from the macro call site, and
can be allowed for a particular macro by adding
`#[allow(macro_trailing_semicolon)]`.

The lint is set to warn for all internal rustc crates (when being built
by a stage1 compiler). After the next beta bump, we can enable
the lint for the bootstrap compiler as well.

Author: Aaron1011

Cargo.lock

@@ -3746,6 +3746,7 @@ dependencies = [
  "rustc_errors",
  "rustc_feature",
  "rustc_lexer",
+ "rustc_lint_defs",
  "rustc_macros",
  "rustc_parse",
  "rustc_serialize",

compiler/rustc_expand/Cargo.toml

@@ -18,6 +18,7 @@ rustc_attr = { path = "../rustc_attr" }
 rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_errors = { path = "../rustc_errors" }
 rustc_feature = { path = "../rustc_feature" }
+rustc_lint_defs = { path = "../rustc_lint_defs" }
 rustc_macros = { path = "../rustc_macros" }
 rustc_lexer = { path = "../rustc_lexer" }
 rustc_parse = { path = "../rustc_parse" }

compiler/rustc_expand/src/base.rs

@@ -347,6 +347,7 @@ pub trait TTMacroExpander {
         ecx: &'cx mut ExtCtxt<'_>,
         span: Span,
         input: TokenStream,
+        nearest_parent: NodeId,
     ) -> Box<dyn MacResult + 'cx>;
 }
 
@@ -362,6 +363,7 @@ where
         ecx: &'cx mut ExtCtxt<'_>,
         span: Span,
         input: TokenStream,
+        _nearest_parent: NodeId,
     ) -> Box<dyn MacResult + 'cx> {
         self(ecx, span, input)
     }

compiler/rustc_expand/src/expand.rs

@@ -721,7 +721,11 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 SyntaxExtensionKind::LegacyBang(expander) => {
                     let prev = self.cx.current_expansion.prior_type_ascription;
                     self.cx.current_expansion.prior_type_ascription = mac.prior_type_ascription;
-                    let tok_result = expander.expand(self.cx, span, mac.args.inner_tokens());
+                    // FIXME - this is imprecise. We should try to lint as close to the macro call
+                    // as possible (we cannot take attributes from the macro call itself).
+                    let nearest_parent = self.cx.resolver.lint_node_id(invoc.expansion_data.id);
+                    let tok_result =
+                        expander.expand(self.cx, span, mac.args.inner_tokens(), nearest_parent);
                     let result = if let Some(result) = fragment_kind.make_from(tok_result) {
                         result
                     } else {

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -11,12 +11,14 @@ use crate::mbe::transcribe::transcribe;
 use rustc_ast as ast;
 use rustc_ast::token::{self, NonterminalKind, NtTT, Token, TokenKind::*};
 use rustc_ast::tokenstream::{DelimSpan, TokenStream};
+use rustc_ast::NodeId;
 use rustc_ast_pretty::pprust;
 use rustc_attr::{self as attr, TransparencyError};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::{Applicability, DiagnosticBuilder};
 use rustc_feature::Features;
+use rustc_lint_defs::builtin::MACRO_TRAILING_SEMICOLON;
 use rustc_parse::parser::Parser;
 use rustc_session::parse::ParseSess;
 use rustc_session::Session;
@@ -37,6 +39,7 @@ crate struct ParserAnyMacro<'a> {
     site_span: Span,
     /// The ident of the macro we're parsing
     macro_ident: Ident,
+    nearest_parent: NodeId,
     arm_span: Span,
 }
 
@@ -110,7 +113,8 @@ 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, arm_span } = *self;
+        let ParserAnyMacro { site_span, macro_ident, ref mut parser, nearest_parent, arm_span } =
+            *self;
         let snapshot = &mut parser.clone();
         let fragment = match parse_ast_fragment(parser, kind) {
             Ok(f) => f,
@@ -124,6 +128,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(
+                MACRO_TRAILING_SEMICOLON,
+                parser.token.span,
+                nearest_parent,
+                "trailing semicolon in macro used in expression position",
+            );
             parser.bump();
         }
 
@@ -149,6 +159,7 @@ impl TTMacroExpander for MacroRulesMacroExpander {
         cx: &'cx mut ExtCtxt<'_>,
         sp: Span,
         input: TokenStream,
+        nearest_parent: NodeId,
     ) -> Box<dyn MacResult + 'cx> {
         if !self.valid {
             return DummyResult::any(sp);
@@ -160,6 +171,7 @@ impl TTMacroExpander for MacroRulesMacroExpander {
             self.name,
             self.transparency,
             input,
+            nearest_parent,
             &self.lhses,
             &self.rhses,
         )
@@ -187,6 +199,7 @@ fn generic_extension<'cx>(
     name: Ident,
     transparency: Transparency,
     arg: TokenStream,
+    nearest_parent: NodeId,
     lhses: &[mbe::TokenTree],
     rhses: &[mbe::TokenTree],
 ) -> Box<dyn MacResult + 'cx> {
@@ -287,6 +300,7 @@ fn generic_extension<'cx>(
                     // macro leaves unparsed tokens.
                     site_span: sp,
                     macro_ident: name,
+                    nearest_parent,
                     arm_span,
                 });
             }

compiler/rustc_lint_defs/src/builtin.rs

@@ -1,3 +1,4 @@
+// ignore-tidy-filelength
 //! Some lints that are built in to the compiler.
 //!
 //! These are the built-in lints that are emitted direct in the main
@@ -2833,6 +2834,49 @@ declare_lint! {
     "detects `#[unstable]` on stable trait implementations for stable types"
 }
 
+declare_lint! {
+    /// The `macro_trailing_semicolon` lint detects trailing semicolons
+    /// in macro bodies when the macro is invoked in expression position.
+    /// This was previous accepted, but is being phased out.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// macro_rules! foo {
+    ///     () => { true; }
+    /// }
+    ///
+    /// fn main() {
+    ///     let val = match true {
+    ///         true => false,
+    ///         _ => foo!()
+    ///     };
+    /// }
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// Previous, Rust ignored trailing semicolon in a macro
+    /// body when a macro was invoked in expression position.
+    /// However, this makes the treatment of semicolons in the language
+    /// inconsistent, and could lead to unexpected runtime behavior
+    /// in some circumstances (e.g. if the macro author expects
+    /// a value to be dropped).
+    ///
+    /// This is a [future-incompatible] lint to transition this
+    /// to a hard error in the future. See [issue #79813] for more details.
+    ///
+    /// [issue #79813]: https://github.com/rust-lang/rust/issues/79813
+    /// [future-incompatible]: ../index.md#future-incompatible-lints
+    pub MACRO_TRAILING_SEMICOLON,
+    Allow,
+    "trailing semicolon in macro body used as expression",
+    @future_incompatible = FutureIncompatibleInfo {
+        reference: "issue #79813 <https://github.com/rust-lang/rust/issues/79813>",
+        edition: None,
+    };
+}
+
 declare_lint_pass! {
     /// Does nothing as a lint pass, but registers some `Lint`s
     /// that are used by other parts of the compiler.
@@ -2920,6 +2964,7 @@ declare_lint_pass! {
         USELESS_DEPRECATED,
         UNSUPPORTED_NAKED_FUNCTIONS,
         MISSING_ABI,
+        MACRO_TRAILING_SEMICOLON,
     ]
 }
 

src/bootstrap/bootstrap.py

@@ -833,6 +833,7 @@ def build_bootstrap(self):
         target_linker = self.get_toml("linker", build_section)
         if target_linker is not None:
             env["RUSTFLAGS"] += " -C linker=" + target_linker
+        # cfg(bootstrap): Add `-Wmacro_trailing_semicolon` after the next beta bump
         env["RUSTFLAGS"] += " -Wrust_2018_idioms -Wunused_lifetimes"
         if self.get_toml("deny-warnings", "rust") != "false":
             env["RUSTFLAGS"] += " -Dwarnings"

src/bootstrap/builder.rs

@@ -1250,6 +1250,12 @@ impl<'a> Builder<'a> {
             // some code doesn't go through this `rustc` wrapper.
             lint_flags.push("-Wrust_2018_idioms");
             lint_flags.push("-Wunused_lifetimes");
+            // cfg(bootstrap): unconditionally enable this warning after the next beta bump
+            // This is currently disabled for the stage1 libstd, since build scripts
+            // will end up using the bootstrap compiler (which doesn't yet support this lint)
+            if compiler.stage != 0 && mode != Mode::Std {
+                lint_flags.push("-Wmacro_trailing_semicolon");
+            }
 
             if self.config.deny_warnings {
                 lint_flags.push("-Dwarnings");

src/test/ui/lint/macro-trailing-semicolon/allow-macro-trailing-semicolon.rs

@@ -0,0 +1,15 @@
+// check-pass
+// Ensure that trailing semicolons are allowed by default
+
+macro_rules! foo {
+    () => {
+        true;
+    }
+}
+
+fn main() {
+    let val = match true {
+        true => false,
+        _ => foo!()
+    };
+}

src/test/ui/lint/macro-trailing-semicolon/macro-trailing-semicolon.rs

@@ -0,0 +1,30 @@
+// check-pass
+#![warn(macro_trailing_semicolon)]
+
+#[allow(dead_code)]
+macro_rules! foo {
+    ($val:ident) => {
+        true; //~ WARN trailing
+              //~| WARN this was previously
+              //~| WARN trailing
+              //~| WARN this was previously
+    }
+}
+
+fn main() {
+    // This `allow` doesn't work
+    #[allow(macro_trailing_semicolon)]
+    let _ = {
+        foo!(first)
+    };
+
+    // This 'allow' doesn't work either
+    #[allow(macro_trailing_semicolon)]
+    let _ = foo!(second);
+
+    // But this 'allow' does
+    #[allow(macro_trailing_semicolon)]
+    fn inner() {
+        let _ = foo!(third);
+    }
+}

src/test/ui/lint/macro-trailing-semicolon/macro-trailing-semicolon.stderr

@@ -0,0 +1,33 @@
+warning: trailing semicolon in macro used in expression position
+  --> $DIR/macro-trailing-semicolon.rs:7:13
+   |
+LL |         true;
+   |             ^
+...
+LL |         foo!(first)
+   |         ----------- in this macro invocation
+   |
+note: the lint level is defined here
+  --> $DIR/macro-trailing-semicolon.rs:2:9
+   |
+LL | #![warn(macro_trailing_semicolon)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+   = 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: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+warning: trailing semicolon in macro used in expression position
+  --> $DIR/macro-trailing-semicolon.rs:7:13
+   |
+LL |         true;
+   |             ^
+...
+LL |     let _ = foo!(second);
+   |             ------------ in this macro invocation
+   |
+   = 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: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+warning: 2 warnings emitted
+