Skip to content

Commit be08bed

Browse files
committed
Implement the unused_macro_rules lint
1 parent a5700cf commit be08bed

File tree

8 files changed

+136
-20
lines changed

8 files changed

+136
-20
lines changed

compiler/rustc_expand/src/base.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,9 @@ pub struct SyntaxExtension {
711711
/// Built-in macros have a couple of special properties like availability
712712
/// in `#[no_implicit_prelude]` modules, so we have to keep this flag.
713713
pub builtin_name: Option<Symbol>,
714+
/// Rule spans. Used for linting. If the macro is not made up of rules,
715+
/// it's empty.
716+
pub rule_spans: Vec<Span>,
714717
}
715718

716719
impl SyntaxExtension {
@@ -740,6 +743,7 @@ impl SyntaxExtension {
740743
edition,
741744
builtin_name: None,
742745
kind,
746+
rule_spans: Vec::new(),
743747
}
744748
}
745749

@@ -753,6 +757,7 @@ impl SyntaxExtension {
753757
edition: Edition,
754758
name: Symbol,
755759
attrs: &[ast::Attribute],
760+
rule_spans: Vec<Span>,
756761
) -> SyntaxExtension {
757762
let allow_internal_unstable =
758763
attr::allow_internal_unstable(sess, &attrs).collect::<Vec<Symbol>>();
@@ -800,6 +805,7 @@ impl SyntaxExtension {
800805
helper_attrs,
801806
edition,
802807
builtin_name,
808+
rule_spans,
803809
}
804810
}
805811

@@ -887,6 +893,8 @@ pub trait ResolverExpand {
887893
force: bool,
888894
) -> Result<Lrc<SyntaxExtension>, Indeterminate>;
889895

896+
fn record_macro_rule_usage(&mut self, mac_id: NodeId, rule_index: usize);
897+
890898
fn check_unused_macros(&mut self);
891899

892900
// Resolver interfaces for specific built-in macros.

compiler/rustc_expand/src/mbe/macro_rules.rs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ impl<'a> ParserAnyMacro<'a> {
156156
}
157157

