1
Fork 0

Rollup merge of #119592 - petrochenkov:unload, r=compiler-errors

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 https://github.com/rust-lang/rust/pull/117772.
This commit is contained in:
Matthias Krüger 2024-02-08 09:06:31 +01:00 committed by GitHub
commit 4e11d03d0e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 52 additions and 16 deletions

View file

@ -534,7 +534,10 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
) -> Option<CrateNum> { ) -> Option<CrateNum> {
self.used_extern_options.insert(name); self.used_extern_options.insert(name);
match self.maybe_resolve_crate(name, dep_kind, None) { 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) => { Err(err) => {
let missing_core = let missing_core =
self.maybe_resolve_crate(sym::core, CrateDepKind::Explicit, None).is_err(); self.maybe_resolve_crate(sym::core, CrateDepKind::Explicit, None).is_err();
@ -1067,6 +1070,16 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
pub fn maybe_process_path_extern(&mut self, name: Symbol) -> Option<CrateNum> { pub fn maybe_process_path_extern(&mut self, name: Symbol) -> Option<CrateNum> {
self.maybe_resolve_crate(name, CrateDepKind::Explicit, None).ok() 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> { fn global_allocator_spans(krate: &ast::Crate) -> Vec<Span> {

View file

@ -106,6 +106,8 @@ pub(crate) struct CrateMetadata {
private_dep: bool, private_dep: bool,
/// The hash for the host proc macro. Used to support `-Z dual-proc-macro`. /// The hash for the host proc macro. Used to support `-Z dual-proc-macro`.
host_hash: Option<Svh>, host_hash: Option<Svh>,
/// The crate was used non-speculatively.
used: bool,
/// Additional data used for decoding `HygieneData` (e.g. `SyntaxContext` /// Additional data used for decoding `HygieneData` (e.g. `SyntaxContext`
/// and `ExpnId`). /// and `ExpnId`).
@ -1811,6 +1813,7 @@ impl CrateMetadata {
source: Lrc::new(source), source: Lrc::new(source),
private_dep, private_dep,
host_hash, host_hash,
used: false,
extern_crate: None, extern_crate: None,
hygiene_context: Default::default(), hygiene_context: Default::default(),
def_key_cache: Default::default(), def_key_cache: Default::default(),
@ -1860,6 +1863,10 @@ impl CrateMetadata {
self.private_dep &= private_dep; self.private_dep &= private_dep;
} }
pub(crate) fn used(&self) -> bool {
self.used
}
pub(crate) fn required_panic_strategy(&self) -> Option<PanicStrategy> { pub(crate) fn required_panic_strategy(&self) -> Option<PanicStrategy> {
self.root.required_panic_strategy self.root.required_panic_strategy
} }

View file

@ -26,6 +26,7 @@ use rustc_span::symbol::{kw, Symbol};
use rustc_span::Span; use rustc_span::Span;
use std::any::Any; use std::any::Any;
use std::mem;
use super::{Decodable, DecodeContext, DecodeIterator}; use super::{Decodable, DecodeContext, DecodeIterator};
@ -576,12 +577,24 @@ impl CStore {
self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess) 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) { pub(crate) fn update_extern_crate(&mut self, cnum: CrateNum, extern_crate: ExternCrate) {
let cmeta = self.get_crate_data_mut(cnum); let cmeta = self.get_crate_data_mut(cnum);
if cmeta.update_extern_crate(extern_crate) { if cmeta.update_extern_crate(extern_crate) {
// Propagate the extern crate info to dependencies if it was updated. // Propagate the extern crate info to dependencies if it was updated.
let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate }; 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 { for &dep_cnum in &dependencies {
self.update_extern_crate(dep_cnum, extern_crate); self.update_extern_crate(dep_cnum, extern_crate);
} }

View file

@ -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::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS};
use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::{BindingAnnotation, PrimTy, TraitCandidate}; use rustc_hir::{BindingAnnotation, PrimTy, TraitCandidate};
use rustc_metadata::creader::CStore;
use rustc_middle::middle::resolve_bound_vars::Set1; use rustc_middle::middle::resolve_bound_vars::Set1;
use rustc_middle::{bug, span_bug}; use rustc_middle::{bug, span_bug};
use rustc_session::config::{CrateType, ResolveDocLinks}; use rustc_session::config::{CrateType, ResolveDocLinks};
@ -4547,14 +4548,20 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
if let Some(res) = res if let Some(res) = res
&& let Some(def_id) = res.opt_def_id() && let Some(def_id) = res.opt_def_id()
&& !def_id.is_local() && !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. if self.r.tcx.crate_types().contains(&CrateType::ProcMacro)
return None; && 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 res
}); });

View file

@ -1629,6 +1629,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.tcx self.tcx
.sess .sess
.time("resolve_postprocess", || self.crate_loader(|c| c.postprocess(krate))); .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. // Make sure we don't mutate the cstore from here on.

View file

@ -1,11 +1,6 @@
error: extern location for std does not exist: 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 error: aborting due to 2 previous errors
|
= 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