Skip to content

Commit 4382cef

Browse files
authored
Rollup merge of rust-lang#97903 - est31:unused_macro_rules_compile_error, r=petrochenkov
Never regard macro rules with compile_error! invocations as unused The very point of compile_error! is to never be reached, and one of the use cases of the macro, currently also listed as examples in the documentation of compile_error, is to create nicer errors for wrong macro invocations. Thus, we should never warn about unused macro arms that contain invocations of compile_error. See also rust-lang#96150 (comment) and the discussion after that. Furthermore, the PR also contains two commits to silence `unused_macro_rules` when a macro has an invalid rule, and to add a test that `unused_macros` does not behave badly in the same situation. r? `@petrochenkov` as I've talked to them about this
2 parents ec55c61 + 787e24c commit 4382cef

10 files changed

+158
-10
lines changed

compiler/rustc_expand/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![allow(rustc::potential_query_instability)]
2+
#![feature(array_windows)]
23
#![feature(associated_type_bounds)]
34
#![feature(associated_type_defaults)]
45
#![feature(if_let_guard)]

compiler/rustc_expand/src/mbe/macro_rules.rs

+40-6
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ pub fn compile_declarative_macro(
380380
features: &Features,
381381
def: &ast::Item,
382382
edition: Edition,
383-
) -> (SyntaxExtension, Vec<Span>) {
383+
) -> (SyntaxExtension, Vec<(usize, Span)>) {
384384
debug!("compile_declarative_macro: {:?}", def);
385385
let mk_syn_ext = |expander| {
386386
SyntaxExtension::new(
@@ -539,11 +539,22 @@ pub fn compile_declarative_macro(
539539
None => {}
540540
}
541541

542-
// Compute the spans of the macro rules
543-
// We only take the span of the lhs here,
544-
// so that the spans of created warnings are smaller.
545-
let rule_spans = if def.id != DUMMY_NODE_ID {
546-
lhses.iter().map(|lhs| lhs.span()).collect::<Vec<_>>()
542+
// Compute the spans of the macro rules for unused rule linting.
543+
// To avoid warning noise, only consider the rules of this
544+
// macro for the lint, if all rules are valid.
545+
// Also, we are only interested in non-foreign macros.
546+
let rule_spans = if valid && def.id != DUMMY_NODE_ID {
547+
lhses
548+
.iter()
549+
.zip(rhses.iter())
550+
.enumerate()
551+
// If the rhs contains an invocation like compile_error!,
552+
// don't consider the rule for the unused rule lint.
553+
.filter(|(_idx, (_lhs, rhs))| !has_compile_error_macro(rhs))
554+
// We only take the span of the lhs here,
555+
// so that the spans of created warnings are smaller.
556+
.map(|(idx, (lhs, _rhs))| (idx, lhs.span()))
557+
.collect::<Vec<_>>()
547558
} else {
548559
Vec::new()
549560
};
@@ -651,6 +662,29 @@ fn check_matcher(sess: &ParseSess, def: &ast::Item, matcher: &[mbe::TokenTree])
651662
err == sess.span_diagnostic.err_count()
652663
}
653664

665+
fn has_compile_error_macro(rhs: &mbe::TokenTree) -> bool {
666+
match rhs {
667+
mbe::TokenTree::Delimited(_sp, d) => {
668+
let has_compile_error = d.tts.array_windows::<3>().any(|[ident, bang, args]| {
669+
if let mbe::TokenTree::Token(ident) = ident &&
670+
let TokenKind::Ident(ident, _) = ident.kind &&
671+
ident == sym::compile_error &&
672+
let mbe::TokenTree::Token(bang) = bang &&
673+
let TokenKind::Not = bang.kind &&
674+
let mbe::TokenTree::Delimited(_, del) = args &&
675+
del.delim != Delimiter::Invisible
676+
{
677+
true
678+
} else {
679+
false
680+
}
681+
});
682+
if has_compile_error { true } else { d.tts.iter().any(has_compile_error_macro) }
683+
}
684+
_ => false,
685+
}
686+
}
687+
654688
// `The FirstSets` for a matcher is a mapping from subsequences in the
655689
// matcher to the FIRST set for that subsequence.
656690
//

compiler/rustc_resolve/src/build_reduced_graph.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1220,12 +1220,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12201220
ident: Ident,
12211221
def_id: LocalDefId,
12221222
node_id: NodeId,
1223-
rule_spans: &[Span],
1223+
rule_spans: &[(usize, Span)],
12241224
) {
12251225
if !ident.as_str().starts_with('_') {
12261226
self.r.unused_macros.insert(def_id, (node_id, ident));
1227-
for (rule_i, rule_span) in rule_spans.iter().enumerate() {
1228-
self.r.unused_macro_rules.insert((def_id, rule_i), (ident, *rule_span));
1227+
for (rule_i, rule_span) in rule_spans.iter() {
1228+
self.r.unused_macro_rules.insert((def_id, *rule_i), (ident, *rule_span));
12291229
}
12301230
}
12311231
}

compiler/rustc_resolve/src/macros.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ impl<'a> Resolver<'a> {
870870
&mut self,
871871
item: &ast::Item,
872872
edition: Edition,
873-
) -> (SyntaxExtension, Vec<Span>) {
873+
) -> (SyntaxExtension, Vec<(usize, Span)>) {
874874
let (mut result, mut rule_spans) = compile_declarative_macro(
875875
&self.session,
876876
self.session.features_untracked(),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![deny(unused_macro_rules)]
2+
// To make sure we are not hitting this
3+
#![deny(unused_macros)]
4+
5+
macro_rules! num {
6+
(one) => { 1 };
7+
// Most simple (and common) case
8+
(two) => { compile_error!("foo"); };
9+
// Some nested use
10+
(two_) => { foo(compile_error!("foo")); };
11+
(three) => { 3 };
12+
(four) => { 4 }; //~ ERROR: rule of macro
13+
}
14+
const _NUM: u8 = num!(one) + num!(three);
15+
16+
// compile_error not used as a macro invocation
17+
macro_rules! num2 {
18+
(one) => { 1 };
19+
// Only identifier present
20+
(two) => { fn compile_error() {} }; //~ ERROR: rule of macro
21+
// Only identifier and bang present
22+
(two_) => { compile_error! }; //~ ERROR: rule of macro
23+
(three) => { 3 };
24+
}
25+
const _NUM2: u8 = num2!(one) + num2!(three);
26+
27+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: 5th rule of macro `num` is never used
2+
--> $DIR/unused-macro-rules-compile-error.rs:12:5
3+
|
4+
LL | (four) => { 4 };
5+
| ^^^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/unused-macro-rules-compile-error.rs:1:9
9+
|
10+
LL | #![deny(unused_macro_rules)]
11+
| ^^^^^^^^^^^^^^^^^^
12+
13+
error: 3rd rule of macro `num2` is never used
14+
--> $DIR/unused-macro-rules-compile-error.rs:22:5
15+
|
16+
LL | (two_) => { compile_error! };
17+
| ^^^^^^
18+
19+
error: 2nd rule of macro `num2` is never used
20+
--> $DIR/unused-macro-rules-compile-error.rs:20:5
21+
|
22+
LL | (two) => { fn compile_error() {} };
23+
| ^^^^^
24+
25+
error: aborting due to 3 previous errors
26+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#![deny(unused_macro_rules)]
2+
3+
macro_rules! foo {
4+
(v) => {};
5+
(w) => {};
6+
() => 0; //~ ERROR: macro rhs must be delimited
7+
}
8+
9+
fn main() {
10+
foo!(v);
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: macro rhs must be delimited
2+
--> $DIR/unused-macro-rules-malformed-rule.rs:6:11
3+
|
4+
LL | () => 0;
5+
| ^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#![deny(unused_macros)]
2+
3+
macro_rules! foo { //~ ERROR: unused macro definition
4+
(v) => {};
5+
() => 0; //~ ERROR: macro rhs must be delimited
6+
}
7+
8+
macro_rules! bar {
9+
(v) => {};
10+
() => 0; //~ ERROR: macro rhs must be delimited
11+
}
12+
13+
fn main() {
14+
bar!(v);
15+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: macro rhs must be delimited
2+
--> $DIR/unused-macros-malformed-rule.rs:5:11
3+
|
4+
LL | () => 0;
5+
| ^
6+
7+
error: macro rhs must be delimited
8+
--> $DIR/unused-macros-malformed-rule.rs:10:11
9+
|
10+
LL | () => 0;
11+
| ^
12+
13+
error: unused macro definition: `foo`
14+
--> $DIR/unused-macros-malformed-rule.rs:3:14
15+
|
16+
LL | macro_rules! foo {
17+
| ^^^
18+
|
19+
note: the lint level is defined here
20+
--> $DIR/unused-macros-malformed-rule.rs:1:9
21+
|
22+
LL | #![deny(unused_macros)]
23+
| ^^^^^^^^^^^^^
24+
25+
error: aborting due to 3 previous errors
26+

0 commit comments

Comments
 (0)