Skip to content

Commit f6bc738

Browse files
authored
Rollup merge of #87385 - Aaron1011:final-enable-semi, r=petrochenkov
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.
2 parents 9391d55 + e70ce57 commit f6bc738

21 files changed

+135
-37
lines changed

Diff for: compiler/rustc_expand/src/mbe/macro_rules.rs

+19-7
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ crate struct ParserAnyMacro<'a> {
4545
lint_node_id: NodeId,
4646
is_trailing_mac: bool,
4747
arm_span: Span,
48+
/// Whether or not this macro is defined in the current crate
49+
is_local: bool,
4850
}
4951

5052
crate fn annotate_err_with_kind(
@@ -124,6 +126,7 @@ impl<'a> ParserAnyMacro<'a> {
124126
lint_node_id,
125127
arm_span,
126128
is_trailing_mac,
129+
is_local,
127130
} = *self;
128131
let snapshot = &mut parser.clone();
129132
let fragment = match parse_ast_fragment(parser, kind) {
@@ -138,13 +141,15 @@ impl<'a> ParserAnyMacro<'a> {
138141
// `macro_rules! m { () => { panic!(); } }` isn't parsed by `.parse_expr()`,
139142
// but `m!()` is allowed in expression positions (cf. issue #34706).
140143
if kind == AstFragmentKind::Expr && parser.token == token::Semi {
141-
parser.sess.buffer_lint_with_diagnostic(
142-
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
143-
parser.token.span,
144-
lint_node_id,
145-
"trailing semicolon in macro used in expression position",
146-
BuiltinLintDiagnostics::TrailingMacro(is_trailing_mac, macro_ident),
147-
);
144+
if is_local {
145+
parser.sess.buffer_lint_with_diagnostic(
146+
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
147+
parser.token.span,
148+
lint_node_id,
149+
"trailing semicolon in macro used in expression position",
150+
BuiltinLintDiagnostics::TrailingMacro(is_trailing_mac, macro_ident),
151+
);
152+
}
148153
parser.bump();
149154
}
150155

@@ -162,6 +167,7 @@ struct MacroRulesMacroExpander {
162167
lhses: Vec<mbe::TokenTree>,
163168
rhses: Vec<mbe::TokenTree>,
164169
valid: bool,
170+
is_local: bool,
165171
}
166172

167173
impl TTMacroExpander for MacroRulesMacroExpander {
@@ -183,6 +189,7 @@ impl TTMacroExpander for MacroRulesMacroExpander {
183189
input,
184190
&self.lhses,
185191
&self.rhses,
192+
self.is_local,
186193
)
187194
}
188195
}
@@ -210,6 +217,7 @@ fn generic_extension<'cx>(
210217
arg: TokenStream,
211218
lhses: &[mbe::TokenTree],
212219
rhses: &[mbe::TokenTree],
220+
is_local: bool,
213221
) -> Box<dyn MacResult + 'cx> {
214222
let sess = &cx.sess.parse_sess;
215223

@@ -311,6 +319,7 @@ fn generic_extension<'cx>(
311319
lint_node_id: cx.current_expansion.lint_node_id,
312320
is_trailing_mac: cx.current_expansion.is_trailing_mac,
313321
arm_span,
322+
is_local,
314323
});
315324
}
316325
Failure(token, msg) => match best_failure {
@@ -544,6 +553,9 @@ pub fn compile_declarative_macro(
544553
lhses,
545554
rhses,
546555
valid,
556+
// Macros defined in the current crate have a real node id,
557+
// whereas macros from an external crate have a dummy id.
558+
is_local: def.id != DUMMY_NODE_ID,
547559
}))
548560
}
549561

