Skip to content

Commit

Permalink
Rollup merge of rust-lang#87385 - Aaron1011:final-enable-semi, r=petr…
Browse files Browse the repository at this point in the history
…ochenkov

Make `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` warn by default

This PR makes the `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint warn by default.

To avoid showing a large number of un-actionable warnings to users, we only enable the lint for macros defined in the same crate. This ensures that users will be able to fix the warning by simply removing a semicolon.

In the future, I'd like to enable this lint unconditionally, and eventually make it into a hard error in a future edition. This PR is a step towards that goal.
  • Loading branch information
JohnTitor committed Jul 30, 2021
2 parents 94a9765 + e70ce57 commit d29e007
Show file tree
Hide file tree
Showing 21 changed files with 135 additions and 37 deletions.
26 changes: 19 additions & 7 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ crate struct ParserAnyMacro<'a> {
lint_node_id: NodeId,
is_trailing_mac: bool,
arm_span: Span,
/// Whether or not this macro is defined in the current crate
is_local: bool,
}

crate fn annotate_err_with_kind(
Expand Down Expand Up @@ -124,6 +126,7 @@ impl<'a> ParserAnyMacro<'a> {
lint_node_id,
arm_span,
is_trailing_mac,
is_local,
} = *self;
let snapshot = &mut parser.clone();
let fragment = match parse_ast_fragment(parser, kind) {
Expand All @@ -138,13 +141,15 @@ 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_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),
);
if is_local {
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();
}

Expand All @@ -162,6 +167,7 @@ struct MacroRulesMacroExpander {
lhses: Vec<mbe::TokenTree>,
rhses: Vec<mbe::TokenTree>,
valid: bool,
is_local: bool,
}

impl TTMacroExpander for MacroRulesMacroExpander {
Expand All @@ -183,6 +189,7 @@ impl TTMacroExpander for MacroRulesMacroExpander {
input,
&self.lhses,
&self.rhses,
self.is_local,
)
}
}
Expand Down Expand Up @@ -210,6 +217,7 @@ fn generic_extension<'cx>(
arg: TokenStream,
lhses: &[mbe::TokenTree],
rhses: &[mbe::TokenTree],
is_local: bool,
) -> Box<dyn MacResult + 'cx> {
let sess = &cx.sess.parse_sess;

Expand Down Expand Up @@ -311,6 +319,7 @@ fn generic_extension<'cx>(
lint_node_id: cx.current_expansion.lint_node_id,
is_trailing_mac: cx.current_expansion.is_trailing_mac,
arm_span,
is_local,
});
}
Failure(token, msg) => match best_failure {
Expand Down Expand Up @@ -544,6 +553,9 @@ pub fn compile_declarative_macro(
lhses,
rhses,
valid,
// Macros defined in the current crate have a real node id,
// whereas macros from an external crate have a dummy id.
is_local: def.id != DUMMY_NODE_ID,
}))
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2799,7 +2799,7 @@ declare_lint! {
/// [issue #79813]: https://github.com/rust-lang/rust/issues/79813
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
Allow,
Warn,
"trailing semicolon in macro body used as expression",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #79813 <https://github.com/rust-lang/rust/issues/79813>",
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ macro_rules! dbg {
// `$val` expression could be a block (`{ .. }`), in which case the `eprintln!`
// will be malformed.
() => {
$crate::eprintln!("[{}:{}]", $crate::file!(), $crate::line!());
$crate::eprintln!("[{}:{}]", $crate::file!(), $crate::line!())
};
($val:expr $(,)?) => {
// Use of `match` here is intentional because it affects the lifetimes
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/hygiene/auxiliary/intercrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub mod foo {
mod bar {
fn f() -> u32 { 1 }
pub macro m() {
f();
f()
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/hygiene/hygienic-label-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ macro_rules! foo {
}

pub fn main() {
'x: loop { foo!() }
'x: loop { foo!(); }
}
4 changes: 2 additions & 2 deletions src/test/ui/hygiene/hygienic-label-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error[E0426]: use of undeclared label `'x`
LL | () => { break 'x; }
| ^^ undeclared label `'x`
...
LL | 'x: loop { foo!() }
| ------ in this macro invocation
LL | 'x: loop { foo!(); }
| ------- in this macro invocation
|
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/hygiene/hygienic-label-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ macro_rules! foo {

pub fn main() {
'x: for _ in 0..1 {
foo!()
foo!();
};
}
4 changes: 2 additions & 2 deletions src/test/ui/hygiene/hygienic-label-3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error[E0426]: use of undeclared label `'x`
LL | () => { break 'x; }
| ^^ undeclared label `'x`
...
LL | foo!()
| ------ in this macro invocation
LL | foo!();
| ------- in this macro invocation
|
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#[macro_export]
macro_rules! my_macro {
() => { true; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// aux-build:foreign-crate.rs
// check-pass

extern crate foreign_crate;

// Test that we do not lint for a macro in a foreign crate
fn main() {
let _ = foreign_crate::my_macro!();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// check-pass
// Ensure that trailing semicolons cause warnings by default

macro_rules! foo {
() => {
true; //~ WARN trailing semicolon in macro
//~| WARN this was previously
}
}

fn main() {
let _val = match true {
true => false,
_ => foo!()
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
warning: trailing semicolon in macro used in expression position
--> $DIR/warn-semicolon-in-expressions-from-macros.rs:6:13
|
LL | true;
| ^
...
LL | _ => foo!()
| ------ in this macro invocation
|
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
= 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 the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: 1 warning emitted

2 changes: 2 additions & 0 deletions src/test/ui/macros/macro-context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ macro_rules! m {
//~| ERROR macro expansion ignores token `;`
//~| ERROR cannot find type `i` in this scope
//~| ERROR cannot find value `i` in this scope
//~| WARN trailing semicolon in macro
//~| WARN this was previously
}

fn main() {
Expand Down
16 changes: 15 additions & 1 deletion src/test/ui/macros/macro-context.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,21 @@ LL | let i = m!();
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 6 previous errors
warning: trailing semicolon in macro used in expression position
--> $DIR/macro-context.rs:3:15
|
LL | () => ( i ; typeof );
| ^
...
LL | let i = m!();
| ---- in this macro invocation
|
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
= 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 the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 6 previous errors; 1 warning emitted

Some errors have detailed explanations: E0412, E0425.
For more information about an error, try `rustc --explain E0412`.
12 changes: 12 additions & 0 deletions src/test/ui/macros/macro-in-expression-context.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
macro_rules! foo {
() => {
assert_eq!("A", "A");
//~^ WARN trailing semicolon in macro
//~| WARN this was previously
//~| NOTE macro invocations at the end of a block
//~| NOTE to ignore the value produced by the macro
//~| NOTE for more information
//~| NOTE `#[warn(semicolon_in_expressions_from_macros)]` on by default
assert_eq!("B", "B");
}
//~^^ ERROR macro expansion ignores token `assert_eq` and any following
Expand All @@ -12,4 +18,10 @@ macro_rules! foo {
fn main() {
foo!();
//~^ NOTE caused by the macro expansion here
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
}
12 changes: 12 additions & 0 deletions src/test/ui/macros/macro-in-expression-context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
macro_rules! foo {
() => {
assert_eq!("A", "A");
//~^ WARN trailing semicolon in macro
//~| WARN this was previously
//~| NOTE macro invocations at the end of a block
//~| NOTE to ignore the value produced by the macro
//~| NOTE for more information
//~| NOTE `#[warn(semicolon_in_expressions_from_macros)]` on by default
assert_eq!("B", "B");
}
//~^^ ERROR macro expansion ignores token `assert_eq` and any following
Expand All @@ -12,4 +18,10 @@ macro_rules! foo {
fn main() {
foo!()
//~^ NOTE caused by the macro expansion here
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
//~| NOTE in this expansion
}
20 changes: 18 additions & 2 deletions src/test/ui/macros/macro-in-expression-context.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: macro expansion ignores token `assert_eq` and any following
--> $DIR/macro-in-expression-context.rs:6:9
--> $DIR/macro-in-expression-context.rs:12:9
|
LL | assert_eq!("B", "B");
| ^^^^^^^^^
Expand All @@ -11,5 +11,21 @@ LL | foo!()
|
= note: the usage of `foo!` is likely invalid in expression context

error: aborting due to previous error
warning: trailing semicolon in macro used in expression position
--> $DIR/macro-in-expression-context.rs:5:29
|
LL | assert_eq!("A", "A");
| ^
...
LL | foo!()
| ------ in this macro invocation
|
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
= 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)

error: aborting due to previous error; 1 warning emitted

2 changes: 1 addition & 1 deletion src/test/ui/proc-macro/nested-nonterminal-tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ macro_rules! wrap {
(first, $e:expr) => { wrap!(second, $e + 1) };
(second, $e:expr) => { wrap!(third, $e + 2) };
(third, $e:expr) => {
print_bang!($e + 3);
print_bang!($e + 3)
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/needless_borrow_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
fn f1(_: &str) {}
macro_rules! m1 {
($e:expr) => {
f1($e);
f1($e)
};
}
macro_rules! m3 {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/ref_binding_to_reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
fn f1(_: &str) {}
macro_rules! m2 {
($e:expr) => {
f1(*$e);
f1(*$e)
};
}
macro_rules! m3 {
Expand Down

0 comments on commit d29e007

Please sign in to comment.