Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve: Scale back unloading of speculatively loaded crates #121167

Merged
merged 1 commit into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1070,16 +1070,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
pub fn maybe_process_path_extern(&mut self, name: Symbol) -> Option<CrateNum> {
self.maybe_resolve_crate(name, CrateDepKind::Explicit, None).ok()
}

pub fn unload_unused_crates(&mut self) {
for opt_cdata in &mut self.cstore.metas {
if let Some(cdata) = opt_cdata
&& !cdata.used()
{
*opt_cdata = None;
}
}
}
}

fn global_allocator_spans(krate: &ast::Crate) -> Vec<Span> {
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,16 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
tcx.untracked().cstore.freeze();
tcx.arena.alloc_from_iter(CStore::from_tcx(tcx).iter_crate_data().map(|(cnum, _)| cnum))
},
used_crates: |tcx, ()| {
// The list of loaded crates is now frozen in query cache,
// so make sure cstore is not mutably accessed from here on.
tcx.untracked().cstore.freeze();
tcx.arena.alloc_from_iter(
CStore::from_tcx(tcx)
.iter_crate_data()
.filter_map(|(cnum, data)| data.used().then_some(cnum)),
)
},
..providers.queries
};
provide_extern(&mut providers.extern_queries);
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,6 +1872,13 @@ rustc_queries! {
eval_always
desc { "fetching all foreign CrateNum instances" }
}
// Crates that are loaded non-speculatively (not for diagnostics or doc links).
// FIXME: This is currently only used for collecting lang items, but should be used instead of
// `crates` in most other cases too.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we describe what situations would require using crates instead or is there not a common theme?

Copy link
Contributor Author

@petrochenkov petrochenkov Feb 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metadata encoding at least, if the second item in #121167 (comment) is not fixed.
If metadata includes unused crates, then SVH and binary depinfo should probably include them either.

Any diagnostic logic, like trimmed_def_paths may include them as well, because why not.

Lang item collection, impl collection, linking certainly shouldn't include speculatively loaded crates.

query used_crates(_: ()) -> &'tcx [CrateNum] {
eval_always
desc { "fetching `CrateNum`s for all crates loaded non-speculatively" }
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we directly change the implementation of crates?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases the new version should not be used (e.g. in metadata encoding due to #121167 (comment)), but I didn't audit in which exactly and switched only one case that is necessary for #117772.


/// A list of all traits in a crate, used by rustdoc and error reporting.
query traits(_: CrateNum) -> &'tcx [DefId] {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ fn get_lang_items(tcx: TyCtxt<'_>, (): ()) -> LanguageItems {
let mut collector = LanguageItemCollector::new(tcx, resolver);

// Collect lang items in other crates.
for &cnum in tcx.crates(()).iter() {
for &cnum in tcx.used_crates(()).iter() {
for &(def_id, lang_item) in tcx.defined_lang_items(cnum).iter() {
collector.collect_item(lang_item, def_id, None);
}
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS};
use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::{PrimTy, TraitCandidate};
use rustc_metadata::creader::CStore;
use rustc_middle::middle::resolve_bound_vars::Set1;
use rustc_middle::{bug, span_bug};
use rustc_session::config::{CrateType, ResolveDocLinks};
Expand Down Expand Up @@ -4574,10 +4573,6 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
// Encoding foreign def ids in proc macro crate metadata will ICE.
return None;
}
// Doc paths should be resolved speculatively and should not produce any
// diagnostics, but if they are indeed resolved, then we need to keep the
// corresponding crate alive.
CStore::from_tcx_mut(self.r.tcx).set_used_recursively(def_id.krate);
}
res
});
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1651,7 +1651,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.tcx
.sess
.time("resolve_postprocess", || self.crate_loader(|c| c.postprocess(krate)));
self.crate_loader(|c| c.unload_unused_crates());
});

// Make sure we don't mutate the cstore from here on.
Expand Down
9 changes: 8 additions & 1 deletion tests/ui/extern-flag/empty-extern-arg.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
error: extern location for std does not exist:

error: `#[panic_handler]` function required, but not found

error: unwinding panics are not supported without std
|
= help: using nightly cargo, use -Zbuild-std with panic="abort" to avoid unwinding
= note: since the core library is usually precompiled with panic="unwind", rebuilding your crate with panic="abort" may not be enough to fix the problem

error: requires `sized` lang_item

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

Loading