Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SEMICOLON_IN_EXPRESSIONS_FROM_MACROS warn by default #87385

Merged
merged 4 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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