diff --git a/Cargo.lock b/Cargo.lock index fb5ae6ce66303..992ebdb801af7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3746,6 +3746,7 @@ dependencies = [ "rustc_errors", "rustc_feature", "rustc_lexer", + "rustc_lint_defs", "rustc_macros", "rustc_parse", "rustc_serialize", diff --git a/compiler/rustc_expand/Cargo.toml b/compiler/rustc_expand/Cargo.toml index 25c2851f6de59..7413b0d9431f9 100644 --- a/compiler/rustc_expand/Cargo.toml +++ b/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" } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index e8c711cae64ef..73fbde70bda9f 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/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::SEMICOLON_IN_EXPRESSIONS_FROM_MACROS; 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, + lint_node_id: NodeId, arm_span: Span, } @@ -110,7 +113,8 @@ fn emit_frag_parse_err( impl<'a> ParserAnyMacro<'a> { crate fn make(mut self: Box>, kind: AstFragmentKind) -> AstFragment { - let ParserAnyMacro { site_span, macro_ident, ref mut parser, arm_span } = *self; + let ParserAnyMacro { site_span, macro_ident, ref mut parser, lint_node_id, 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( + SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, + parser.token.span, + lint_node_id, + "trailing semicolon in macro used in expression position", + ); parser.bump(); } @@ -276,6 +286,7 @@ fn generic_extension<'cx>( let mut p = Parser::new(sess, tts, false, None); p.last_type_ascription = cx.current_expansion.prior_type_ascription; + let lint_node_id = cx.resolver.lint_node_id(cx.current_expansion.id); // Let the context choose how to interpret the result. // Weird, but useful for X-macros. @@ -287,6 +298,7 @@ fn generic_extension<'cx>( // macro leaves unparsed tokens. site_span: sp, macro_ident: name, + lint_node_id, arm_span, }); } diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 658372ac336a8..a8bf1ce51bb74 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/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,52 @@ declare_lint! { "detects `#[unstable]` on stable trait implementations for stable types" } +declare_lint! { + /// The `semicolon_in_expressions_from_macros` 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,compile_fail + /// #![deny(semicolon_in_expressions_from_macros)] + /// macro_rules! foo { + /// () => { true; } + /// } + /// + /// fn main() { + /// let val = match true { + /// true => false, + /// _ => foo!() + /// }; + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### 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 SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, + Allow, + "trailing semicolon in macro body used as expression", + @future_incompatible = FutureIncompatibleInfo { + reference: "issue #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 +2967,7 @@ declare_lint_pass! { USELESS_DEPRECATED, UNSUPPORTED_NAKED_FUNCTIONS, MISSING_ABI, + SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, ] } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 5d6120cd31076..d0adee2429d9a 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -344,6 +344,8 @@ impl<'a> ResolverExpand for Resolver<'a> { } fn lint_node_id(&mut self, expn_id: ExpnId) -> NodeId { + // FIXME - make this more precise. This currently returns the NodeId of the + // nearest closing item - we should try to return the closest parent of the ExpnId self.invocation_parents .get(&expn_id) .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id]) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 5350c9eefe753..8576f57959a6f 100644 --- a/src/bootstrap/bootstrap.py +++ b/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 `-Wsemicolon_in_expressions_from_macros` after the next beta bump env["RUSTFLAGS"] += " -Wrust_2018_idioms -Wunused_lifetimes" if self.get_toml("deny-warnings", "rust") != "false": env["RUSTFLAGS"] += " -Dwarnings" diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 62065e27dd966..4b58e609f1560 100644 --- a/src/bootstrap/builder.rs +++ b/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("-Wsemicolon_in_expressions_from_macros"); + } if self.config.deny_warnings { lint_flags.push("-Dwarnings"); diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs b/src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs new file mode 100644 index 0000000000000..6f9e6ec0a57ff --- /dev/null +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.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!() + }; +} diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs new file mode 100644 index 0000000000000..605d5a0309cfc --- /dev/null +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs @@ -0,0 +1,30 @@ +// check-pass +#![warn(semicolon_in_expressions_from_macros)] + +#[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(semicolon_in_expressions_from_macros)] + let _ = { + foo!(first) + }; + + // This 'allow' doesn't work either + #[allow(semicolon_in_expressions_from_macros)] + let _ = foo!(second); + + // But this 'allow' does + #[allow(semicolon_in_expressions_from_macros)] + fn inner() { + let _ = foo!(third); + } +} diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr new file mode 100644 index 0000000000000..6f9f879661a41 --- /dev/null +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr @@ -0,0 +1,33 @@ +warning: trailing semicolon in macro used in expression position + --> $DIR/semicolon-in-expressions-from-macros.rs:7:13 + | +LL | true; + | ^ +... +LL | foo!(first) + | ----------- in this macro invocation + | +note: the lint level is defined here + --> $DIR/semicolon-in-expressions-from-macros.rs:2:9 + | +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 + = 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/semicolon-in-expressions-from-macros.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 + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: 2 warnings emitted +