Skip to content

Commit

Permalink
Rollup merge of rust-lang#41907 - est31:macro_unused, r=jseyfried
Browse files Browse the repository at this point in the history
Add lint for unused macros

Addresses parts of rust-lang#34938, to add a lint for unused macros.

We now output warnings by default when we encounter a macro that we didn't use for expansion.

Issues to be resolved before this PR is ready for merge:

- [x] fix the NodeId issue described above
- [x] remove all unused macros from rustc and the libraries or set `#[allow(unused_macros)]` next to them if they should be kept for some reason. This is needed for successful boostrap and bors to accept the PR. -> rust-lang#41934
- [x] ~~implement the full extent of rust-lang#34938, that means the macro match arm checking as well.~~ *let's not do this for now*
  • Loading branch information
frewsxcv authored May 16, 2017
2 parents 403fde8 + 25b7f10 commit d5a11b3
Show file tree
Hide file tree
Showing 31 changed files with 137 additions and 19 deletions.
1 change: 1 addition & 0 deletions src/libcore/num/wrapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use super::Wrapping;

use ops::*;

#[allow(unused_macros)]
macro_rules! sh_impl_signed {
($t:ident, $f:ident) => (
#[stable(feature = "rust1", since = "1.0.0")]
Expand Down
1 change: 1 addition & 0 deletions src/libcore/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,7 @@ macro_rules! neg_impl_numeric {
($($t:ty)*) => { neg_impl_core!{ x => -x, $($t)*} }
}

#[allow(unused_macros)]
macro_rules! neg_impl_unsigned {
($($t:ty)*) => {
neg_impl_core!{ x => {
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ declare_lint! {
"detects unreachable patterns"
}

declare_lint! {
pub UNUSED_MACROS,
Warn,
"detects macros that were not used"
}

declare_lint! {
pub WARNINGS,
Warn,
Expand Down Expand Up @@ -259,6 +265,7 @@ impl LintPass for HardwiredLints {
DEAD_CODE,
UNREACHABLE_CODE,
UNREACHABLE_PATTERNS,
UNUSED_MACROS,
WARNINGS,
UNUSED_FEATURES,
STABLE_FEATURES,
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use hir;
use hir::def_id::LOCAL_CRATE;
use hir::intravisit as hir_visit;
use syntax::visit as ast_visit;
use syntax::tokenstream::ThinTokenStream;

/// Information about the registered lints.
///
Expand Down Expand Up @@ -1125,6 +1126,13 @@ impl<'a> ast_visit::Visitor<'a> for EarlyContext<'a> {
fn visit_attribute(&mut self, attr: &'a ast::Attribute) {
run_lints!(self, check_attribute, early_passes, attr);
}

fn visit_mac_def(&mut self, _mac: &'a ThinTokenStream, id: ast::NodeId) {
let lints = self.sess.lints.borrow_mut().take(id);
for early_lint in lints {
self.early_lint(&early_lint);
}
}
}

enum CheckLintNameResult {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,8 @@ pub fn phase_2_configure_and_expand<F>(sess: &Session,

let krate = ecx.monotonic_expander().expand_crate(krate);

ecx.check_unused_macros();

let mut missing_fragment_specifiers: Vec<_> =
ecx.parse_sess.missing_fragment_specifiers.borrow().iter().cloned().collect();
missing_fragment_specifiers.sort();
Expand Down
11 changes: 0 additions & 11 deletions src/librustc_incremental/persist/preds/compress/test_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,3 @@ macro_rules! graph {
}
}
}

macro_rules! set {
($( $value:expr ),*) => {
{
use $crate::rustc_data_structures::fx::FxHashSet;
let mut set = FxHashSet();
$(set.insert($value);)*
set
}
}
}
3 changes: 2 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
UNUSED_MUST_USE,
UNUSED_UNSAFE,
PATH_STATEMENTS,
UNUSED_ATTRIBUTES);
UNUSED_ATTRIBUTES,
UNUSED_MACROS);

// Guidelines for creating a future incompatibility lint:
//
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_plugin/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ impl<'a> Registry<'a> {
}
self.syntax_exts.push((name, match extension {
NormalTT(ext, _, allow_internal_unstable) => {
NormalTT(ext, Some(self.krate_span), allow_internal_unstable)
let nid = ast::CRATE_NODE_ID;
NormalTT(ext, Some((nid, self.krate_span)), allow_internal_unstable)
}
IdentTT(ext, _, allow_internal_unstable) => {
IdentTT(ext, Some(self.krate_span), allow_internal_unstable)
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,10 @@ pub struct Resolver<'a> {
pub whitelisted_legacy_custom_derives: Vec<Name>,
pub found_unresolved_macro: bool,

// List of crate local macros that we need to warn about as being unused.
// Right now this only includes macro_rules! macros.
unused_macros: FxHashSet<DefId>,

// Maps the `Mark` of an expansion to its containing module or block.
invocations: FxHashMap<Mark, &'a InvocationData<'a>>,

Expand Down Expand Up @@ -1392,6 +1396,7 @@ impl<'a> Resolver<'a> {
potentially_unused_imports: Vec::new(),
struct_constructors: DefIdMap(),
found_unresolved_macro: false,
unused_macros: FxHashSet(),
}
}

Expand Down
26 changes: 24 additions & 2 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use resolve_imports::ImportResolver;
use rustc::hir::def_id::{DefId, BUILTIN_MACROS_CRATE, CRATE_DEF_INDEX, DefIndex};
use rustc::hir::def::{Def, Export};
use rustc::hir::map::{self, DefCollector};
use rustc::ty;
use rustc::{ty, lint};
use syntax::ast::{self, Name, Ident};
use syntax::attr::{self, HasAttrs};
use syntax::errors::DiagnosticBuilder;
Expand Down Expand Up @@ -291,12 +291,32 @@ impl<'a> base::Resolver for Resolver<'a> {
},
};
self.macro_defs.insert(invoc.expansion_data.mark, def.def_id());
self.unused_macros.remove(&def.def_id());
Ok(Some(self.get_macro(def)))
}

fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy> {
self.resolve_macro_to_def(scope, path, kind, force).map(|def| self.get_macro(def))
self.resolve_macro_to_def(scope, path, kind, force).map(|def| {
self.unused_macros.remove(&def.def_id());
self.get_macro(def)
})
}

fn check_unused_macros(&self) {
for did in self.unused_macros.iter() {
let id_span = match *self.macro_map[did] {
SyntaxExtension::NormalTT(_, isp, _) => isp,
_ => None,
};
if let Some((id, span)) = id_span {
let lint = lint::builtin::UNUSED_MACROS;
let msg = "unused macro definition".to_string();
self.session.add_lint(lint, id, span, msg);
} else {
bug!("attempted to create unused macro error, but span not available");
}
}
}
}

Expand Down Expand Up @@ -687,6 +707,8 @@ impl<'a> Resolver<'a> {
if attr::contains_name(&item.attrs, "macro_export") {
let def = Def::Macro(def_id, MacroKind::Bang);
self.macro_exports.push(Export { name: ident.name, def: def, span: item.span });
} else {
self.unused_macros.insert(def_id);
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ pub enum SyntaxExtension {
///
/// The `bool` dictates whether the contents of the macro can
/// directly use `#[unstable]` things (true == yes).
NormalTT(Box<TTMacroExpander>, Option<Span>, bool),
NormalTT(Box<TTMacroExpander>, Option<(ast::NodeId, Span)>, bool),

/// A function-like syntax extension that has an extra ident before
/// the block.
Expand Down Expand Up @@ -589,6 +589,7 @@ pub trait Resolver {
-> Result<Option<Rc<SyntaxExtension>>, Determinacy>;
fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy>;
fn check_unused_macros(&self);
}

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -618,6 +619,7 @@ impl Resolver for DummyResolver {
_force: bool) -> Result<Rc<SyntaxExtension>, Determinacy> {
Err(Determinacy::Determined)
}
fn check_unused_macros(&self) {}
}

#[derive(Clone)]
Expand Down Expand Up @@ -800,6 +802,10 @@ impl<'a> ExtCtxt<'a> {
pub fn name_of(&self, st: &str) -> ast::Name {
Symbol::intern(st)
}

pub fn check_unused_macros(&self) {
self.resolver.check_unused_macros();
}
}

/// Extract a string literal from the macro expanded version of `expr`,
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
call_site: span,
callee: NameAndSpan {
format: MacroBang(Symbol::intern(&format!("{}", path))),
span: exp_span,
span: exp_span.map(|(_, s)| s),
allow_internal_unstable: allow_internal_unstable,
},
});
Expand Down
4 changes: 3 additions & 1 deletion src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ pub fn compile(sess: &ParseSess, features: &RefCell<Features>, def: &ast::Item)
valid: valid,
});

NormalTT(exp, Some(def.span), attr::contains_name(&def.attrs, "allow_internal_unstable"))
NormalTT(exp,
Some((def.id, def.span)),
attr::contains_name(&def.attrs, "allow_internal_unstable"))
}

fn check_lhs_nt_follows(sess: &ParseSess,
Expand Down
6 changes: 5 additions & 1 deletion src/libsyntax/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use abi::Abi;
use ast::*;
use syntax_pos::Span;
use codemap::Spanned;
use tokenstream::ThinTokenStream;

#[derive(Copy, Clone, PartialEq, Eq)]
pub enum FnKind<'a> {
Expand Down Expand Up @@ -112,6 +113,9 @@ pub trait Visitor<'ast>: Sized {
// definition in your trait impl:
// visit::walk_mac(self, _mac)
}
fn visit_mac_def(&mut self, _mac: &'ast ThinTokenStream, _id: NodeId) {
// Nothing to do
}
fn visit_path(&mut self, path: &'ast Path, _id: NodeId) {
walk_path(self, path)
}
Expand Down Expand Up @@ -290,7 +294,7 @@ pub fn walk_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a Item) {
walk_list!(visitor, visit_trait_item, methods);
}
ItemKind::Mac(ref mac) => visitor.visit_mac(mac),
ItemKind::MacroDef(..) => {},
ItemKind::MacroDef(ref ts) => visitor.visit_mac_def(ts, item.id),
}
walk_list!(visitor, visit_attribute, &item.attrs);
}
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail-fulldeps/proc-macro/resolve-error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// aux-build:bang_proc_macro.rs

