Skip to content

Commit

Permalink
Rollup merge of #119592 - petrochenkov:unload, r=compiler-errors
Browse files Browse the repository at this point in the history
resolve: Unload speculatively resolved crates before freezing cstore

Name resolution sometimes loads additional crates to improve diagnostics (e.g. suggest imports).
Not all of these diagnostics result in errors, sometimes they are just warnings, like in #117772.

If additional crates loaded speculatively stay and gets listed by things like `query crates` then they may produce further errors like duplicated lang items, because lang items from speculatively loaded crates are as good as from non-speculatively loaded crates.
They can probably do things like adding unintended impls from speculatively loaded crates to method resolution as well.
The extra crates will also get into the crate's metadata as legitimate dependencies.

In this PR I remove the speculative crates from cstore when name resolution is finished and cstore is frozen.
This is better than e.g. filtering away speculative crates in `query crates` because things like `DefId`s referring to these crates and leaking to later compilation stages can produce ICEs much easier, allowing to detect them.

The unloading could potentially be skipped if any errors were reported (to allow using `DefId`s from speculatively loaded crates for recovery), but I didn't do it in this PR because I haven't seen such cases of recovery. We can reconsider later if any relevant ICEs are reported.

Unblocks #117772.
  • Loading branch information
matthiaskrgr authored Feb 8, 2024
2 parents 870a01a + a2ab48c commit 4e11d03
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 16 deletions.
15 changes: 14 additions & 1 deletion compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,10 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
) -> Option<CrateNum> {
self.used_extern_options.insert(name);
match self.maybe_resolve_crate(name, dep_kind, None) {
Ok(cnum) => Some(cnum),
Ok(cnum) => {
self.cstore.set_used_recursively(cnum);
Some(cnum)
}
Err(err) => {
let missing_core =
self.maybe_resolve_crate(sym::core, CrateDepKind::Explicit, None).is_err();
Expand Down Expand Up @@ -1067,6 +1070,16 @@ 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
7 changes: 7 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ pub(crate) struct CrateMetadata {
private_dep: bool,
/// The hash for the host proc macro. Used to support `-Z dual-proc-macro`.
host_hash: Option<Svh>,
/// The crate was used non-speculatively.
used: bool,

/// Additional data used for decoding `HygieneData` (e.g. `SyntaxContext`
/// and `ExpnId`).
Expand Down Expand Up @@ -1811,6 +1813,7 @@ impl CrateMetadata {
source: Lrc::new(source),
private_dep,
host_hash,
used: false,
extern_crate: None,
hygiene_context: Default::default(),
def_key_cache: Default::default(),
Expand Down Expand Up @@ -1860,6 +1863,10 @@ impl CrateMetadata {
self.private_dep &= private_dep;
}

pub(crate) fn used(&self) -> bool {
self.used
}

pub(crate) fn required_panic_strategy(&self) -> Option<PanicStrategy> {
self.root.required_panic_strategy
}
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use rustc_span::symbol::{kw, Symbol};
use rustc_span::Span;

use std::any::Any;
use std::mem;

use super::{Decodable, DecodeContext, DecodeIterator};

Expand Down Expand Up @@ -576,12 +577,24 @@ impl CStore {
self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess)
}

pub fn set_used_recursively(&mut self, cnum: CrateNum) {
let cmeta = self.get_crate_data_mut(cnum);
if !cmeta.used {
cmeta.used = true;
let dependencies = mem::take(&mut cmeta.dependencies);
for &dep_cnum in &dependencies {
self.set_used_recursively(dep_cnum);
}
self.get_crate_data_mut(cnum).dependencies = dependencies;
}
}

pub(crate) fn update_extern_crate(&mut self, cnum: CrateNum, extern_crate: ExternCrate) {
let cmeta = self.get_crate_data_mut(cnum);
if cmeta.update_extern_crate(extern_crate) {
// Propagate the extern crate info to dependencies if it was updated.
let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate };
let dependencies = std::mem::take(&mut cmeta.dependencies);
let dependencies = mem::take(&mut cmeta.dependencies);
for &dep_cnum in &dependencies {
self.update_extern_crate(dep_cnum, extern_crate);
}
Expand Down
21 changes: 14 additions & 7 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ 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::{BindingAnnotation, 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 @@ -4547,14 +4548,20 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
if let Some(res) = res
&& let Some(def_id) = res.opt_def_id()
&& !def_id.is_local()
&& self.r.tcx.crate_types().contains(&CrateType::ProcMacro)
&& matches!(
self.r.tcx.sess.opts.resolve_doc_links,
ResolveDocLinks::ExportedMetadata
)
{
// Encoding foreign def ids in proc macro crate metadata will ICE.
return None;
if self.r.tcx.crate_types().contains(&CrateType::ProcMacro)
&& matches!(
self.r.tcx.sess.opts.resolve_doc_links,
ResolveDocLinks::ExportedMetadata
)
{
// 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: 1 addition & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1629,6 +1629,7 @@ 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: 2 additions & 7 deletions tests/ui/extern-flag/empty-extern-arg.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
error: extern location for std does not exist:

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

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: aborting due to 3 previous errors
error: aborting due to 2 previous errors

0 comments on commit 4e11d03

Please sign in to comment.