Auto merge of #38024 - jseyfried:avoid_needless_proc_macro_deps, r=nrc

Avoid loading needless proc-macro dependencies Fixes #37958 when no proc-macros are exported; in particular, without `pub extern crate proc_macros;`, `#![feature(macro_reexport)]`, or `#![feature(use_extern_macros)]`. I opened rust-lang/cargo#3334 for exported proc macros. r? @alexcrichton
rust-lang · 39c267a8d5ab141faaf5d4b33a20cac62cdc4507 · bors committed on GitHub Nov 28, 2016 · Nov 28, 2016 · 39c267a
2 parents 0b399e5 + 1fd9041
commit 39c267a8d5ab141faaf5d4b33a20cac62cdc4507
diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs
@@ -67,6 +67,9 @@ pub struct CrateSource {
 
 #[derive(RustcEncodable, RustcDecodable, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Debug)]
 pub enum DepKind {
+    /// A dependency that is only used for its macros, none of which are visible from other crates.
+    /// These are included in the metadata only as placeholders and are ignored when decoding.
+    UnexportedMacrosOnly,
     /// A dependency that is only used for its macros.
     MacrosOnly,
     /// A dependency that is always injected into the dependency list and so
@@ -77,6 +80,15 @@ pub enum DepKind {
     Explicit,
 }
 
+impl DepKind {
+    pub fn macros_only(self) -> bool {
+        match self {
+            DepKind::UnexportedMacrosOnly | DepKind::MacrosOnly => true,
+            DepKind::Implicit | DepKind::Explicit => false,
+        }
+    }
+}
+
 #[derive(PartialEq, Clone, Debug)]
 pub enum LibSource {
     Some(PathBuf),
@@ -220,6 +232,7 @@ pub trait CrateStore<'tcx> {
     fn dylib_dependency_formats(&self, cnum: CrateNum)
                                     -> Vec<(CrateNum, LinkagePreference)>;
     fn dep_kind(&self, cnum: CrateNum) -> DepKind;
+    fn export_macros(&self, cnum: CrateNum);
     fn lang_items(&self, cnum: CrateNum) -> Vec<(DefIndex, usize)>;
     fn missing_lang_items(&self, cnum: CrateNum) -> Vec<lang_items::LangItem>;
     fn is_staged_api(&self, cnum: CrateNum) -> bool;
@@ -393,6 +406,7 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
         { bug!("missing_lang_items") }
     fn is_staged_api(&self, cnum: CrateNum) -> bool { bug!("is_staged_api") }
     fn dep_kind(&self, cnum: CrateNum) -> DepKind { bug!("is_explicitly_linked") }
+    fn export_macros(&self, cnum: CrateNum) { bug!("export_macros") }
     fn is_allocator(&self, cnum: CrateNum) -> bool { bug!("is_allocator") }
     fn is_panic_runtime(&self, cnum: CrateNum) -> bool { bug!("is_panic_runtime") }
     fn is_compiler_builtins(&self, cnum: CrateNum) -> bool { bug!("is_compiler_builtins") }
diff --git a/src/librustc/middle/dependency_format.rs b/src/librustc/middle/dependency_format.rs
@@ -124,7 +124,7 @@ fn calculate_type(sess: &session::Session,
                 return v;
             }
             for cnum in sess.cstore.crates() {
-                if sess.cstore.dep_kind(cnum) == DepKind::MacrosOnly { continue }
+                if sess.cstore.dep_kind(cnum).macros_only() { continue }
                 let src = sess.cstore.used_crate_source(cnum);
                 if src.rlib.is_some() { continue }
                 sess.err(&format!("dependency `{}` not found in rlib format",
@@ -157,7 +157,7 @@ fn calculate_type(sess: &session::Session,
     // dependencies, ensuring there are no conflicts. The only valid case for a
     // dependency to be relied upon twice is for both cases to rely on a dylib.
     for cnum in sess.cstore.crates() {
-        if sess.cstore.dep_kind(cnum) == DepKind::MacrosOnly { continue }
+        if sess.cstore.dep_kind(cnum).macros_only() { continue }
         let name = sess.cstore.crate_name(cnum);
         let src = sess.cstore.used_crate_source(cnum);
         if src.dylib.is_some() {
diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs
@@ -171,7 +171,7 @@ impl<'a> CrateLoader<'a> {
                     name: name,
                     id: i.id,
                     dep_kind: if attr::contains_name(&i.attrs, "no_link") {
-                        DepKind::MacrosOnly
+                        DepKind::UnexportedMacrosOnly
                     } else {
                         DepKind::Explicit
                     },
@@ -350,7 +350,7 @@ impl<'a> CrateLoader<'a> {
             };
 
             self.load(&mut locate_ctxt).or_else(|| {
-                dep_kind = DepKind::MacrosOnly;
+                dep_kind = DepKind::UnexportedMacrosOnly;
 
                 let mut proc_macro_locator = locator::Context {
                     target: &self.sess.host,
@@ -373,7 +373,7 @@ impl<'a> CrateLoader<'a> {
             LoadResult::Previous(cnum) => {
                 let data = self.cstore.get_crate_data(cnum);
                 if data.root.macro_derive_registrar.is_some() {
-                    dep_kind = DepKind::MacrosOnly;
+                    dep_kind = DepKind::UnexportedMacrosOnly;
                 }
                 data.dep_kind.set(cmp::max(data.dep_kind.get(), dep_kind));
                 (cnum, data)
@@ -460,28 +460,23 @@ impl<'a> CrateLoader<'a> {
             return cstore::CrateNumMap::new();
         }
 
-        // The map from crate numbers in the crate we're resolving to local crate
-        // numbers
-        let deps = crate_root.crate_deps.decode(metadata);
-        let map: FxHashMap<_, _> = deps.enumerate().map(|(crate_num, dep)| {
+        // The map from crate numbers in the crate we're resolving to local crate numbers.
+        // We map 0 and all other holes in the map to our parent crate. The "additional"
+        // self-dependencies should be harmless.
+        ::std::iter::once(krate).chain(crate_root.crate_deps.decode(metadata).map(|dep| {
             debug!("resolving dep crate {} hash: `{}`", dep.name, dep.hash);
+            if dep.kind == DepKind::UnexportedMacrosOnly {
+                return krate;
+            }
             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,
             );
-            (CrateNum::new(crate_num + 1), local_cnum)
-        }).collect();
-
-        let max_cnum = map.values().cloned().max().map(|cnum| cnum.as_u32()).unwrap_or(0);
-
-        // we map 0 and all other holes in the map to our parent crate. The "additional"
-        // self-dependencies should be harmless.
-        (0..max_cnum+1).map(|cnum| {
-            map.get(&CrateNum::from_u32(cnum)).cloned().unwrap_or(krate)
-        }).collect()
+            local_cnum
+        })).collect()
     }
 
     fn read_extension_crate(&mut self, span: Span, info: &ExternCrateInfo) -> ExtensionCrate {
@@ -614,7 +609,7 @@ impl<'a> CrateLoader<'a> {
              name: Symbol::intern(name),
              ident: Symbol::intern(name),
              id: ast::DUMMY_NODE_ID,
-             dep_kind: DepKind::MacrosOnly,
+             dep_kind: DepKind::UnexportedMacrosOnly,
         });
 
         if ekrate.target_only {
diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs
@@ -197,7 +197,7 @@ impl CStore {
             .borrow()
             .iter()
             .filter_map(|(&cnum, data)| {
-                if data.dep_kind.get() == DepKind::MacrosOnly { return None; }
+                if data.dep_kind.get().macros_only() { return None; }
                 let path = match prefer {
                     LinkagePreference::RequireDynamic => data.source.dylib.clone().map(|p| p.0),
                     LinkagePreference::RequireStatic => data.source.rlib.clone().map(|p| p.0),
diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs
@@ -228,6 +228,12 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
         self.get_crate_data(cnum).dep_kind.get()
     }
 
+    fn export_macros(&self, cnum: CrateNum) {
+        if self.get_crate_data(cnum).dep_kind.get() == DepKind::UnexportedMacrosOnly {
+            self.get_crate_data(cnum).dep_kind.set(DepKind::MacrosOnly)
+        }
+    }
+
     fn lang_items(&self, cnum: CrateNum) -> Vec<(DefIndex, usize)>
     {
         self.get_crate_data(cnum).get_lang_items()
diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs
@@ -21,7 +21,7 @@ use rustc::util::nodemap::FxHashMap;
 use rustc::hir;
 use rustc::hir::intravisit::IdRange;
 
-use rustc::middle::cstore::{DepKind, InlinedItem, LinkagePreference};
+use rustc::middle::cstore::{InlinedItem, LinkagePreference};
 use rustc::hir::def::{self, Def, CtorKind};
 use rustc::hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
 use rustc::middle::lang_items;
@@ -729,7 +729,7 @@ impl<'a, 'tcx> CrateMetadata {
         };
 
         // Iterate over all children.
-        let macros_only = self.dep_kind.get() == DepKind::MacrosOnly;
+        let macros_only = self.dep_kind.get().macros_only();
         for child_index in item.children.decode(self) {
             if macros_only {
                 continue
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
@@ -21,7 +21,7 @@ use Namespace::{self, TypeNS, ValueNS, MacroNS};
 use ResolveResult::Success;
 use {resolve_error, resolve_struct_error, ResolutionError};
 
-use rustc::middle::cstore::{DepKind, LoadedMacro};
+use rustc::middle::cstore::LoadedMacro;
 use rustc::hir::def::*;
 use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId};
 use rustc::ty;
@@ -492,7 +492,7 @@ impl<'b> Resolver<'b> {
 
     fn get_extern_crate_root(&mut self, cnum: CrateNum) -> Module<'b> {
         let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX };
-        let macros_only = self.session.cstore.dep_kind(cnum) == DepKind::MacrosOnly;
+        let macros_only = self.session.cstore.dep_kind(cnum).macros_only();
         let arenas = self.arenas;
         *self.extern_crate_roots.entry((cnum, macros_only)).or_insert_with(|| {
             arenas.alloc_module(ModuleS {
@@ -567,7 +567,8 @@ impl<'b> Resolver<'b> {
         if self.current_module.parent.is_some() && legacy_imports != LegacyMacroImports::default() {
             span_err!(self.session, item.span, E0468,
                       "an `extern crate` loading macros must be at the crate root");
-        } else if self.session.cstore.dep_kind(cnum) == DepKind::MacrosOnly &&
+        } else if !self.use_extern_macros &&
+                  self.session.cstore.dep_kind(cnum).macros_only() &&
                   legacy_imports == LegacyMacroImports::default() {
             let msg = "custom derive crates and `#[no_link]` crates have no effect without \
                        `#[macro_use]`";
@@ -590,7 +591,9 @@ impl<'b> Resolver<'b> {
             }
         }
         for (name, span) in legacy_imports.reexports {
-            self.used_crates.insert(module.def_id().unwrap().krate);
+            let krate = module.def_id().unwrap().krate;
+            self.used_crates.insert(krate);
+            self.session.cstore.export_macros(krate);
             let result = self.resolve_name_in_module(module, name, MacroNS, false, None);
             if let Success(binding) = result {
                 self.macro_exports.push(Export { name: name, def: binding.def() });
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
@@ -743,6 +743,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                (binding.is_import() || binding.is_extern_crate()) {
                 let def = binding.def();
                 if def != Def::Err {
+                    if !def.def_id().is_local() {
+                        self.session.cstore.export_macros(def.def_id().krate);
+                    }
                     reexports.push(Export { name: name, def: def });
                 }
             }