#![feature(proc_macro)]
#![allow(unused_macros)]

#[macro_use]
extern crate derive_foo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

// gate-test-allow_internal_unstable

#![allow(unused_macros)]

macro_rules! bar {
() => {
// more layers don't help:
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/feature-gate-allow-internal-unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

#[allow_internal_unstable] //~ ERROR allow_internal_unstable side-steps
macro_rules! foo {
() => {}
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/invalid-macro-matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

macro_rules! invalid {
_ => (); //~ ERROR invalid macro matcher
}
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/issue-21356.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

macro_rules! test { ($wrong:t_ty ..) => () }
//~^ ERROR: invalid fragment specifier `t_ty`

Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/issue-39388.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

macro_rules! assign {
(($($a:tt)*) = ($($b:tt))*) => { //~ ERROR expected `*` or `+`
$($a)* = $($b)*
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/issue-39404.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

#![deny(missing_fragment_specifier)] //~ NOTE lint level defined here
#![allow(unused_macros)]

macro_rules! m { ($i) => {} }
//~^ ERROR missing fragment specifier
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/issue-5067.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

macro_rules! foo {
( $()* ) => {};
//~^ ERROR repetition matches empty token tree
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/macro-expansion-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

mod macros_cant_escape_fns {
fn f() {
macro_rules! m { () => { 3 + 4 } }
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/macro-follow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
// Check the macro follow sets (see corresponding rpass test).

#![allow(unused_macros)]

// FOLLOW(pat) = {FatArrow, Comma, Eq, Or, Ident(if), Ident(in)}
macro_rules! follow_pat {
($p:pat ()) => {}; //~ERROR `$p:pat` is followed by `(`
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/macro-followed-by-seq-bad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
// Regression test for issue #25436: check that things which can be
// followed by any token also permit X* to come afterwards.

#![allow(unused_macros)]

macro_rules! foo {
( $a:expr $($b:tt)* ) => { }; //~ ERROR not allowed for `expr` fragments
( $a:ty $($b:tt)* ) => { }; //~ ERROR not allowed for `ty` fragments
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/macro-input-future-proofing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

macro_rules! errors_everywhere {
($ty:ty <) => (); //~ ERROR `$ty:ty` is followed by `<`, which is not allowed for `ty`
($ty:ty < foo ,) => (); //~ ERROR `$ty:ty` is followed by `<`, which is not allowed for `ty`
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/macro-shadowing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

// aux-build:two_macros.rs

#![allow(unused_macros)]

macro_rules! foo { () => {} }
macro_rules! macro_one { () => {} }
#[macro_use(macro_two)] extern crate two_macros;
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/unused-macro-with-bad-frag-spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

// Issue #21370

macro_rules! test {
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/unused-macro-with-follow-violation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused_macros)]

macro_rules! test {
($e:expr +) => () //~ ERROR not allowed for `expr` fragments
}
Expand Down
Loading

0 comments on commit d5a11b3

Please sign in to comment.