Auto merge of #37824 - jseyfried:symbols, r=eddyb

Clean up `ast::Attribute`, `ast::CrateConfig`, and string interning

This PR
 - removes `ast::Attribute_` (changing `Attribute` from `Spanned<Attribute_>` to a struct),
 - moves a `MetaItem`'s name from the `MetaItemKind` variants to a field of `MetaItem`,
 - avoids needlessly wrapping `ast::MetaItem` with `P`,
 - moves string interning into `syntax::symbol` (`ast::Name` is a reexport of `symbol::Symbol` for now),
 - replaces `InternedString` with `Symbol` in the AST, HIR, and various other places, and
 - refactors `ast::CrateConfig` from a `Vec` to a `HashSet`.

r? @eddyb

7 files changed, 66 insertions, 65 deletions

diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs
index 56def16ea45..269fb1dcea4 100644
--- a/src/librustc_metadata/creader.rs
+++ b/src/librustc_metadata/creader.rs
@@ -37,7 +37,7 @@ use syntax::abi::Abi;
 use syntax::attr;
 use syntax::ext::base::SyntaxExtension;
 use syntax::feature_gate::{self, GateIssue};
-use syntax::parse::token::{InternedString, intern};
+use syntax::symbol::Symbol;
 use syntax_pos::{Span, DUMMY_SP};
 use log;
 
@@ -52,7 +52,7 @@ pub struct CrateLoader<'a> {
     cstore: &'a CStore,
     next_crate_num: CrateNum,
     foreign_item_map: FxHashMap<String, Vec<ast::NodeId>>,
