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

src/doc/rust.md

@@ -661,6 +661,7 @@ Attributes on the anonymous crate module define important metadata that influenc
 the behavior of the compiler.
 
 ~~~~ {.rust}
+# #![allow(unused_attribute)]
 // Crate ID
 #![crate_id = "projx#2.5"]
 

src/doc/rustdoc.md

@@ -11,6 +11,7 @@ Documenting Rust APIs is quite simple. To document a given item, we have "doc
 comments":
 
 ~~~
+# #![allow(unused_attribute)]
 // the "link" crate attribute is currently required for rustdoc, but normally
 // isn't needed.
 #![crate_id = "universe"]

src/doc/tutorial.md

@@ -3166,6 +3166,7 @@ without conflict.
 Therefore, if you plan to compile your crate as a library, you should annotate it with that information:
 
 ~~~~
+# #![allow(unused_attribute)]
 // `lib.rs`
 
 # #![crate_type = "lib"]
@@ -3189,6 +3190,7 @@ Other crate settings and metadata include things like enabling/disabling certain
 or setting the crate type (library or executable) explicitly:
 
 ~~~~
+# #![allow(unused_attribute)]
 // `lib.rs`
 // ...
 
@@ -3208,6 +3210,7 @@ Now for something that you can actually compile yourself.
 We define two crates, and use one of them as a library in the other.
 
 ~~~~
+# #![allow(unused_attribute)]
 // `world.rs`
 #![crate_id = "world#0.42"]
 