Diff for: compiler/rustc_lint_defs/src/builtin.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2799,7 +2799,7 @@ declare_lint! {
27992799
/// [issue #79813]: https://github.com/rust-lang/rust/issues/79813
28002800
/// [future-incompatible]: ../index.md#future-incompatible-lints
28012801
pub SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
2802-
Allow,
2802+
Warn,
28032803
"trailing semicolon in macro body used as expression",
28042804
@future_incompatible = FutureIncompatibleInfo {
28052805
reference: "issue #79813 <https://github.com/rust-lang/rust/issues/79813>",

Diff for: library/std/src/macros.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ macro_rules! dbg {
290290
// `$val` expression could be a block (`{ .. }`), in which case the `eprintln!`
291291
// will be malformed.
292292
() => {
293-
$crate::eprintln!("[{}:{}]", $crate::file!(), $crate::line!());
293+
$crate::eprintln!("[{}:{}]", $crate::file!(), $crate::line!())
294294
};
295295
($val:expr $(,)?) => {
296296
// Use of `match` here is intentional because it affects the lifetimes

Diff for: src/test/ui/hygiene/auxiliary/intercrate.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ pub mod foo {
55
mod bar {
66
fn f() -> u32 { 1 }
77
pub macro m() {
8-
f();
8+
f()
99
}
1010
}
1111
}

Diff for: src/test/ui/hygiene/hygienic-label-1.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ macro_rules! foo {
33
}
44

55
pub fn main() {
6-
'x: loop { foo!() }
6+
'x: loop { foo!(); }
77
}

Diff for: src/test/ui/hygiene/hygienic-label-1.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ error[E0426]: use of undeclared label `'x`
44
LL | () => { break 'x; }
55
| ^^ undeclared label `'x`
66
...
7-
LL | 'x: loop { foo!() }
8-
| ------ in this macro invocation
7+
LL | 'x: loop { foo!(); }
8+
| ------- in this macro invocation
99
|
1010
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
1111

Diff for: src/test/ui/hygiene/hygienic-label-3.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ macro_rules! foo {
44

55
pub fn main() {
66
'x: for _ in 0..1 {
7-
foo!()
7+
foo!();
88
};
99
}

Diff for: src/test/ui/hygiene/hygienic-label-3.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ error[E0426]: use of undeclared label `'x`
44
LL | () => { break 'x; }
55
| ^^ undeclared label `'x`
66
...
7-
LL | foo!()
8-
| ------ in this macro invocation
7+
LL | foo!();
8+
| ------- in this macro invocation
99
|
1010
= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
1111

Diff for: src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs

-15
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#[macro_export]
2+
macro_rules! my_macro {
3+
() => { true; }
4+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// aux-build:foreign-crate.rs
2+
// check-pass
3+
4+
extern crate foreign_crate;
5+
6+
// Test that we do not lint for a macro in a foreign crate
7+
fn main() {
8+
let _ = foreign_crate::my_macro!();
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// check-pass
2+
// Ensure that trailing semicolons cause warnings by default
3+
4+
macro_rules! foo {
5+
() => {
6+
true; //~ WARN trailing semicolon in macro
7+
//~| WARN this was previously
8+
}
9+
}
10+
11+
fn main() {
12+
let _val = match true {
13+
true => false,
14+
_ => foo!()
15+
};
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
warning: trailing semicolon in macro used in expression position
2+
--> $DIR/warn-semicolon-in-expressions-from-macros.rs:6:13
3+
|
4+
LL | true;
5+
| ^
6+
...
7+
LL | _ => foo!()
8+
| ------ in this macro invocation
9+
|
10+
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
11+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
12+
= note: for more information, see issue #79813 <https://github.com/rust-lang/rust/issues/79813>
13+
= note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
14+
15+
warning: 1 warning emitted
16+

Diff for: src/test/ui/macros/macro-context.rs

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ macro_rules! m {
66
//~| ERROR macro expansion ignores token `;`
77
//~| ERROR cannot find type `i` in this scope
88
//~| ERROR cannot find value `i` in this scope
9+
//~| WARN trailing semicolon in macro
10+
//~| WARN this was previously
911
}
1012

1113
fn main() {

Diff for: src/test/ui/macros/macro-context.stderr

+15-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,21 @@ LL | let i = m!();
6464
|
6565
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
6666

67-
error: aborting due to 6 previous errors
67+
warning: trailing semicolon in macro used in expression position
68+
--> $DIR/macro-context.rs:3:15
69+
|
70+
LL | () => ( i ; typeof );
71+
| ^
72+
...
73+
LL | let i = m!();
74+
| ---- in this macro invocation
75+
|
76+
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
77+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
78+
= note: for more information, see issue #79813 <https://github.com/rust-lang/rust/issues/79813>
79+
= note: this warning originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
80+
81+
error: aborting due to 6 previous errors; 1 warning emitted
6882

6983
Some errors have detailed explanations: E0412, E0425.
7084
For more information about an error, try `rustc --explain E0412`.

Diff for: src/test/ui/macros/macro-in-expression-context.fixed

+12
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
macro_rules! foo {
44
() => {
55
assert_eq!("A", "A");
6+
//~^ WARN trailing semicolon in macro
7+
//~| WARN this was previously
8+
//~| NOTE macro invocations at the end of a block
9+
//~| NOTE to ignore the value produced by the macro
10+
//~| NOTE for more information
11+
//~| NOTE `#[warn(semicolon_in_expressions_from_macros)]` on by default
612
assert_eq!("B", "B");
713
}
814
//~^^ ERROR macro expansion ignores token `assert_eq` and any following
@@ -12,4 +18,10 @@ macro_rules! foo {
1218
fn main() {
1319
foo!();
1420
//~^ NOTE caused by the macro expansion here
21+
//~| NOTE in this expansion
22+
//~| NOTE in this expansion
23+
//~| NOTE in this expansion
24+
//~| NOTE in this expansion
25+
//~| NOTE in this expansion
26+
//~| NOTE in this expansion
1527
}

Diff for: src/test/ui/macros/macro-in-expression-context.rs

+12
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
macro_rules! foo {
44
() => {
55
assert_eq!("A", "A");
6+
//~^ WARN trailing semicolon in macro
7+
//~| WARN this was previously
8+
//~| NOTE macro invocations at the end of a block
9+
//~| NOTE to ignore the value produced by the macro
10+
//~| NOTE for more information
11+
//~| NOTE `#[warn(semicolon_in_expressions_from_macros)]` on by default
612
assert_eq!("B", "B");
713
}
814
//~^^ ERROR macro expansion ignores token `assert_eq` and any following
@@ -12,4 +18,10 @@ macro_rules! foo {
1218
fn main() {
1319
foo!()
1420
//~^ NOTE caused by the macro expansion here
21+
//~| NOTE in this expansion
22+
//~| NOTE in this expansion
23+
//~| NOTE in this expansion
24+
//~| NOTE in this expansion
25+
//~| NOTE in this expansion
26+
//~| NOTE in this expansion
1527
}

Diff for: src/test/ui/macros/macro-in-expression-context.stderr

+18-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: macro expansion ignores token `assert_eq` and any following
2-
--> $DIR/macro-in-expression-context.rs:6:9
2+
--> $DIR/macro-in-expression-context.rs:12:9
33
|
44
LL | assert_eq!("B", "B");
55
| ^^^^^^^^^
@@ -11,5 +11,21 @@ LL | foo!()
1111
|
1212
= note: the usage of `foo!` is likely invalid in expression context
1313

14-
error: aborting due to previous error
14+
warning: trailing semicolon in macro used in expression position
15+
--> $DIR/macro-in-expression-context.rs:5:29
16+
|
17+
LL | assert_eq!("A", "A");
18+
| ^
19+
...
20+
LL | foo!()
21+
| ------ in this macro invocation
22+
|
23+
= note: `#[warn(semicolon_in_expressions_from_macros)]` on by default
24+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
25+
= note: for more information, see issue #79813 <https://github.com/rust-lang/rust/issues/79813>
26+
= note: macro invocations at the end of a block are treated as expressions
27+
= note: to ignore the value produced by the macro, add a semicolon after the invocation of `foo`
28+
= note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
29+
30+
error: aborting due to previous error; 1 warning emitted
1531

Diff for: src/test/ui/proc-macro/nested-nonterminal-tokens.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ macro_rules! wrap {
1717
(first, $e:expr) => { wrap!(second, $e + 1) };
1818
(second, $e:expr) => { wrap!(third, $e + 2) };
1919
(third, $e:expr) => {
20-
print_bang!($e + 3);
20+
print_bang!($e + 3)
2121
};
2222
}
2323

Diff for: src/tools/clippy/tests/ui/needless_borrow_pat.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
fn f1(_: &str) {}
88
macro_rules! m1 {
99
($e:expr) => {
10-
f1($e);
10+
f1($e)
1111
};
1212
}
1313
macro_rules! m3 {

Diff for: src/tools/clippy/tests/ui/ref_binding_to_reference.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
fn f1(_: &str) {}
88
macro_rules! m2 {
99
($e:expr) => {
10-
f1(*$e);
10+
f1(*$e)
1111
};
1212
}
1313
macro_rules! m3 {

0 commit comments

Comments
 (0)