-    local_crate_name: String,
+    local_crate_name: Symbol,
 }
 
 fn dump_crates(cstore: &CStore) {
@@ -70,8 +70,8 @@ fn dump_crates(cstore: &CStore) {
 
 #[derive(Debug)]
 struct ExternCrateInfo {
-    ident: String,
-    name: String,
+    ident: Symbol,
+    name: Symbol,
     id: ast::NodeId,
     dep_kind: DepKind,
 }
@@ -80,7 +80,7 @@ fn register_native_lib(sess: &Session,
                        cstore: &CStore,
                        span: Option<Span>,
                        lib: NativeLibrary) {
-    if lib.name.is_empty() {
+    if lib.name.as_str().is_empty() {
         match span {
             Some(span) => {
                 struct_span_err!(sess, span, E0454,
@@ -147,7 +147,7 @@ impl<'a> CrateLoader<'a> {
             cstore: cstore,
             next_crate_num: cstore.next_crate_num(),
             foreign_item_map: FxHashMap(),
-            local_crate_name: local_crate_name.to_owned(),
+            local_crate_name: Symbol::intern(local_crate_name),
         }
     }
 
@@ -160,12 +160,12 @@ impl<'a> CrateLoader<'a> {
                     Some(name) => {
                         validate_crate_name(Some(self.sess), &name.as_str(),
                                             Some(i.span));
-                        name.to_string()
+                        name
                     }
-                    None => i.ident.to_string(),
+                    None => i.ident.name,
                 };
                 Some(ExternCrateInfo {
-                    ident: i.ident.to_string(),
+                    ident: i.ident.name,
                     name: name,
                     id: i.id,
                     dep_kind: if attr::contains_name(&i.attrs, "no_link") {
@@ -179,7 +179,7 @@ impl<'a> CrateLoader<'a> {
         }
     }
 
-    fn existing_match(&self, name: &str, hash: Option<&Svh>, kind: PathKind)
+    fn existing_match(&self, name: Symbol, hash: Option<&Svh>, kind: PathKind)
                       -> Option<CrateNum> {
         let mut ret = None;
         self.cstore.iter_crate_data(|cnum, data| {
@@ -201,7 +201,7 @@ impl<'a> CrateLoader<'a> {
             // `source` stores paths which are normalized which may be different
             // from the strings on the command line.
             let source = self.cstore.used_crate_source(cnum);
-            if let Some(locs) = self.sess.opts.externs.get(name) {
+            if let Some(locs) = self.sess.opts.externs.get(&*name.as_str()) {
                 let found = locs.iter().any(|l| {
                     let l = fs::canonicalize(l).ok();
                     source.dylib.as_ref().map(|p| &p.0) == l.as_ref() ||
@@ -233,7 +233,7 @@ impl<'a> CrateLoader<'a> {
                                   root: &CrateRoot) {
         // Check for (potential) conflicts with the local crate
         if self.local_crate_name == root.name &&
-           self.sess.local_crate_disambiguator() == &root.disambiguator[..] {
+           self.sess.local_crate_disambiguator() == root.disambiguator {
             span_fatal!(self.sess, span, E0519,
                         "the current crate is indistinguishable from one of its \
                          dependencies: it has the same crate-name `{}` and was \
@@ -258,8 +258,8 @@ impl<'a> CrateLoader<'a> {
 
     fn register_crate(&mut self,
                       root: &Option<CratePaths>,
-                      ident: &str,
-                      name: &str,
+                      ident: Symbol,
+                      name: Symbol,
                       span: Span,
                       lib: Library,
                       dep_kind: DepKind)
@@ -290,7 +290,7 @@ impl<'a> CrateLoader<'a> {
         let cnum_map = self.resolve_crate_deps(root, &crate_root, &metadata, cnum, span, dep_kind);
 
         let cmeta = Rc::new(cstore::CrateMetadata {
-            name: name.to_string(),
+            name: name,
             extern_crate: Cell::new(None),
             key_map: metadata.load_key_map(crate_root.index),
             proc_macros: crate_root.macro_derive_registrar.map(|_| {
@@ -314,8 +314,8 @@ impl<'a> CrateLoader<'a> {
 
     fn resolve_crate(&mut self,
                      root: &Option<CratePaths>,
-                     ident: &str,
-                     name: &str,
+                     ident: Symbol,
+                     name: Symbol,
                      hash: Option<&Svh>,
                      span: Span,
                      path_kind: PathKind,
@@ -459,13 +459,12 @@ impl<'a> CrateLoader<'a> {
         let deps = crate_root.crate_deps.decode(metadata);
         let map: FxHashMap<_, _> = deps.enumerate().map(|(crate_num, dep)| {
             debug!("resolving dep crate {} hash: `{}`", dep.name, dep.hash);
-            let dep_name = &dep.name.as_str();
             let dep_kind = match dep_kind {
                 DepKind::MacrosOnly => DepKind::MacrosOnly,
                 _ => dep.kind,
             };
             let (local_cnum, ..) = self.resolve_crate(
-                root, dep_name, dep_name, Some(&dep.hash), span, PathKind::Dependency, dep_kind,
+                root, dep.name, dep.name, Some(&dep.hash), span, PathKind::Dependency, dep_kind,
             );
             (CrateNum::new(crate_num + 1), local_cnum)
         }).collect();
@@ -485,13 +484,11 @@ impl<'a> CrateLoader<'a> {
         let target_triple = &self.sess.opts.target_triple[..];
         let is_cross = target_triple != config::host_triple();
         let mut target_only = false;
-        let ident = info.ident.clone();
-        let name = info.name.clone();
         let mut locate_ctxt = locator::Context {
             sess: self.sess,
             span: span,
-            ident: &ident[..],
-            crate_name: &name[..],
+            ident: info.ident,
+            crate_name: info.name,
             hash: None,
             filesearch: self.sess.host_filesearch(PathKind::Crate),
             target: &self.sess.host,
@@ -585,11 +582,11 @@ impl<'a> CrateLoader<'a> {
                                       trait_name: &str,
                                       expand: fn(TokenStream) -> TokenStream,
                                       attributes: &[&'static str]) {
-                let attrs = attributes.iter().map(|s| InternedString::new(s)).collect();
+                let attrs = attributes.iter().cloned().map(Symbol::intern).collect();
                 let derive = SyntaxExtension::CustomDerive(
                     Box::new(CustomDerive::new(expand, attrs))
                 );
-                self.0.push((intern(trait_name), Rc::new(derive)));
+                self.0.push((Symbol::intern(trait_name), Rc::new(derive)));
             }
         }
 
@@ -607,8 +604,8 @@ impl<'a> CrateLoader<'a> {
     pub fn find_plugin_registrar(&mut self, span: Span, name: &str)
                                  -> Option<(PathBuf, Svh, DefIndex)> {
         let ekrate = self.read_extension_crate(span, &ExternCrateInfo {
-             name: name.to_string(),
-             ident: name.to_string(),
+             name: Symbol::intern(name),
+             ident: Symbol::intern(name),
              id: ast::DUMMY_NODE_ID,
              dep_kind: DepKind::MacrosOnly,
         });
@@ -645,7 +642,7 @@ impl<'a> CrateLoader<'a> {
         let libs = self.cstore.get_used_libraries();
         for (foreign_lib, list) in self.foreign_item_map.iter() {
             let is_static = libs.borrow().iter().any(|lib| {
-                *foreign_lib == lib.name && lib.kind == cstore::NativeStatic
+                lib.name == &**foreign_lib && lib.kind == cstore::NativeStatic
             });
             if is_static {
                 for id in list {
@@ -708,8 +705,8 @@ impl<'a> CrateLoader<'a> {
         // in terms of everyone has a compatible panic runtime format, that's
         // performed later as part of the `dependency_format` module.
         let name = match desired_strategy {
-            PanicStrategy::Unwind => "panic_unwind",
-            PanicStrategy::Abort => "panic_abort",
+            PanicStrategy::Unwind => Symbol::intern("panic_unwind"),
+            PanicStrategy::Abort => Symbol::intern("panic_abort"),
         };
         info!("panic runtime not found -- loading {}", name);
 
@@ -791,9 +788,9 @@ impl<'a> CrateLoader<'a> {
         // * Staticlibs and Rust dylibs use system malloc
         // * Rust dylibs used as dependencies to rust use jemalloc
         let name = if need_lib_alloc && !self.sess.opts.cg.prefer_dynamic {
-            &self.sess.target.target.options.lib_allocation_crate
+            Symbol::intern(&self.sess.target.target.options.lib_allocation_crate)
         } else {
-            &self.sess.target.target.options.exe_allocation_crate
+            Symbol::intern(&self.sess.target.target.options.exe_allocation_crate)
         };
         let dep_kind = DepKind::Implicit;
         let (cnum, data) =
@@ -855,8 +852,8 @@ impl<'a> CrateLoader<'a> {
 impl<'a> CrateLoader<'a> {
     pub fn preprocess(&mut self, krate: &ast::Crate) {
         for attr in krate.attrs.iter().filter(|m| m.name() == "link_args") {
-            if let Some(ref linkarg) = attr.value_str() {
-                self.cstore.add_used_link_args(&linkarg);
+            if let Some(linkarg) = attr.value_str() {
+                self.cstore.add_used_link_args(&linkarg.as_str());
             }
         }
     }
@@ -869,7 +866,7 @@ impl<'a> CrateLoader<'a> {
         // First, add all of the custom #[link_args] attributes
         for m in i.attrs.iter().filter(|a| a.check_name("link_args")) {
             if let Some(linkarg) = m.value_str() {
-                self.cstore.add_used_link_args(&linkarg);
+                self.cstore.add_used_link_args(&linkarg.as_str());
             }
         }
 
@@ -881,7 +878,7 @@ impl<'a> CrateLoader<'a> {
             };
             let kind = items.iter().find(|k| {
                 k.check_name("kind")
-            }).and_then(|a| a.value_str());
+            }).and_then(|a| a.value_str()).map(Symbol::as_str);
             let kind = match kind.as_ref().map(|s| &s[..]) {
                 Some("static") => cstore::NativeStatic,
                 Some("dylib") => cstore::NativeUnknown,
@@ -903,7 +900,7 @@ impl<'a> CrateLoader<'a> {
                     struct_span_err!(self.sess, m.span, E0459,
                                      "#[link(...)] specified without `name = \"foo\"`")
                         .span_label(m.span, &format!("missing `name` argument")).emit();
-                    InternedString::new("foo")
+                    Symbol::intern("foo")
                 }
             };
             let cfg = items.iter().find(|k| {
@@ -913,7 +910,7 @@ impl<'a> CrateLoader<'a> {
                 list[0].meta_item().unwrap().clone()
             });
             let lib = NativeLibrary {
-                name: n.to_string(),
+                name: n,
                 kind: kind,
                 cfg: cfg,
             };
@@ -944,7 +941,7 @@ impl<'a> middle::cstore::CrateLoader for CrateLoader<'a> {
 
         for &(ref name, kind) in &self.sess.opts.libs {
             let lib = NativeLibrary {
-                name: name.clone(),
+                name: Symbol::intern(name),
                 kind: kind,
                 cfg: None,
             };
@@ -962,7 +959,7 @@ impl<'a> middle::cstore::CrateLoader for CrateLoader<'a> {
 
         let info = self.extract_crate_info(item).unwrap();
         let (cnum, ..) = self.resolve_crate(
-            &None, &info.ident, &info.name, None, item.span, PathKind::Crate, info.dep_kind,
+            &None, info.ident, info.name, None, item.span, PathKind::Crate, info.dep_kind,
         );
 
         let def_id = definitions.opt_local_def_id(item.id).unwrap();
diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs
index 37853b7473a..ce47b936ddc 100644
--- a/src/librustc_metadata/cstore.rs
+++ b/src/librustc_metadata/cstore.rs
@@ -29,6 +29,7 @@ use std::path::PathBuf;
 use flate::Bytes;
 use syntax::{ast, attr};
 use syntax::ext::base::SyntaxExtension;
+use syntax::symbol::Symbol;
 use syntax_pos;
 
 pub use rustc::middle::cstore::{NativeLibrary, LinkagePreference};
@@ -58,7 +59,7 @@ pub struct ImportedFileMap {
 }
 
 pub struct CrateMetadata {
-    pub name: String,
+    pub name: Symbol,
 
     /// Information about the extern crate that caused this crate to
     /// be loaded. If this is `None`, then the crate was injected
@@ -213,7 +214,7 @@ impl CStore {
     }
 
     pub fn add_used_library(&self, lib: NativeLibrary) {
-        assert!(!lib.name.is_empty());
+        assert!(!lib.name.as_str().is_empty());
         self.used_libraries.borrow_mut().push(lib);
     }
 
@@ -249,14 +250,14 @@ impl CStore {
 }
 
 impl CrateMetadata {
-    pub fn name(&self) -> &str {
-        &self.root.name
+    pub fn name(&self) -> Symbol {
+        self.root.name
     }
     pub fn hash(&self) -> Svh {
         self.root.hash
     }
-    pub fn disambiguator(&self) -> &str {
-        &self.root.disambiguator
+    pub fn disambiguator(&self) -> Symbol {
+        self.root.disambiguator
     }
 
     pub fn is_staged_api(&self) -> bool {
diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs
index 2018d829597..407166203de 100644
--- a/src/librustc_metadata/cstore_impl.rs
+++ b/src/librustc_metadata/cstore_impl.rs
@@ -31,7 +31,8 @@ use rustc_back::PanicStrategy;
 use std::path::PathBuf;
 use syntax::ast;
 use syntax::attr;
-use syntax::parse::{token, new_parser_from_source_str};
+use syntax::parse::new_parser_from_source_str;
+use syntax::symbol::Symbol;
 use syntax_pos::mk_sp;
 use rustc::hir::svh::Svh;
 use rustc_back::target::Target;
@@ -262,14 +263,14 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
         self.get_crate_data(cnum).panic_strategy()
     }
 
-    fn crate_name(&self, cnum: CrateNum) -> token::InternedString
+    fn crate_name(&self, cnum: CrateNum) -> Symbol
     {
-        token::intern_and_get_ident(&self.get_crate_data(cnum).name[..])
+        self.get_crate_data(cnum).name
     }
 
-    fn original_crate_name(&self, cnum: CrateNum) -> token::InternedString
+    fn original_crate_name(&self, cnum: CrateNum) -> Symbol
     {
-        token::intern_and_get_ident(&self.get_crate_data(cnum).name())
+        self.get_crate_data(cnum).name()
     }
 
     fn extern_crate(&self, cnum: CrateNum) -> Option<ExternCrate>
@@ -282,9 +283,9 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
         self.get_crate_hash(cnum)
     }
 
-    fn crate_disambiguator(&self, cnum: CrateNum) -> token::InternedString
+    fn crate_disambiguator(&self, cnum: CrateNum) -> Symbol
     {
-        token::intern_and_get_ident(&self.get_crate_data(cnum).disambiguator())
+        self.get_crate_data(cnum).disambiguator()
     }
 
     fn plugin_registrar_fn(&self, cnum: CrateNum) -> Option<DefId>
diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs
index 3af9d291ae5..fb1314992c0 100644
--- a/src/librustc_metadata/decoder.rs
+++ b/src/librustc_metadata/decoder.rs
@@ -934,7 +934,7 @@ impl<'a, 'tcx> CrateMetadata {
             .decode(self)
             .map(|mut attr| {
                 // Need new unique IDs: old thread-local IDs won't map to new threads.
-                attr.node.id = attr::mk_attr_id();
+                attr.id = attr::mk_attr_id();
                 attr
             })
             .collect()
diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs
index 3ab542442a1..2f4b0d5c87b 100644
--- a/src/librustc_metadata/encoder.rs
+++ b/src/librustc_metadata/encoder.rs
@@ -34,7 +34,7 @@ use std::rc::Rc;
 use std::u32;
 use syntax::ast::{self, CRATE_NODE_ID};
 use syntax::attr;
-use syntax;
+use syntax::symbol::Symbol;
 use syntax_pos;
 
 use rustc::hir::{self, PatKind};
@@ -600,7 +600,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             if let PatKind::Binding(_, ref path1, _) = arg.pat.node {
                 path1.node
             } else {
-                syntax::parse::token::intern("")
+                Symbol::intern("")
             }
         }))
     }
@@ -1119,7 +1119,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         let deps = get_ordered_deps(self.cstore);
         self.lazy_seq(deps.iter().map(|&(_, ref dep)| {
             CrateDep {
-                name: syntax::parse::token::intern(dep.name()),
+                name: dep.name(),
                 hash: dep.hash(),
                 kind: dep.dep_kind.get(),
             }
@@ -1279,10 +1279,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         let is_proc_macro = tcx.sess.crate_types.borrow().contains(&CrateTypeProcMacro);
         let root = self.lazy(&CrateRoot {
             rustc_version: rustc_version(),
-            name: link_meta.crate_name.clone(),
+            name: link_meta.crate_name,
             triple: tcx.sess.opts.target_triple.clone(),
             hash: link_meta.crate_hash,
-            disambiguator: tcx.sess.local_crate_disambiguator().to_string(),
+            disambiguator: tcx.sess.local_crate_disambiguator(),
             panic_strategy: tcx.sess.panic_strategy(),
             plugin_registrar_fn: tcx.sess
                 .plugin_registrar_fn
diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs
index b677a63edc0..f5196f7ea84 100644
--- a/src/librustc_metadata/locator.rs
+++ b/src/librustc_metadata/locator.rs
@@ -227,6 +227,7 @@ use rustc_llvm as llvm;
 use rustc_llvm::{False, ObjectFile, mk_section_iter};
 use rustc_llvm::archive_ro::ArchiveRO;
 use errors::DiagnosticBuilder;
+use syntax::symbol::Symbol;
 use syntax_pos::Span;
 use rustc_back::target::Target;
 
@@ -249,8 +250,8 @@ pub struct CrateMismatch {
 pub struct Context<'a> {
     pub sess: &'a Session,
     pub span: Span,
-    pub ident: &'a str,
-    pub crate_name: &'a str,
+    pub ident: Symbol,
+    pub crate_name: Symbol,
     pub hash: Option<&'a Svh>,
     // points to either self.sess.target.target or self.sess.host, must match triple
     pub target: &'a Target,
@@ -422,7 +423,7 @@ impl<'a> Context<'a> {
         // must be loaded via -L plus some filtering.
         if self.hash.is_none() {
             self.should_match_name = false;
-            if let Some(s) = self.sess.opts.externs.get(self.crate_name) {
+            if let Some(s) = self.sess.opts.externs.get(&self.crate_name.as_str()) {
                 return self.find_commandline_library(s.iter());
             }
             self.should_match_name = true;
@@ -533,7 +534,7 @@ impl<'a> Context<'a> {
                     if let Some((ref p, _)) = lib.rlib {
                         err.note(&format!("path: {}", p.display()));
                     }
-                    note_crate_name(&mut err, &lib.metadata.get_root().name);
+                    note_crate_name(&mut err, &lib.metadata.get_root().name.as_str());
                 }
                 err.emit();
                 None
diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs
index 32c8c5e2ee8..e11719dc40f 100644
--- a/src/librustc_metadata/schema.rs
+++ b/src/librustc_metadata/schema.rs
@@ -22,6 +22,7 @@ use rustc_back::PanicStrategy;
 
 use rustc_serialize as serialize;
 use syntax::{ast, attr};
+use syntax::symbol::Symbol;
 use syntax_pos::{self, Span};
 
 use std::marker::PhantomData;
@@ -163,10 +164,10 @@ pub enum LazyState {
 #[derive(RustcEncodable, RustcDecodable)]
 pub struct CrateRoot {
     pub rustc_version: String,
-    pub name: String,
+    pub name: Symbol,
     pub triple: String,
     pub hash: hir::svh::Svh,
-    pub disambiguator: String,
+    pub disambiguator: Symbol,
     pub panic_strategy: PanicStrategy,
     pub plugin_registrar_fn: Option<DefIndex>,
     pub macro_derive_registrar: Option<DefIndex>,