Skip to content

Commit

Permalink
Add SEMICOLON_IN_EXPRESSIONS_FROM_MACROS lint
Browse files Browse the repository at this point in the history
cc rust-lang#79813

This PR adds an allow-by-default future-compatibility lint
`SEMICOLON_IN_EXPRESSIONS_FROM_MACROS`. 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(semicolon_in_expressions_from_macros)]`.

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.
  • Loading branch information
Aaron1011 committed Jan 28, 2021
1 parent 0e19020 commit f902551
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 1 deletion.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3746,6 +3746,7 @@ dependencies = [
"rustc_errors",
"rustc_feature",
"rustc_lexer",
"rustc_lint_defs",
"rustc_macros",
"rustc_parse",
"rustc_serialize",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_expand/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
}

Expand Down Expand Up @@ -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, lint_node_id, arm_span } =
*self;
let snapshot = &mut parser.clone();
let fragment = match parse_ast_fragment(parser, kind) {
Ok(f) => f,
Expand All @@ -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();
}

Expand Down Expand Up @@ -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.
Expand All @@ -287,6 +298,7 @@ fn generic_extension<'cx>(
// macro leaves unparsed tokens.
site_span: sp,
macro_ident: name,
lint_node_id,
arm_span,
});
}
Expand Down
48 changes: 48 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 <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.
Expand Down Expand Up @@ -2920,6 +2967,7 @@ declare_lint_pass! {
USELESS_DEPRECATED,
UNSUPPORTED_NAKED_FUNCTIONS,
MISSING_ABI,
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
]
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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!()
};
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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 <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/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 <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

0 comments on commit f902551

Please sign in to comment.