158158
struct MacroRulesMacroExpander {
159+
id: NodeId,
159160
name: Ident,
160161
span: Span,
161162
transparency: Transparency,
@@ -179,6 +180,7 @@ impl TTMacroExpander for MacroRulesMacroExpander {
179180
cx,
180181
sp,
181182
self.span,
183+
self.id,
182184
self.name,
183185
self.transparency,
184186
input,
@@ -207,6 +209,7 @@ fn generic_extension<'cx, 'tt>(
207209
cx: &'cx mut ExtCtxt<'_>,
208210
sp: Span,
209211
def_span: Span,
212+
id: NodeId,
210213
name: Ident,
211214
transparency: Transparency,
212215
arg: TokenStream,
@@ -297,6 +300,8 @@ fn generic_extension<'cx, 'tt>(
297300
let mut p = Parser::new(sess, tts, false, None);
298301
p.last_type_ascription = cx.current_expansion.prior_type_ascription;
299302

303+
cx.resolver.record_macro_rule_usage(id, i);
304+
300305
// Let the context choose how to interpret the result.
301306
// Weird, but useful for X-macros.
302307
return Box::new(ParserAnyMacro {
@@ -375,7 +380,7 @@ pub fn compile_declarative_macro(
375380
edition: Edition,
376381
) -> SyntaxExtension {
377382
debug!("compile_declarative_macro: {:?}", def);
378-
let mk_syn_ext = |expander| {
383+
let mk_syn_ext = |expander, rule_spans| {
379384
SyntaxExtension::new(
380385
sess,
381386
SyntaxExtensionKind::LegacyBang(expander),
@@ -384,8 +389,10 @@ pub fn compile_declarative_macro(
384389
edition,
385390
def.ident.name,
386391
&def.attrs,
392+
rule_spans,
387393
)
388394
};
395+
let dummy_syn_ext = || mk_syn_ext(Box::new(macro_rules_dummy_expander), Vec::new());
389396

390397
let diag = &sess.parse_sess.span_diagnostic;
391398
let lhs_nm = Ident::new(sym::lhs, def.span);
@@ -446,17 +453,17 @@ pub fn compile_declarative_macro(
446453
let s = parse_failure_msg(&token);
447454
let sp = token.span.substitute_dummy(def.span);
448455
sess.parse_sess.span_diagnostic.struct_span_err(sp, &s).span_label(sp, msg).emit();
449-
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
456+
return dummy_syn_ext();
450457
}
451458
Error(sp, msg) => {
452459
sess.parse_sess
453460
.span_diagnostic
454461
.struct_span_err(sp.substitute_dummy(def.span), &msg)
455462
.emit();
456-
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
463+
return dummy_syn_ext();
457464
}
458465
ErrorReported => {
459-
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
466+
return dummy_syn_ext();
460467
}
461468
};
462469

@@ -531,6 +538,11 @@ pub fn compile_declarative_macro(
531538
None => {}
532539
}
533540

541+
// Compute the spans of the macro rules
542+
// We only take the span of the lhs here,
543+
// so that the spans of created warnings are smaller.
544+
let rule_spans = lhses.iter().map(|lhs| lhs.span()).collect::<Vec<_>>();
545+
534546
// Convert the lhses into `MatcherLoc` form, which is better for doing the
535547
// actual matching. Unless the matcher is invalid.
536548
let lhses = if valid {
@@ -550,17 +562,21 @@ pub fn compile_declarative_macro(
550562
vec![]
551563
};
552564

553-
mk_syn_ext(Box::new(MacroRulesMacroExpander {
554-
name: def.ident,
555-
span: def.span,
556-
transparency,
557-
lhses,
558-
rhses,
559-
valid,
560-
// Macros defined in the current crate have a real node id,
561-
// whereas macros from an external crate have a dummy id.
562-
is_local: def.id != DUMMY_NODE_ID,
563-
}))
565+
mk_syn_ext(
566+
Box::new(MacroRulesMacroExpander {
567+
name: def.ident,
568+
span: def.span,
569+
id: def.id,
570+
transparency,
571+
lhses,
572+
rhses,
573+
valid,
574+
// Macros defined in the current crate have a real node id,
575+
// whereas macros from an external crate have a dummy id.
576+
is_local: def.id != DUMMY_NODE_ID,
577+
}),
578+
rule_spans,
579+
)
564580
}
565581

566582
fn check_lhs_nt_follows(sess: &ParseSess, def: &ast::Item, lhs: &mbe::TokenTree) -> bool {

compiler/rustc_metadata/src/rmeta/decoder.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
932932
self.root.edition,
933933
Symbol::intern(name),
934934
&attrs,
935+
Vec::new(),
935936
)
936937
}
937938

compiler/rustc_resolve/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ version = "0.0.0"
44
edition = "2021"
55

66
[lib]
7-
test = false
87
doctest = false
98

109
[dependencies]

compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,9 +1218,18 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12181218
// Mark the given macro as unused unless its name starts with `_`.
12191219
// Macro uses will remove items from this set, and the remaining
12201220
// items will be reported as `unused_macros`.
1221-
fn insert_unused_macro(&mut self, ident: Ident, def_id: LocalDefId, node_id: NodeId) {
1221+
fn insert_unused_macro(
1222+
&mut self,
1223+
ident: Ident,
1224+
def_id: LocalDefId,
1225+
node_id: NodeId,
1226+
rule_count: usize,
1227+
) {
12221228
if !ident.as_str().starts_with('_') {
12231229
self.r.unused_macros.insert(def_id, (node_id, ident));
1230+
for rule_i in 0..rule_count {
1231+
self.r.unused_macro_rules.insert((def_id, rule_i), (node_id, ident));
1232+
}
12241233
}
12251234
}
12261235

@@ -1244,6 +1253,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12441253
};
12451254

12461255
let res = Res::Def(DefKind::Macro(ext.macro_kind()), def_id.to_def_id());
1256+
let rule_count = ext.rule_spans.len();
12471257
self.r.macro_map.insert(def_id.to_def_id(), ext);
12481258
self.r.local_macro_def_scopes.insert(def_id, parent_scope.module);
12491259

@@ -1264,7 +1274,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12641274
self.r.define(module, ident, MacroNS, (res, vis, span, expansion, IsMacroExport));
12651275
} else {
12661276
self.r.check_reserved_macro_name(ident, res);
1267-
self.insert_unused_macro(ident, def_id, item.id);
1277+
self.insert_unused_macro(ident, def_id, item.id, rule_count);
12681278
}
12691279
self.r.visibilities.insert(def_id, vis);
12701280
self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
@@ -1285,7 +1295,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12851295
_ => self.resolve_visibility(&item.vis),
12861296
};
12871297
if vis != ty::Visibility::Public {
1288-
self.insert_unused_macro(ident, def_id, item.id);
1298+
self.insert_unused_macro(ident, def_id, item.id, rule_count);
12891299
}
12901300
self.r.define(module, ident, MacroNS, (res, vis, span, expansion));
12911301
self.r.visibilities.insert(def_id, vis);

compiler/rustc_resolve/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,7 @@ pub struct Resolver<'a> {
969969
local_macro_def_scopes: FxHashMap<LocalDefId, Module<'a>>,
970970
ast_transform_scopes: FxHashMap<LocalExpnId, Module<'a>>,
971971
unused_macros: FxHashMap<LocalDefId, (NodeId, Ident)>,
972+
unused_macro_rules: FxHashMap<(LocalDefId, usize), (NodeId, Ident)>,
972973
proc_macro_stubs: FxHashSet<LocalDefId>,
973974
/// Traces collected during macro resolution and validated when it's complete.
974975
single_segment_macro_resolutions:
@@ -1354,6 +1355,7 @@ impl<'a> Resolver<'a> {
13541355
potentially_unused_imports: Vec::new(),
13551356
struct_constructors: Default::default(),
13561357
unused_macros: Default::default(),
1358+
unused_macro_rules: Default::default(),
13571359
proc_macro_stubs: Default::default(),
13581360
single_segment_macro_resolutions: Default::default(),
13591361
multi_segment_macro_resolutions: Default::default(),

compiler/rustc_resolve/src/macros.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ use rustc_hir::def::{self, DefKind, NonMacroAttrKind};
2222
use rustc_hir::def_id::{CrateNum, LocalDefId};
2323
use rustc_middle::middle::stability;
2424
use rustc_middle::ty::RegisteredTools;
25-
use rustc_session::lint::builtin::{LEGACY_DERIVE_HELPERS, SOFT_UNSTABLE, UNUSED_MACROS};
25+
use rustc_session::lint::builtin::{
26+
LEGACY_DERIVE_HELPERS, SOFT_UNSTABLE, UNUSED_MACROS, UNUSED_MACRO_RULES,
27+
};
2628
use rustc_session::lint::BuiltinLintDiagnostics;
2729
use rustc_session::parse::feature_err;
2830
use rustc_session::Session;
@@ -36,6 +38,9 @@ use std::mem;
3638

3739
type Res = def::Res<NodeId>;
3840

41+
#[cfg(test)]
42+
mod tests;
43+
3944
/// Binding produced by a `macro_rules` item.
4045
/// Not modularized, can shadow previous `macro_rules` bindings, etc.
4146
#[derive(Debug)]
@@ -311,6 +316,12 @@ impl<'a> ResolverExpand for Resolver<'a> {
311316
Ok(ext)
312317
}
313318

319+
fn record_macro_rule_usage(&mut self, id: NodeId, rule_i: usize) {
320+
if let Some(did) = self.opt_local_def_id(id) {
321+
self.unused_macro_rules.remove(&(did, rule_i));
322+
}
323+
}
324+
314325
fn check_unused_macros(&mut self) {
315326
for (_, &(node_id, ident)) in self.unused_macros.iter() {
316327
self.lint_buffer.buffer_lint(
@@ -320,6 +331,24 @@ impl<'a> ResolverExpand for Resolver<'a> {
320331
&format!("unused macro definition: `{}`", ident.as_str()),
321332
);
322333
}
334+
for (&(def_id, arm_i), &(node_id, ident)) in self.unused_macro_rules.iter() {
335+
if self.unused_macros.contains_key(&def_id) {
336+
// We already lint the entire macro as unused
337+
continue;
338+
}
339+
let ext = &self.macro_map[&def_id.to_def_id()];
340+
let rule_span = ext.rule_spans[arm_i];
341+
self.lint_buffer.buffer_lint(
342+
UNUSED_MACRO_RULES,
343+
node_id,
344+
rule_span,
345+
&format!(
346+
"{} rule of macro `{}` is never used",
347+
ordinalize(arm_i + 1),
348+
ident.as_str()
349+
),
350+
);
351+
}
323352
}
324353

325354
fn has_derive_copy(&self, expn_id: LocalExpnId) -> bool {
@@ -877,3 +906,14 @@ impl<'a> Resolver<'a> {
877906
result
878907
}
879908
}
909+
910+
/// Convert the given number into the corresponding ordinal
911+
fn ordinalize(v: usize) -> String {
912+
let suffix = match ((11..=13).contains(&(v % 100)), v % 10) {
913+
(false, 1) => "st",
914+
(false, 2) => "nd",
915+
(false, 3) => "rd",
916+
_ => "th",
917+
};
918+
format!("{v}{suffix}")
919+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use super::ordinalize;
2+
3+
#[test]
4+
fn test_ordinalize() {
5+
assert_eq!(ordinalize(1), "1st");
6+
assert_eq!(ordinalize(2), "2nd");
7+
assert_eq!(ordinalize(3), "3rd");
8+
assert_eq!(ordinalize(4), "4th");
9+
assert_eq!(ordinalize(5), "5th");
10+
// ...
11+
assert_eq!(ordinalize(10), "10th");
12+
assert_eq!(ordinalize(11), "11th");
13+
assert_eq!(ordinalize(12), "12th");
14+
assert_eq!(ordinalize(13), "13th");
15+
assert_eq!(ordinalize(14), "14th");
16+
// ...
17+
assert_eq!(ordinalize(20), "20th");
18+
assert_eq!(ordinalize(21), "21st");
19+
assert_eq!(ordinalize(22), "22nd");
20+
assert_eq!(ordinalize(23), "23rd");
21+
assert_eq!(ordinalize(24), "24th");
22+
// ...
23+
assert_eq!(ordinalize(30), "30th");
24+
assert_eq!(ordinalize(31), "31st");
25+
assert_eq!(ordinalize(32), "32nd");
26+
assert_eq!(ordinalize(33), "33rd");
27+
assert_eq!(ordinalize(34), "34th");
28+
// ...
29+
assert_eq!(ordinalize(7010), "7010th");
30+
assert_eq!(ordinalize(7011), "7011th");
31+
assert_eq!(ordinalize(7012), "7012th");
32+
assert_eq!(ordinalize(7013), "7013th");
33+
assert_eq!(ordinalize(7014), "7014th");
34+
// ...
35+
assert_eq!(ordinalize(7020), "7020th");
36+
assert_eq!(ordinalize(7021), "7021st");
37+
assert_eq!(ordinalize(7022), "7022nd");
38+
assert_eq!(ordinalize(7023), "7023rd");
39+
assert_eq!(ordinalize(7024), "7024th");
40+
}

0 commit comments

Comments
 (0)