Skip to content

Commit

Permalink
Rollup merge of rust-lang#96150 - est31:unused_macro_rules, r=fee1-dead
Browse files Browse the repository at this point in the history
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 ~~(and currently integrates)~~ rust-lang#96149 and rust-lang#96156.

Fixes rust-lang#73576
  • Loading branch information
Dylan-DPC authored Apr 26, 2022
2 parents 9a92b50 + c347636 commit 0aec217
Show file tree
Hide file tree
Showing 33 changed files with 382 additions and 70 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
span: Span,
) -> Result<&'ll Value, ()> {
// macros for error handling:
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
macro_rules! emit_error {
($msg: tt) => {
emit_error!($msg, )
Expand Down Expand Up @@ -1144,6 +1145,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
span: Span,
args: &[OperandRef<'tcx, &'ll Value>],
) -> Result<&'ll Value, ()> {
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
macro_rules! emit_error {
($msg: tt) => {
emit_error!($msg, )
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,9 @@ pub struct SyntaxExtension {
/// Built-in macros have a couple of special properties like availability
/// in `#[no_implicit_prelude]` modules, so we have to keep this flag.
pub builtin_name: Option<Symbol>,
/// Rule spans. Used for linting. If the macro is not made up of rules,
/// it's empty.
pub rule_spans: Vec<Span>,
}

impl SyntaxExtension {
Expand Down Expand Up @@ -740,6 +743,7 @@ impl SyntaxExtension {
edition,
builtin_name: None,
kind,
rule_spans: Vec::new(),
}
}

Expand All @@ -753,6 +757,7 @@ impl SyntaxExtension {
edition: Edition,
name: Symbol,
attrs: &[ast::Attribute],
rule_spans: Vec<Span>,
) -> SyntaxExtension {
let allow_internal_unstable =
attr::allow_internal_unstable(sess, &attrs).collect::<Vec<Symbol>>();
Expand Down Expand Up @@ -800,6 +805,7 @@ impl SyntaxExtension {
helper_attrs,
edition,
builtin_name,
rule_spans,
}
}

Expand Down Expand Up @@ -887,6 +893,8 @@ pub trait ResolverExpand {
force: bool,
) -> Result<Lrc<SyntaxExtension>, Indeterminate>;

fn record_macro_rule_usage(&mut self, mac_id: NodeId, rule_index: usize);

fn check_unused_macros(&mut self);

// Resolver interfaces for specific built-in macros.
Expand Down
46 changes: 31 additions & 15 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl<'a> ParserAnyMacro<'a> {
}

struct MacroRulesMacroExpander {
id: NodeId,
name: Ident,
span: Span,
transparency: Transparency,
Expand All @@ -179,6 +180,7 @@ impl TTMacroExpander for MacroRulesMacroExpander {
cx,
sp,
self.span,
self.id,
self.name,
self.transparency,
input,
Expand Down Expand Up @@ -207,6 +209,7 @@ fn generic_extension<'cx, 'tt>(
cx: &'cx mut ExtCtxt<'_>,
sp: Span,
def_span: Span,
id: NodeId,
name: Ident,
transparency: Transparency,
arg: TokenStream,
Expand Down Expand Up @@ -297,6 +300,8 @@ fn generic_extension<'cx, 'tt>(
let mut p = Parser::new(sess, tts, false, None);
p.last_type_ascription = cx.current_expansion.prior_type_ascription;

cx.resolver.record_macro_rule_usage(id, i);

// Let the context choose how to interpret the result.
// Weird, but useful for X-macros.
return Box::new(ParserAnyMacro {
Expand Down Expand Up @@ -375,7 +380,7 @@ pub fn compile_declarative_macro(
edition: Edition,
) -> SyntaxExtension {
debug!("compile_declarative_macro: {:?}", def);
let mk_syn_ext = |expander| {
let mk_syn_ext = |expander, rule_spans| {
SyntaxExtension::new(
sess,
SyntaxExtensionKind::LegacyBang(expander),
Expand All @@ -384,8 +389,10 @@ pub fn compile_declarative_macro(
edition,
def.ident.name,
&def.attrs,
rule_spans,
)
};
let dummy_syn_ext = || mk_syn_ext(Box::new(macro_rules_dummy_expander), Vec::new());

let diag = &sess.parse_sess.span_diagnostic;
let lhs_nm = Ident::new(sym::lhs, def.span);
Expand Down Expand Up @@ -446,17 +453,17 @@ pub fn compile_declarative_macro(
let s = parse_failure_msg(&token);
let sp = token.span.substitute_dummy(def.span);
sess.parse_sess.span_diagnostic.struct_span_err(sp, &s).span_label(sp, msg).emit();
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
return dummy_syn_ext();
}
Error(sp, msg) => {
sess.parse_sess
.span_diagnostic
.struct_span_err(sp.substitute_dummy(def.span), &msg)
.emit();
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
return dummy_syn_ext();
}
ErrorReported => {
return mk_syn_ext(Box::new(macro_rules_dummy_expander));
return dummy_syn_ext();
}
};

Expand Down Expand Up @@ -531,6 +538,11 @@ pub fn compile_declarative_macro(
None => {}
}

// Compute the spans of the macro rules
// We only take the span of the lhs here,
// so that the spans of created warnings are smaller.
let rule_spans = lhses.iter().map(|lhs| lhs.span()).collect::<Vec<_>>();

// Convert the lhses into `MatcherLoc` form, which is better for doing the
// actual matching. Unless the matcher is invalid.
let lhses = if valid {
Expand All @@ -550,17 +562,21 @@ pub fn compile_declarative_macro(
vec![]
};

mk_syn_ext(Box::new(MacroRulesMacroExpander {
name: def.ident,
span: def.span,
transparency,
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,
}))
mk_syn_ext(
Box::new(MacroRulesMacroExpander {
name: def.ident,
span: def.span,
id: def.id,
transparency,
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,
}),
rule_spans,
)
}

fn check_lhs_nt_follows(sess: &ParseSess, def: &ast::Item, lhs: &mbe::TokenTree) -> bool {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
PATH_STATEMENTS,
UNUSED_ATTRIBUTES,
UNUSED_MACROS,
UNUSED_MACRO_RULES,
UNUSED_ALLOCATION,
UNUSED_DOC_COMMENTS,
UNUSED_EXTERN_CRATES,
Expand Down
30 changes: 30 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,35 @@ declare_lint! {
"detects macros that were not used"
}

declare_lint! {
/// The `unused_macro_rules` lint detects macro rules that were not used.
///
/// ### Example
///
/// ```rust
/// macro_rules! unused_empty {
/// (hello) => { println!("Hello, world!") };
/// () => { println!("empty") };
/// }
///
/// fn main() {
/// unused_empty!(hello);
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Unused macro rules may signal a mistake or unfinished code. Furthermore,
/// they slow down compilation. Right now, silencing the warning is not
/// supported on a single rule level, so you have to add an allow to the
/// entire macro definition.
pub UNUSED_MACRO_RULES,
Warn,
"detects macro rules that were not used"
}

declare_lint! {
/// The `warnings` lint allows you to change the level of other
/// lints which produce warnings.
Expand Down Expand Up @@ -3138,6 +3167,7 @@ declare_lint_pass! {
OVERLAPPING_RANGE_ENDPOINTS,
BINDINGS_WITH_VARIANT_NAME,
UNUSED_MACROS,
UNUSED_MACRO_RULES,
WARNINGS,
UNUSED_FEATURES,
STABLE_FEATURES,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
self.root.edition,
Symbol::intern(name),
&attrs,
Vec::new(),
)
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ macro_rules! make_mir_visitor {
// for best performance, we want to use an iterator rather
// than a for-loop, to avoid calling `body::Body::invalidate` for
// each basic block.
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
macro_rules! basic_blocks {
(mut) => (body.basic_blocks_mut().iter_enumerated_mut());
() => (body.basic_blocks().iter_enumerated());
Expand All @@ -279,6 +280,7 @@ macro_rules! make_mir_visitor {
self.visit_local_decl(local, & $($mutability)? body.local_decls[local]);
}

#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
macro_rules! type_annotations {
(mut) => (body.user_type_annotations.iter_enumerated_mut());
() => (body.user_type_annotations.iter_enumerated());
Expand Down Expand Up @@ -932,6 +934,7 @@ macro_rules! make_mir_visitor {
body: &$($mutability)? Body<'tcx>,
location: Location
) {
#[cfg_attr(not(bootstrap), allow(unused_macro_rules))]
macro_rules! basic_blocks {
(mut) => (body.basic_blocks_mut());
() => (body.basic_blocks());
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_resolve/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ version = "0.0.0"
edition = "2021"

[lib]
test = false
doctest = false

[dependencies]
Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,9 +1218,18 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
// Mark the given macro as unused unless its name starts with `_`.
// Macro uses will remove items from this set, and the remaining
// items will be reported as `unused_macros`.
fn insert_unused_macro(&mut self, ident: Ident, def_id: LocalDefId, node_id: NodeId) {
fn insert_unused_macro(
&mut self,
ident: Ident,
def_id: LocalDefId,
node_id: NodeId,
rule_count: usize,
) {
if !ident.as_str().starts_with('_') {
self.r.unused_macros.insert(def_id, (node_id, ident));
for rule_i in 0..rule_count {
self.r.unused_macro_rules.insert((def_id, rule_i), (node_id, ident));
}
}
}

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

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

Expand All @@ -1264,7 +1274,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
self.r.define(module, ident, MacroNS, (res, vis, span, expansion, IsMacroExport));
} else {
self.r.check_reserved_macro_name(ident, res);
self.insert_unused_macro(ident, def_id, item.id);
self.insert_unused_macro(ident, def_id, item.id, rule_count);
}
self.r.visibilities.insert(def_id, vis);
self.r.arenas.alloc_macro_rules_scope(MacroRulesScope::Binding(
Expand All @@ -1285,7 +1295,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
_ => self.resolve_visibility(&item.vis),
};
if vis != ty::Visibility::Public {
self.insert_unused_macro(ident, def_id, item.id);
self.insert_unused_macro(ident, def_id, item.id, rule_count);
}
self.r.define(module, ident, MacroNS, (res, vis, span, expansion));
self.r.visibilities.insert(def_id, vis);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ pub struct Resolver<'a> {
local_macro_def_scopes: FxHashMap<LocalDefId, Module<'a>>,
ast_transform_scopes: FxHashMap<LocalExpnId, Module<'a>>,
unused_macros: FxHashMap<LocalDefId, (NodeId, Ident)>,
unused_macro_rules: FxHashMap<(LocalDefId, usize), (NodeId, Ident)>,
proc_macro_stubs: FxHashSet<LocalDefId>,
/// Traces collected during macro resolution and validated when it's complete.
single_segment_macro_resolutions:
Expand Down Expand Up @@ -1354,6 +1355,7 @@ impl<'a> Resolver<'a> {
potentially_unused_imports: Vec::new(),
struct_constructors: Default::default(),
unused_macros: Default::default(),
unused_macro_rules: Default::default(),
proc_macro_stubs: Default::default(),
single_segment_macro_resolutions: Default::default(),
multi_segment_macro_resolutions: Default::default(),
Expand Down
Loading

0 comments on commit 0aec217

Please sign in to comment.