diff options
| author | bors <bors@rust-lang.org> | 2014-05-24 17:21:20 -0700 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2014-05-24 17:21:20 -0700 |
| commit | 07563be6ebe081c8f6666a7b6eb68d8e32774f2f (patch) | |
| tree | 30b4848c49319e66eaf68b86081f6fdf5c99455b /src/libsyntax | |
| parent | 6304a27b80f3923a8ffc009418c302aa8b06fb93 (diff) | |
| parent | 334799326486e46b67c5405ba9584a26878988a4 (diff) | |
auto merge of #14373 : sfackler/rust/unused-attr, r=huonw
The compiler now tracks which attributes were actually looked at during the compilation process and warns for those that were unused. Some things of note: * The tracking is done via thread locals, as it made the implementation more straightforward. Note that this shouldn't hamper any future parallelization as each task can have its own thread local state which can be merged for the lint pass. If there are serious objections to this, I can restructure things to explicitly pass the state around. * There are a number of attributes that have to be special-cased and globally whitelisted. This happens for four reasons: * The `doc` and `automatically_derived` attributes are used by rustdoc, but not by the compiler. * The crate-level attributes `license`, `desc` and `comment` aren't currently used by anything. * Stability attributes as well as `must_use` are checked only when the tagged item is used, so we can't guarantee that the compiler's looked at them. * 12 attributes are used only in trans, which happens after the lint pass. #14300 is adding infrastructure to track lint state through trans, which this lint should also be able to use to handle the last case. For the other attributes, the right solution would probably involve a specific pass to mark uses that occur in the correct context. For example, a `doc` attribute attached to a match arm should generate a warning, but will not currently. RFC: 0002-attribute-usage
Diffstat (limited to 'src/libsyntax')
| -rw-r--r-- | src/libsyntax/ast.rs | 4 | ||||
| -rw-r--r-- | src/libsyntax/attr.rs | 120 | ||||
| -rw-r--r-- | src/libsyntax/ext/build.rs | 2 | ||||
| -rw-r--r-- | src/libsyntax/ext/deriving/generic/mod.rs | 3 | ||||
| -rw-r--r-- | src/libsyntax/ext/expand.rs | 7 | ||||
| -rw-r--r-- | src/libsyntax/fold.rs | 1 | ||||
| -rw-r--r-- | src/libsyntax/parse/attr.rs | 8 |
7 files changed, 108 insertions, 37 deletions
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index d4c01746098..e77d1faf05d 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1024,9 +1024,13 @@ pub enum AttrStyle { AttrInner, } +#[deriving(Clone, Eq, TotalEq, Encodable, Decodable, Hash)] +pub struct AttrId(pub uint); + // doc-comments are promoted to attributes that have is_sugared_doc = true #[deriving(Clone, Eq, TotalEq, Encodable, Decodable, Hash)] pub struct Attribute_ { + pub id: AttrId, pub style: AttrStyle, pub value: @MetaItem, pub is_sugared_doc: bool, diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index 77c335b8936..527e851ae35 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -11,7 +11,7 @@ // Functions dealing with attributes and meta items use ast; -use ast::{Attribute, Attribute_, MetaItem, MetaWord, MetaNameValue, MetaList}; +use ast::{AttrId, Attribute, Attribute_, MetaItem, MetaWord, MetaNameValue, MetaList}; use codemap::{Span, Spanned, spanned, dummy_spanned}; use codemap::BytePos; use diagnostic::SpanHandler; @@ -21,11 +21,26 @@ use parse::token; use crateid::CrateId; use collections::HashSet; +use collections::bitv::BitvSet; + +local_data_key!(used_attrs: BitvSet) + +pub fn mark_used(attr: &Attribute) { + let mut used = used_attrs.replace(None).unwrap_or_else(|| BitvSet::new()); + let AttrId(id) = attr.node.id; + used.insert(id); + used_attrs.replace(Some(used)); +} + +pub fn is_used(attr: &Attribute) -> bool { + let AttrId(id) = attr.node.id; + used_attrs.get().map_or(false, |used| used.contains(&id)) +} pub trait AttrMetaMethods { - // This could be changed to `fn check_name(&self, name: InternedString) -> - // bool` which would facilitate a side table recording which - // attributes/meta items are used/unused. + fn check_name(&self, name: &str) -> bool { + name == self.name().get() + } /// Retrieve the name of the meta item, e.g. foo in #[foo], /// #[foo="bar"] and #[foo(bar)] @@ -47,6 +62,13 @@ pub trait AttrMetaMethods { } impl AttrMetaMethods for Attribute { + fn check_name(&self, name: &str) -> bool { + let matches = name == self.name().get(); + if matches { + mark_used(self); + } + matches + } fn name(&self) -> InternedString { self.meta().name() } fn value_str(&self) -> Option<InternedString> { self.meta().value_str() @@ -127,9 +149,9 @@ impl AttributeMethods for Attribute { token::intern_and_get_ident(strip_doc_comment_decoration( comment.get()).as_slice())); if self.node.style == ast::AttrOuter { - mk_attr_outer(meta) + mk_attr_outer(self.node.id, meta) } else { - mk_attr_inner(meta) + mk_attr_inner(self.node.id, meta) } } else { *self @@ -158,9 +180,18 @@ pub fn mk_word_item(name: InternedString) -> @MetaItem { @dummy_spanned(MetaWord(name)) } +local_data_key!(next_attr_id: uint) + +pub fn mk_attr_id() -> AttrId { + let id = next_attr_id.replace(None).unwrap_or(0); + next_attr_id.replace(Some(id + 1)); + AttrId(id) +} + /// Returns an inner attribute with the given value. -pub fn mk_attr_inner(item: @MetaItem) -> Attribute { +pub fn mk_attr_inner(id: AttrId, item: @MetaItem) -> Attribute { dummy_spanned(Attribute_ { + id: id, style: ast::AttrInner, value: item, is_sugared_doc: false, @@ -168,19 +199,22 @@ pub fn mk_attr_inner(item: @MetaItem) -> Attribute { } /// Returns an outer attribute with the given value. -pub fn mk_attr_outer(item: @MetaItem) -> Attribute { +pub fn mk_attr_outer(id: AttrId, item: @MetaItem) -> Attribute { dummy_spanned(Attribute_ { + id: id, style: ast::AttrOuter, value: item, is_sugared_doc: false, }) } -pub fn mk_sugared_doc_attr(text: InternedString, lo: BytePos, hi: BytePos) +pub fn mk_sugared_doc_attr(id: AttrId, text: InternedString, lo: BytePos, + hi: BytePos) -> Attribute { let style = doc_comment_style(text.get()); let lit = spanned(lo, hi, ast::LitStr(text, ast::CookedStr)); let attr = Attribute_ { + id: id, style: style, value: @spanned(lo, hi, MetaNameValue(InternedString::new("doc"), lit)), @@ -206,14 +240,14 @@ pub fn contains_name<AM: AttrMetaMethods>(metas: &[AM], name: &str) -> bool { debug!("attr::contains_name (name={})", name); metas.iter().any(|item| { debug!(" testing: {}", item.name()); - item.name().equiv(&name) + item.check_name(name) }) } pub fn first_attr_value_str_by_name(attrs: &[Attribute], name: &str) -> Option<InternedString> { attrs.iter() - .find(|at| at.name().equiv(&name)) + .find(|at| at.check_name(name)) .and_then(|at| at.value_str()) } @@ -221,7 +255,7 @@ pub fn last_meta_item_value_str_by_name(items: &[@MetaItem], name: &str) -> Option<InternedString> { items.iter() .rev() - .find(|mi| mi.name().equiv(&name)) + .find(|mi| mi.check_name(name)) .and_then(|i| i.value_str()) } @@ -257,7 +291,7 @@ pub fn sort_meta_items(items: &[@MetaItem]) -> Vec<@MetaItem> { */ pub fn find_linkage_metas(attrs: &[Attribute]) -> Vec<@MetaItem> { let mut result = Vec::new(); - for attr in attrs.iter().filter(|at| at.name().equiv(&("link"))) { + for attr in attrs.iter().filter(|at| at.check_name("link")) { match attr.meta().node { MetaList(_, ref items) => result.push_all(items.as_slice()), _ => () @@ -286,17 +320,21 @@ pub fn find_inline_attr(attrs: &[Attribute]) -> InlineAttr { // FIXME (#2809)---validate the usage of #[inline] and #[inline] attrs.iter().fold(InlineNone, |ia,attr| { match attr.node.value.node { - MetaWord(ref n) if n.equiv(&("inline")) => InlineHint, - MetaList(ref n, ref items) if n.equiv(&("inline")) => { - if contains_name(items.as_slice(), "always") { - InlineAlways - } else if contains_name(items.as_slice(), "never") { - InlineNever - } else { + MetaWord(ref n) if n.equiv(&("inline")) => { + mark_used(attr); InlineHint } - } - _ => ia + MetaList(ref n, ref items) if n.equiv(&("inline")) => { + mark_used(attr); + if contains_name(items.as_slice(), "always") { + InlineAlways + } else if contains_name(items.as_slice(), "never") { + InlineNever + } else { + InlineHint + } + } + _ => ia } }) } @@ -314,9 +352,9 @@ pub fn test_cfg<AM: AttrMetaMethods, It: Iterator<AM>> // this would be much nicer as a chain of iterator adaptors, but // this doesn't work. - let some_cfg_matches = metas.any(|mi| { + let some_cfg_matches = metas.fold(false, |matches, mi| { debug!("testing name: {}", mi.name()); - if mi.name().equiv(&("cfg")) { // it is a #[cfg()] attribute + let this_matches = if mi.check_name("cfg") { // it is a #[cfg()] attribute debug!("is cfg"); no_cfgs = false; // only #[cfg(...)] ones are understood. @@ -344,7 +382,8 @@ pub fn test_cfg<AM: AttrMetaMethods, It: Iterator<AM>> } } else { false - } + }; + matches || this_matches }); debug!("test_cfg (no_cfgs={}, some_cfg_matches={})", no_cfgs, some_cfg_matches); no_cfgs || some_cfg_matches @@ -367,11 +406,13 @@ pub enum StabilityLevel { Locked } -/// Find the first stability attribute. `None` if none exists. -pub fn find_stability<AM: AttrMetaMethods, It: Iterator<AM>>(mut metas: It) - -> Option<Stability> { - for m in metas { - let level = match m.name().get() { +pub fn find_stability_generic<'a, + AM: AttrMetaMethods, + I: Iterator<&'a AM>> + (mut attrs: I) + -> Option<(Stability, &'a AM)> { + for attr in attrs { + let level = match attr.name().get() { "deprecated" => Deprecated, "experimental" => Experimental, "unstable" => Unstable, @@ -381,14 +422,22 @@ pub fn find_stability<AM: AttrMetaMethods, It: Iterator<AM>>(mut metas: It) _ => continue // not a stability level }; - return Some(Stability { + return Some((Stability { level: level, - text: m.value_str() - }); + text: attr.value_str() + }, attr)); } None } +/// Find the first stability attribute. `None` if none exists. +pub fn find_stability(attrs: &[Attribute]) -> Option<Stability> { + find_stability_generic(attrs.iter()).map(|(s, attr)| { + mark_used(attr); + s + }) +} + pub fn require_unique_names(diagnostic: &SpanHandler, metas: &[@MetaItem]) { let mut set = HashSet::new(); for meta in metas.iter() { @@ -415,11 +464,12 @@ pub fn require_unique_names(diagnostic: &SpanHandler, metas: &[@MetaItem]) { * present (before fields, if any) with that type; reprensentation * optimizations which would remove it will not be done. */ -pub fn find_repr_attr(diagnostic: &SpanHandler, attr: @ast::MetaItem, acc: ReprAttr) +pub fn find_repr_attr(diagnostic: &SpanHandler, attr: &Attribute, acc: ReprAttr) -> ReprAttr { let mut acc = acc; - match attr.node { + match attr.node.value.node { ast::MetaList(ref s, ref items) if s.equiv(&("repr")) => { + mark_used(attr); for item in items.iter() { match item.node { ast::MetaWord(ref word) => { diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 3c7415ae0e9..449feb3afbf 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -12,6 +12,7 @@ use abi; use ast::{P, Ident}; use ast; use ast_util; +use attr; use codemap::{Span, respan, Spanned, DUMMY_SP}; use ext::base::ExtCtxt; use ext::quote::rt::*; @@ -927,6 +928,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { fn attribute(&self, sp: Span, mi: @ast::MetaItem) -> ast::Attribute { respan(sp, ast::Attribute_ { + id: attr::mk_attr_id(), style: ast::AttrOuter, value: mi, is_sugared_doc: false, diff --git a/src/libsyntax/ext/deriving/generic/mod.rs b/src/libsyntax/ext/deriving/generic/mod.rs index 0875daddc0f..5f18193437e 100644 --- a/src/libsyntax/ext/deriving/generic/mod.rs +++ b/src/libsyntax/ext/deriving/generic/mod.rs @@ -182,6 +182,7 @@ use std::cell::RefCell; use ast; use ast::{P, EnumDef, Expr, Ident, Generics, StructDef}; use ast_util; +use attr; use attr::AttrMetaMethods; use ext::base::ExtCtxt; use ext::build::AstBuilder; @@ -430,6 +431,8 @@ impl<'a> TraitDef<'a> { self.span, cx.meta_word(self.span, InternedString::new("automatically_derived"))); + // Just mark it now since we know that it'll end up used downstream + attr::mark_used(&attr); let opt_trait_ref = Some(trait_ref); let ident = ast_util::impl_pretty_name(&opt_trait_ref, self_type); cx.item( diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 989d0a463c3..658e4bafbe2 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -265,6 +265,8 @@ pub fn expand_item(it: @ast::Item, fld: &mut MacroExpander) match fld.extsbox.find(&intern(mname.get())) { Some(&ItemDecorator(dec_fn)) => { + attr::mark_used(attr); + fld.cx.bt_push(ExpnInfo { call_site: attr.span, callee: NameAndSpan { @@ -336,6 +338,7 @@ fn expand_item_modifiers(mut it: @ast::Item, fld: &mut MacroExpander) match fld.extsbox.find(&intern(mname.get())) { Some(&ItemModifier(dec_fn)) => { + attr::mark_used(attr); fld.cx.bt_push(ExpnInfo { call_site: attr.span, callee: NameAndSpan { @@ -474,7 +477,7 @@ pub fn expand_view_item(vi: &ast::ViewItem, match vi.node { ast::ViewItemExternCrate(..) => { let should_load = vi.attrs.iter().any(|attr| { - attr.name().get() == "phase" && + attr.check_name("phase") && attr.meta_item_list().map_or(false, |phases| { attr::contains_name(phases, "syntax") }) @@ -972,6 +975,7 @@ mod test { use super::*; use ast; use ast::{Attribute_, AttrOuter, MetaWord}; + use attr; use codemap; use codemap::Spanned; use ext::base::{CrateLoader, MacroCrate}; @@ -1103,6 +1107,7 @@ mod test { Spanned { span:codemap::DUMMY_SP, node: Attribute_ { + id: attr::mk_attr_id(), style: AttrOuter, value: @Spanned { node: MetaWord(token::intern_and_get_ident(s)), diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 9813e12de01..ae5cf550bb9 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -360,6 +360,7 @@ fn fold_attribute_<T: Folder>(at: Attribute, fld: &mut T) -> Attribute { Spanned { span: fld.new_span(at.span), node: ast::Attribute_ { + id: at.node.id, style: at.node.style, value: fold_meta_item_(at.node.value, fld), is_sugared_doc: at.node.is_sugared_doc diff --git a/src/libsyntax/parse/attr.rs b/src/libsyntax/parse/attr.rs index 89d1b8f9342..9dcc0877fa4 100644 --- a/src/libsyntax/parse/attr.rs +++ b/src/libsyntax/parse/attr.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use attr; use ast; use codemap::{spanned, Spanned, mk_sp, Span}; use parse::common::*; //resolve bug? @@ -39,6 +40,7 @@ impl<'a> ParserAttr for Parser<'a> { } token::DOC_COMMENT(s) => { let attr = ::attr::mk_sugared_doc_attr( + attr::mk_attr_id(), self.id_to_interned_str(s), self.span.lo, self.span.hi @@ -101,6 +103,7 @@ impl<'a> ParserAttr for Parser<'a> { return Spanned { span: span, node: ast::Attribute_ { + id: attr::mk_attr_id(), style: style, value: value, is_sugared_doc: false @@ -132,7 +135,10 @@ impl<'a> ParserAttr for Parser<'a> { // we need to get the position of this token before we bump. let Span { lo, hi, .. } = self.span; self.bump(); - ::attr::mk_sugared_doc_attr(self.id_to_interned_str(s), lo, hi) + attr::mk_sugared_doc_attr(attr::mk_attr_id(), + self.id_to_interned_str(s), + lo, + hi) } _ => { break; |