@@ -3282,11 +3285,13 @@ fn main() {
 Both auto-insertions can be disabled with an attribute if necessary:
 
 ~~~
+# #![allow(unused_attribute)]
 // In the crate root:
 #![no_std]
 ~~~
 
 ~~~
+# #![allow(unused_attribute)]
 // In any module:
 #![no_implicit_prelude]
 ~~~

src/librustc/back/svh.rs

@@ -91,7 +91,12 @@ impl Svh {
         // types and then use hash_content.  But, since all crate
         // attributes should appear near beginning of the file, it is
         // not such a big deal to be sensitive to their spans for now.
-        krate.attrs.hash(&mut state);
+        //
+        // We hash only the MetaItems instead of the entire Attribute
+        // to avoid hashing the AttrId
+        for attr in krate.attrs.iter() {
+            attr.node.value.hash(&mut state);
+        }
 
         let hash = state.result();
         return Svh {

src/librustc/driver/driver.rs

@@ -716,6 +716,45 @@ fn print_flowgraph<W:io::Writer>(analysis: CrateAnalysis,
 
 pub fn collect_crate_types(session: &Session,
                            attrs: &[ast::Attribute]) -> Vec<config::CrateType> {
+    // Unconditionally collect crate types from attributes to make them used
+    let attr_types: Vec<config::CrateType> = attrs.iter().filter_map(|a| {
+        if a.check_name("crate_type") {
+            match a.value_str() {
+                Some(ref n) if n.equiv(&("rlib")) => {
+                    Some(config::CrateTypeRlib)
+                }
+                Some(ref n) if n.equiv(&("dylib")) => {
+                    Some(config::CrateTypeDylib)
+                }
+                Some(ref n) if n.equiv(&("lib")) => {
+                    Some(config::default_lib_output())
+                }
+                Some(ref n) if n.equiv(&("staticlib")) => {
+                    Some(config::CrateTypeStaticlib)
+                }
+                Some(ref n) if n.equiv(&("bin")) => Some(config::CrateTypeExecutable),
+                Some(_) => {
+                    session.add_lint(lint::UnknownCrateType,
+                                     ast::CRATE_NODE_ID,
+                                     a.span,
+                                     "invalid `crate_type` \
+                                      value".to_strbuf());
+                    None
+                }
+                _ => {
+                    session.add_lint(lint::UnknownCrateType,
+                                     ast::CRATE_NODE_ID,
+                                     a.span,
+                                     "`crate_type` requires a \
+                                      value".to_strbuf());
+                    None
+                }
+            }
+        } else {
+            None
+        }
+    }).collect();
+
     // If we're generating a test executable, then ignore all other output
     // styles at all other locations
     if session.opts.test {
@@ -729,44 +768,7 @@ pub fn collect_crate_types(session: &Session,
     if base.len() > 0 {
         return base
     } else {
-        let iter = attrs.iter().filter_map(|a| {
-            if a.name().equiv(&("crate_type")) {
-                match a.value_str() {
-                    Some(ref n) if n.equiv(&("rlib")) => {
-                        Some(config::CrateTypeRlib)
-                    }
-                    Some(ref n) if n.equiv(&("dylib")) => {
-                        Some(config::CrateTypeDylib)
-                    }
-                    Some(ref n) if n.equiv(&("lib")) => {
-                        Some(config::default_lib_output())
-                    }
-                    Some(ref n) if n.equiv(&("staticlib")) => {
-                        Some(config::CrateTypeStaticlib)
-                    }
-                    Some(ref n) if n.equiv(&("bin")) => Some(config::CrateTypeExecutable),
-                    Some(_) => {
-                        session.add_lint(lint::UnknownCrateType,
-                                         ast::CRATE_NODE_ID,
-                                         a.span,
-                                         "invalid `crate_type` \
-                                          value".to_strbuf());
-                        None
-                    }
-                    _ => {
-                        session.add_lint(lint::UnknownCrateType,
-                                         ast::CRATE_NODE_ID,
-                                         a.span,
-                                         "`crate_type` requires a \
-                                          value".to_strbuf());
-                        None
-                    }
-                }
-            } else {
-                None
-            }
-        });
-        base.extend(iter);
+        base.extend(attr_types.move_iter());
         if base.len() == 0 {
             base.push(config::CrateTypeExecutable);
         }

src/librustc/front/feature_gate.rs

@@ -327,7 +327,7 @@ pub fn check_crate(sess: &Session, krate: &ast::Crate) {
     };
 
     for attr in krate.attrs.iter() {
-        if !attr.name().equiv(&("feature")) {
+        if !attr.check_name("feature") {
             continue
         }
 

src/librustc/front/std_inject.rs

@@ -78,7 +78,7 @@ impl<'a> fold::Folder for StandardLibraryInjector<'a> {
                                          with_version("std"),
                                          ast::DUMMY_NODE_ID),
             attrs: vec!(
-                attr::mk_attr_outer(attr::mk_list_item(
+                attr::mk_attr_outer(attr::mk_attr_id(), attr::mk_list_item(
                         InternedString::new("phase"),
                         vec!(
                             attr::mk_word_item(InternedString::new("syntax")),
@@ -110,10 +110,13 @@ impl<'a> fold::Folder for StandardLibraryInjector<'a> {
         // Add it during the prelude injection instead.
 
         // Add #![feature(phase)] here, because we use #[phase] on extern crate std.
-        let feat_phase_attr = attr::mk_attr_inner(attr::mk_list_item(
+        let feat_phase_attr = attr::mk_attr_inner(attr::mk_attr_id(),
+                                                  attr::mk_list_item(
                                   InternedString::new("feature"),
                                   vec![attr::mk_word_item(InternedString::new("phase"))],
                               ));
+        // std_inject runs after feature checking so manually mark this attr
+        attr::mark_used(&feat_phase_attr);
         krate.attrs.push(feat_phase_attr);
 
         krate
@@ -138,19 +141,25 @@ impl<'a> fold::Folder for PreludeInjector<'a> {
         // This must happen here and not in StandardLibraryInjector because this
         // fold happens second.
 
-        let no_std_attr = attr::mk_attr_inner(attr::mk_word_item(InternedString::new("no_std")));
+        let no_std_attr = attr::mk_attr_inner(attr::mk_attr_id(),
+                                              attr::mk_word_item(InternedString::new("no_std")));
+        // std_inject runs after feature checking so manually mark this attr
+        attr::mark_used(&no_std_attr);
         krate.attrs.push(no_std_attr);
 
         if !no_prelude(krate.attrs.as_slice()) {
             // only add `use std::prelude::*;` if there wasn't a
             // `#![no_implicit_prelude]` at the crate level.
 
             // fold_mod() will insert glob path.
-            let globs_attr = attr::mk_attr_inner(attr::mk_list_item(
+            let globs_attr = attr::mk_attr_inner(attr::mk_attr_id(),
+                                                 attr::mk_list_item(
                 InternedString::new("feature"),
                 vec!(
                     attr::mk_word_item(InternedString::new("globs")),
                 )));
+            // std_inject runs after feature checking so manually mark this attr
+            attr::mark_used(&globs_attr);
             krate.attrs.push(globs_attr);
 
             krate.module = self.fold_mod(&krate.module);

src/librustc/front/test.rs

@@ -253,7 +253,7 @@ fn is_bench_fn(cx: &TestCtxt, i: @ast::Item) -> bool {
 fn is_ignored(cx: &TestCtxt, i: @ast::Item) -> bool {
     i.attrs.iter().any(|attr| {
         // check ignore(cfg(foo, bar))
-        attr.name().equiv(&("ignore")) && match attr.meta_item_list() {
+        attr.check_name("ignore") && match attr.meta_item_list() {
             Some(ref cfgs) => {
                 attr::test_cfg(cx.config.as_slice(), cfgs.iter().map(|x| *x))
             }
@@ -341,7 +341,8 @@ fn mk_test_module(cx: &TestCtxt) -> @ast::Item {
     // This attribute tells resolve to let us call unexported functions
     let resolve_unexported_str = InternedString::new("!resolve_unexported");
     let resolve_unexported_attr =
-        attr::mk_attr_inner(attr::mk_word_item(resolve_unexported_str));
+        attr::mk_attr_inner(attr::mk_attr_id(),
+                            attr::mk_word_item(resolve_unexported_str));
 
     let item = ast::Item {
         ident: token::str_to_ident("__test"),

src/librustc/metadata/csearch.rs

@@ -176,7 +176,7 @@ pub fn get_static_methods_if_impl(cstore: &cstore::CStore,
 
 pub fn get_item_attrs(cstore: &cstore::CStore,
                       def_id: ast::DefId,
-                      f: |Vec<@ast::MetaItem> |) {
+                      f: |Vec<ast::Attribute> |) {
     let cdata = cstore.get_crate_data(def_id.krate);
     decoder::get_item_attrs(&*cdata, def_id.node, f)
 }

src/librustc/metadata/decoder.rs

@@ -953,20 +953,14 @@ pub fn get_tuple_struct_definition_if_ctor(cdata: Cmd,
 
 pub fn get_item_attrs(cdata: Cmd,
                       orig_node_id: ast::NodeId,
-                      f: |Vec<@ast::MetaItem> |) {
+                      f: |Vec<ast::Attribute>|) {
     // The attributes for a tuple struct are attached to the definition, not the ctor;
     // we assume that someone passing in a tuple struct ctor is actually wanting to
     // look at the definition
     let node_id = get_tuple_struct_definition_if_ctor(cdata, orig_node_id);
     let node_id = node_id.map(|x| x.node).unwrap_or(orig_node_id);
     let item = lookup_item(node_id, cdata.data());
-    reader::tagged_docs(item, tag_attributes, |attributes| {
-        reader::tagged_docs(attributes, tag_attribute, |attribute| {
-            f(get_meta_items(attribute));
-            true
-        });
-        true
-    });
+    f(get_attributes(item));
 }
 
 fn struct_field_family_to_visibility(family: Family) -> ast::Visibility {
@@ -1056,6 +1050,7 @@ fn get_attributes(md: ebml::Doc) -> Vec<ast::Attribute> {
             attrs.push(
                 codemap::Spanned {
                     node: ast::Attribute_ {
+                        id: attr::mk_attr_id(),
                         style: ast::AttrOuter,
                         value: meta_item,
                         is_sugared_doc: false,

src/librustc/metadata/encoder.rs

@@ -1436,7 +1436,7 @@ fn synthesize_crate_attrs(ecx: &EncodeContext,
     fn synthesize_crateid_attr(ecx: &EncodeContext) -> Attribute {
         assert!(!ecx.link_meta.crateid.name.is_empty());
 
-        attr::mk_attr_inner(
+        attr::mk_attr_inner(attr::mk_attr_id(),
             attr::mk_name_value_item_str(
                 InternedString::new("crate_id"),
                 token::intern_and_get_ident(ecx.link_meta
@@ -1447,7 +1447,7 @@ fn synthesize_crate_attrs(ecx: &EncodeContext,
 
     let mut attrs = Vec::new();
     for attr in krate.attrs.iter() {
-        if !attr.name().equiv(&("crate_id")) {
+        if !attr.check_name("crate_id") {
             attrs.push(*attr);
         }
     }