Skip to content

Commit 9ac8200

Browse files
Rollup merge of rust-lang#96150 - est31:unused_macro_rules, r=petrochenkov
Implement a lint to warn about unused macro rules This implements a new lint to warn about unused macro rules (arms/matchers), similar to the `unused_macros` lint added by rust-lang#41907 that warns about entire macros. ```rust macro_rules! unused_empty { (hello) => { println!("Hello, world!") }; () => { println!("empty") }; //~ ERROR: 1st rule of macro `unused_empty` is never used } fn main() { unused_empty!(hello); } ``` Builds upon rust-lang#96149 and rust-lang#96156. Fixes rust-lang#73576
2 parents b46024a + 3989f02 commit 9ac8200

33 files changed

+399
-77
lines changed

compiler/rustc_codegen_llvm/src/intrinsic.rs

+2
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
816816
span: Span,
817817
) -> Result<&'ll Value, ()> {
818818
// macros for error handling:
819+
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
819820
macro_rules! emit_error {
820821
($msg: tt) => {
821822
emit_error!($msg, )
@@ -1144,6 +1145,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
11441145
span: Span,
11451146
args: &[OperandRef<'tcx, &'ll Value>],
11461147
) -> Result<&'ll Value, ()> {
1148+
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
11471149
macro_rules! emit_error {
11481150
($msg: tt) => {
11491151
emit_error!($msg, )

compiler/rustc_expand/src/base.rs

+2
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,8 @@ pub trait ResolverExpand {
887887
force: bool,
888888
) -> Result<Lrc<SyntaxExtension>, Indeterminate>;
889889

890+
fn record_macro_rule_usage(&mut self, mac_id: NodeId, rule_index: usize);
891+
890892
fn check_unused_macros(&mut self);
891893

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

compiler/rustc_expand/src/mbe/macro_rules.rs

+28-12
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,13 @@ impl<'a> ParserAnyMacro<'a> {
156156
}
157157

158158
struct MacroRulesMacroExpander {
159+
node_id: NodeId,
159160
name: Ident,
160161
span: Span,
161162
transparency: Transparency,
162163
lhses: Vec<Vec<MatcherLoc>>,
163164
rhses: Vec<mbe::TokenTree>,
164165
valid: bool,
165-
is_local: bool,
166166
}
167167

168168
impl TTMacroExpander for MacroRulesMacroExpander {
@@ -179,12 +179,12 @@ impl TTMacroExpander for MacroRulesMacroExpander {
179179
cx,
180180
sp,
181181
self.span,
182+
self.node_id,
182183
self.name,
183184
self.transparency,
184185
input,
185186
&self.lhses,
186187
&self.rhses,
187-
self.is_local,
188188
)
189189
}
190190
}
@@ -207,14 +207,17 @@ fn generic_extension<'cx, 'tt>(
207207
cx: &'cx mut ExtCtxt<'_>,
208208
sp: Span,
209209
def_span: Span,
210+
node_id: NodeId,
210211
name: Ident,
211212
transparency: Transparency,
212213
arg: TokenStream,
213214
lhses: &'tt [Vec<MatcherLoc>],
214215
rhses: &'tt [mbe::TokenTree],
215-
is_local: bool,
216216
) -> Box<dyn MacResult + 'cx> {
217217
let sess = &cx.sess.parse_sess;
218+
// Macros defined in the current crate have a real node id,
219+
// whereas macros from an external crate have a dummy id.
220+
let is_local = node_id != DUMMY_NODE_ID;
218221

219222
if cx.trace_macros() {
220223
let msg = format!("expanding `{}! {{ {} }}`", name, pprust::tts_to_string(&arg));
@@ -296,6 +299,10 @@ fn generic_extension<'cx, 'tt>(
296299
let mut p = Parser::new(sess, tts, false, None);
297300
p.last_type_ascription = cx.current_expansion.prior_type_ascription;
298301

302+
if is_local {
303+
cx.resolver.record_macro_rule_usage(node_id, i);
304+
}
305+
299306
// Let the context choose how to interpret the result.
300307
// Weird, but useful for X-macros.
301308
return Box::new(ParserAnyMacro {
@@ -372,7 +379,7 @@ pub fn compile_declarative_macro(
372379
features: &Features,
373380
def: &ast::Item,
374381
edition: Edition,
375-
) -> SyntaxExtension {
382+
) -> (SyntaxExtension, Vec<Span>) {
376383
debug!("compile_declarative_macro: {:?}", def);
377384
let mk_syn_ext = |expander| {
378385
SyntaxExtension::new(
@@ -385,6 +392,7 @@ pub fn compile_declarative_macro(
385392
&def.attrs,
386393
)
387394
};
395+
let dummy_syn_ext = || (mk_syn_ext(Box::new(macro_rules_dummy_expander)), Vec::new());
388396

389397
let diag = &sess.parse_sess.span_diagnostic;
390398
let lhs_nm = Ident::new(sym::lhs, def.span);
@@ -445,17 +453,17 @@ pub fn compile_declarative_macro(
445453
let s = parse_failure_msg(&token);
446454
let sp = token.span.substitute_dummy(def.span);
447455
sess.parse_sess.span_diagnostic.struct_span_err(sp, &s).span_label(sp, msg).emit();
448-
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
456+
return dummy_syn_ext();
449457
}
450458
Error(sp, msg) => {
451459
sess.parse_sess
452460
.span_diagnostic
453461
.struct_span_err(sp.substitute_dummy(def.span), &msg)
454462
.emit();
455-
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
463+
return dummy_syn_ext();
456464
}
457465
ErrorReported => {
458-
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
466+
return dummy_syn_ext();
459467
}
460468
};
461469

@@ -530,6 +538,15 @@ pub fn compile_declarative_macro(
530538
None => {}
531539
}
532540

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 = if def.id != DUMMY_NODE_ID {
545+
lhses.iter().map(|lhs| lhs.span()).collect::<Vec<_>>()
546+
} else {
547+
Vec::new()
548+
};
549+
533550
// Convert the lhses into `MatcherLoc` form, which is better for doing the
534551
// actual matching. Unless the matcher is invalid.
535552
let lhses = if valid {
@@ -549,17 +566,16 @@ pub fn compile_declarative_macro(
549566
vec![]
550567
};
551568

552-
mk_syn_ext(Box::new(MacroRulesMacroExpander {
569+
let expander = Box::new(MacroRulesMacroExpander {
553570
name: def.ident,
554571
span: def.span,
572+
node_id: def.id,
555573
transparency,
556574
lhses,
557575
rhses,
558576
valid,
559-
// Macros defined in the current crate have a real node id,
560-
// whereas macros from an external crate have a dummy id.
561-
is_local: def.id != DUMMY_NODE_ID,
562-
}))
577+
});
578+
(mk_syn_ext(expander), rule_spans)
563579
}
564580

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

compiler/rustc_lint/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
303303
PATH_STATEMENTS,
304304
UNUSED_ATTRIBUTES,
305305
UNUSED_MACROS,
306+
UNUSED_MACRO_RULES,
306307
UNUSED_ALLOCATION,
307308
UNUSED_DOC_COMMENTS,
308309
UNUSED_EXTERN_CRATES,

compiler/rustc_lint_defs/src/builtin.rs

+44
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,10 @@ declare_lint! {
749749
declare_lint! {
750750
/// The `unused_macros` lint detects macros that were not used.
751751
///
752+
/// Note that this lint is distinct from the `unused_macro_rules` lint,
753+
/// which checks for single rules that never match of an otherwise used
754+
/// macro, and thus never expand.
755+
///
752756
/// ### Example
753757
///
754758
/// ```rust
@@ -775,6 +779,45 @@ declare_lint! {
775779
"detects macros that were not used"
776780
}
777781

782+
declare_lint! {
783+
/// The `unused_macro_rules` lint detects macro rules that were not used.
784+
///
785+
/// Note that the lint is distinct from the `unused_macros` lint, which
786+
/// fires if the entire macro is never called, while this lint fires for
787+
/// single unused rules of the macro that is otherwise used.
788+
/// `unused_macro_rules` fires only if `unused_macros` wouldn't fire.
789+
///
790+
/// ### Example
791+
///
792+
/// ```rust
793+
/// macro_rules! unused_empty {
794+
/// (hello) => { println!("Hello, world!") }; // This rule is unused
795+
/// () => { println!("empty") }; // This rule is used
796+
/// }
797+
///
798+
/// fn main() {
799+
/// unused_empty!(hello);
800+
/// }
801+
/// ```
802+
///
803+
/// {{produces}}
804+
///
805+
/// ### Explanation
806+
///
807+
/// Unused macro rules may signal a mistake or unfinished code. Furthermore,
808+
/// they slow down compilation. Right now, silencing the warning is not
809+
/// supported on a single rule level, so you have to add an allow to the
810+
/// entire macro definition.
811+
///
812+
/// If you intended to export the macro to make it
813+
/// available outside of the crate, use the [`macro_export` attribute].
814+
///
815+
/// [`macro_export` attribute]: https://doc.rust-lang.org/reference/macros-by-example.html#path-based-scope
816+
pub UNUSED_MACRO_RULES,
817+
Warn,
818+
"detects macro rules that were not used"
819+
}
820+
778821
declare_lint! {
779822
/// The `warnings` lint allows you to change the level of other
780823
/// lints which produce warnings.
@@ -3104,6 +3147,7 @@ declare_lint_pass! {
31043147
OVERLAPPING_RANGE_ENDPOINTS,
31053148
BINDINGS_WITH_VARIANT_NAME,
31063149
UNUSED_MACROS,
3150+
UNUSED_MACRO_RULES,
31073151
WARNINGS,
31083152
UNUSED_FEATURES,
31093153
STABLE_FEATURES,

compiler/rustc_middle/src/mir/visit.rs

+3
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ macro_rules! make_mir_visitor {
258258
// for best performance, we want to use an iterator rather
259259
// than a for-loop, to avoid calling `body::Body::invalidate` for
260260
// each basic block.
261+
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
261262
macro_rules! basic_blocks {
262263
(mut) => (body.basic_blocks_mut().iter_enumerated_mut());
263264
() => (body.basic_blocks().iter_enumerated());
@@ -279,6 +280,7 @@ macro_rules! make_mir_visitor {
279280
self.visit_local_decl(local, & $($mutability)? body.local_decls[local]);
280281
}
281282

283+
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
282284
macro_rules! type_annotations {
283285
(mut) => (body.user_type_annotations.iter_enumerated_mut());
284286
() => (body.user_type_annotations.iter_enumerated());
@@ -932,6 +934,7 @@ macro_rules! make_mir_visitor {
932934
body: &$($mutability)? Body<'tcx>,
933935
location: Location
934936
) {
937+
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
935938
macro_rules! basic_blocks {
936939
(mut) => (body.basic_blocks_mut());
937940
() => (body.basic_blocks());

compiler/rustc_resolve/Cargo.toml

-1
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

+18-8
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ impl<'a> Resolver<'a> {
194194
}
195195

196196
let ext = Lrc::new(match self.cstore().load_macro_untracked(def_id, &self.session) {
197-
LoadedMacro::MacroDef(item, edition) => self.compile_macro(&item, edition),
197+
LoadedMacro::MacroDef(item, edition) => self.compile_macro(&item, edition).0,
198198
LoadedMacro::ProcMacro(ext) => ext,
199199
});
200200

@@ -1218,25 +1218,35 @@ 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_spans: &[Span],
1227+
) {
12221228
if !ident.as_str().starts_with('_') {
12231229
self.r.unused_macros.insert(def_id, (node_id, ident));
1230+
for (rule_i, rule_span) in rule_spans.iter().enumerate() {
1231+
self.r.unused_macro_rules.insert((def_id, rule_i), (ident, *rule_span));
1232+
}
12241233
}
12251234
}
12261235

12271236
fn define_macro(&mut self, item: &ast::Item) -> MacroRulesScopeRef<'a> {
12281237
let parent_scope = self.parent_scope;
12291238
let expansion = parent_scope.expansion;
12301239
let def_id = self.r.local_def_id(item.id);
1231-
let (ext, ident, span, macro_rules) = match &item.kind {
1240+
let (ext, ident, span, macro_rules, rule_spans) = match &item.kind {
12321241
ItemKind::MacroDef(def) => {
1233-
let ext = Lrc::new(self.r.compile_macro(item, self.r.session.edition()));
1234-
(ext, item.ident, item.span, def.macro_rules)
1242+
let (ext, rule_spans) = self.r.compile_macro(item, self.r.session.edition());
1243+
let ext = Lrc::new(ext);
1244+
(ext, item.ident, item.span, def.macro_rules, rule_spans)
12351245
}
12361246
ItemKind::Fn(..) => match self.proc_macro_stub(item) {
12371247
Some((macro_kind, ident, span)) => {
12381248
self.r.proc_macro_stubs.insert(def_id);
1239-
(self.r.dummy_ext(macro_kind), ident, span, false)
1249+
(self.r.dummy_ext(macro_kind), ident, span, false, Vec::new())
12401250
}
12411251
None => return parent_scope.macro_rules,
12421252
},
@@ -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_spans);
12681278
}
12691279
self.r.visibilities.insert(def_id, vis);
12701280
let scope = self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
@@ -1287,7 +1297,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12871297
_ => self.resolve_visibility(&item.vis),
12881298
};
12891299
if vis != ty::Visibility::Public {
1290-
self.insert_unused_macro(ident, def_id, item.id);
1300+
self.insert_unused_macro(ident, def_id, item.id, &rule_spans);
12911301
}
12921302
self.r.define(module, ident, MacroNS, (res, vis, span, expansion));
12931303
self.r.visibilities.insert(def_id, vis);

compiler/rustc_resolve/src/diagnostics.rs

+14
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ use crate::{LexicalScopeBinding, NameBinding, NameBindingKind, PrivacyError, Vis
3535
use crate::{ParentScope, PathResult, ResolutionError, Resolver, Scope, ScopeSet};
3636
use crate::{Segment, UseError};
3737

38+
#[cfg(test)]
39+
mod tests;
40+
3841
type Res = def::Res<ast::NodeId>;
3942

4043
/// A vector of spans and replacements, a message and applicability.
@@ -2675,3 +2678,14 @@ fn is_span_suitable_for_use_injection(s: Span) -> bool {
26752678
// import or other generated ones
26762679
!s.from_expansion()
26772680
}
2681+
2682+
/// Convert the given number into the corresponding ordinal
2683+
crate fn ordinalize(v: usize) -> String {
2684+
let suffix = match ((11..=13).contains(&(v % 100)), v % 10) {
2685+
(false, 1) => "st",
2686+
(false, 2) => "nd",
2687+
(false, 3) => "rd",
2688+
_ => "th",
2689+
};
2690+
format!("{v}{suffix}")
2691+
}
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)