Skip to content

Commit

Permalink
Auto merge of #69432 - petrochenkov:alldeps, r=eddyb
Browse files Browse the repository at this point in the history
rustc_metadata: Load metadata for indirect macro-only dependencies

Imagine this dependency chain between crates
```
Executable crate -> Library crate -> Macro crate
```
where "Library crate" uses the macros from "Macro crate" for some code generation, but doesn't reexport them any further.

Currently, when compiling "Executable crate" we don't even load metadata for it, because why would we want to load any metadata from "Macro crate" if it already did all its code generation job when compiling "Library crate".
Right?

Wrong!
Hygiene data and spans (#68686, #68941) from "Macro crate" still may need to be decoded from "Executable crate".
So we'll have to load them properly.

Questions:
- How this will affect compile times for larger crate trees in practice? How to measure it?
Hygiene/span encoding/decoding will necessarily slow down compilation because right now we just don't do some work that we should do, but this introduces a whole new way to slow down things. E.g. loading metadata for `syn` (and its dependencies) when compiling your executable if one of its library dependencies uses it.
- We are currently detecting whether a crate reexports macros from "Macro crate" or not, could we similarly detect whether a crate "reexports spans" and keep it unloaded if it doesn't?
Or at least "reexports important spans" affecting hygiene, we can probably lose spans that only affect diagnostics.
  • Loading branch information
bors committed Mar 2, 2020
2 parents 1cf090e + 487c378 commit 9dc8dad
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 24 deletions.
5 changes: 1 addition & 4 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ impl CrateSource {
HashStable
)]
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
Expand All @@ -69,7 +66,7 @@ pub enum DepKind {
impl DepKind {
pub fn macros_only(self) -> bool {
match self {
DepKind::UnexportedMacrosOnly | DepKind::MacrosOnly => true,
DepKind::MacrosOnly => true,
DepKind::Implicit | DepKind::Explicit => false,
}
}
Expand Down
9 changes: 3 additions & 6 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ impl<'a> CrateLoader<'a> {
self.load(&mut locator)
.map(|r| (r, None))
.or_else(|| {
dep_kind = DepKind::UnexportedMacrosOnly;
dep_kind = DepKind::MacrosOnly;
self.load_proc_macro(&mut locator, path_kind)
})
.ok_or_else(move || LoadError::LocatorError(locator))?
Expand All @@ -473,7 +473,7 @@ impl<'a> CrateLoader<'a> {
(LoadResult::Previous(cnum), None) => {
let data = self.cstore.get_crate_data(cnum);
if data.is_proc_macro_crate() {
dep_kind = DepKind::UnexportedMacrosOnly;
dep_kind = DepKind::MacrosOnly;
}
data.update_dep_kind(|data_dep_kind| cmp::max(data_dep_kind, dep_kind));
Ok(cnum)
Expand Down Expand Up @@ -547,9 +547,6 @@ impl<'a> CrateLoader<'a> {
"resolving dep crate {} hash: `{}` extra filename: `{}`",
dep.name, dep.hash, dep.extra_filename
);
if dep.kind == DepKind::UnexportedMacrosOnly {
return krate;
}
let dep_kind = match dep_kind {
DepKind::MacrosOnly => DepKind::MacrosOnly,
_ => dep.kind,
Expand Down Expand Up @@ -850,7 +847,7 @@ impl<'a> CrateLoader<'a> {
None => item.ident.name,
};
let dep_kind = if attr::contains_name(&item.attrs, sym::no_link) {
DepKind::UnexportedMacrosOnly
DepKind::MacrosOnly
} else {
DepKind::Explicit
};
Expand Down
10 changes: 1 addition & 9 deletions src/librustc_metadata/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::rmeta::{self, encoder};
use rustc::hir::exports::Export;
use rustc::hir::map::definitions::DefPathTable;
use rustc::hir::map::{DefKey, DefPath, DefPathHash};
use rustc::middle::cstore::{CrateSource, CrateStore, DepKind, EncodedMetadata, NativeLibraryKind};
use rustc::middle::cstore::{CrateSource, CrateStore, EncodedMetadata, NativeLibraryKind};
use rustc::middle::exported_symbols::ExportedSymbol;
use rustc::middle::stability::DeprecationEntry;
use rustc::session::{CrateDisambiguator, Session};
Expand Down Expand Up @@ -393,14 +393,6 @@ pub fn provide(providers: &mut Providers<'_>) {
}

impl CStore {
pub fn export_macros_untracked(&self, cnum: CrateNum) {
let data = self.get_crate_data(cnum);
let mut dep_kind = data.dep_kind.lock();
if *dep_kind == DepKind::UnexportedMacrosOnly {
*dep_kind = DepKind::MacrosOnly;
}
}

pub fn struct_field_names_untracked(&self, def: DefId, sess: &Session) -> Vec<Spanned<Symbol>> {
self.get_crate_data(def.krate).get_struct_field_names(def.index, sess)
}
Expand Down
5 changes: 0 additions & 5 deletions src/librustc_resolve/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1403,11 +1403,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
if is_good_import || binding.is_macro_def() {
let res = binding.res();
if res != Res::Err {
if let Some(def_id) = res.opt_def_id() {
if !def_id.is_local() {
this.cstore().export_macros_untracked(def_id.krate);
}
}
reexports.push(Export { ident, res, span: binding.span, vis: binding.vis });
}
}
Expand Down

0 comments on commit 9dc8dad

Please sign in to comment.