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

rustc: Use tcx.used_crates(()) more #124976

Merged
merged 1 commit into from
May 23, 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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ fn upstream_monomorphizations_provider(
tcx: TyCtxt<'_>,
(): (),
) -> DefIdMap<UnordMap<GenericArgsRef<'_>, CrateNum>> {
let cnums = tcx.crates(());
let cnums = tcx.used_crates(());

let mut instances: DefIdMap<UnordMap<_, _>> = Default::default();

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ pub fn collect_debugger_visualizers_transitive(
tcx.debugger_visualizers(LOCAL_CRATE)
.iter()
.chain(
tcx.crates(())
tcx.used_crates(())
.iter()
.filter(|&cnum| {
let used_crate_source = tcx.used_crate_source(*cnum);
Expand Down Expand Up @@ -849,7 +849,7 @@ impl CrateInfo {
// `compiler_builtins` are always placed last to ensure that they're linked correctly.
used_crates.extend(compiler_builtins);

let crates = tcx.crates(());
let crates = tcx.used_crates(());
let n_crates = crates.len();
let mut info = CrateInfo {
target_cpu,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ fn write_out_deps(tcx: TyCtxt<'_>, outputs: &OutputFilenames, out_filenames: &[P
}
}

for &cnum in tcx.crates(()) {
for &cnum in tcx.crates_including_speculative(()) {
let source = tcx.used_crate_source(cnum);
if let Some((path, _)) = &source.dylib {
files.push(escape_dep_filename(&path.display().to_string()));
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_metadata/src/dependency_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList {
&& sess.crt_static(Some(ty))
&& !sess.target.crt_static_allows_dylibs)
{
for &cnum in tcx.crates(()).iter() {
for &cnum in tcx.used_crates(()).iter() {
if tcx.dep_kind(cnum).macros_only() {
continue;
}
Expand All @@ -164,7 +164,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList {
// Sweep all crates for found dylibs. Add all dylibs, as well as their
// 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 tcx.crates(()).iter() {
for &cnum in tcx.used_crates(()).iter() {
if tcx.dep_kind(cnum).macros_only() {
continue;
}
Expand All @@ -182,7 +182,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList {
}

// Collect what we've got so far in the return vector.
let last_crate = tcx.crates(()).len();
let last_crate = tcx.used_crates(()).len();
let mut ret = (1..last_crate + 1)
.map(|cnum| match formats.get(&CrateNum::new(cnum)) {
Some(&RequireDynamic) => Linkage::Dynamic,
Expand All @@ -196,7 +196,7 @@ fn calculate_type(tcx: TyCtxt<'_>, ty: CrateType) -> DependencyList {
//
// If the crate hasn't been included yet and it's not actually required
// (e.g., it's an allocator) then we skip it here as well.
for &cnum in tcx.crates(()).iter() {
for &cnum in tcx.used_crates(()).iter() {
let src = tcx.used_crate_source(cnum);
if src.dylib.is_none()
&& !formats.contains_key(&cnum)
Expand Down Expand Up @@ -284,7 +284,7 @@ fn add_library(

fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec<CrateNum>) -> Option<DependencyList> {
let all_crates_available_as_rlib = tcx
.crates(())
.used_crates(())
.iter()
.copied()
.filter_map(|cnum| {
Expand All @@ -305,7 +305,7 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec<CrateNum>) -> Option<De
// All crates are available in an rlib format, so we're just going to link
// everything in explicitly so long as it's actually required.
let mut ret = tcx
.crates(())
.used_crates(())
.iter()
.map(|&cnum| match tcx.dep_kind(cnum) {
CrateDepKind::Explicit => Linkage::Static,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
// traversal, but not globally minimal across all crates.
let bfs_queue = &mut VecDeque::new();

for &cnum in tcx.crates(()) {
for &cnum in tcx.crates_including_speculative(()) {
// Ignore crates without a corresponding local `extern crate` item.
if tcx.missing_extern_crate_item(cnum) {
continue;
Expand Down Expand Up @@ -505,7 +505,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
tcx.arena
.alloc_slice(&CStore::from_tcx(tcx).crate_dependencies_in_postorder(LOCAL_CRATE))
},
crates: |tcx, ()| {
crates_including_speculative: |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();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1898,7 +1898,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {

let deps = self
.tcx
.crates(())
.crates_including_speculative(())
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
.iter()
.map(|&cnum| {
let dep = CrateDep {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
let krate = tcx.hir_crate(());
let hir_body_hash = krate.opt_hir_hash.expect("HIR hash missing while computing crate hash");

let upstream_crates = upstream_crates(tcx);
let upstream_crates = upstream_crates_for_hashing(tcx);

let resolutions = tcx.resolutions(());

Expand Down Expand Up @@ -1085,9 +1085,9 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, _: LocalCrate) -> Svh {
Svh::new(crate_hash)
}

fn upstream_crates(tcx: TyCtxt<'_>) -> Vec<(StableCrateId, Svh)> {
fn upstream_crates_for_hashing(tcx: TyCtxt<'_>) -> Vec<(StableCrateId, Svh)> {
let mut upstream_crates: Vec<_> = tcx
.crates(())
.crates_including_speculative(())
.iter()
.map(|&cnum| {
let stable_crate_id = tcx.stable_crate_id(cnum);
Expand Down
17 changes: 13 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1860,13 +1860,22 @@ rustc_queries! {
eval_always
desc { "calculating the stability index for the local crate" }
}
query crates(_: ()) -> &'tcx [CrateNum] {
/// All loaded crates, including those loaded purely for doc links or diagnostics.
/// (Diagnostics include lints, so speculatively loaded crates may occur in successful
/// compilation even without doc links.)
/// Should be used when encoding crate metadata (and therefore when generating crate hash,
/// depinfo and similar things), to avoid dangling crate references in other encoded data,
/// like source maps.
/// May also be used for diagnostics - if we are loading a crate anyway we can suggest some
/// items from it as well.
/// But otherwise, `used_crates` should generally be used.
query crates_including_speculative(_: ()) -> &'tcx [CrateNum] {
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.
/// Crates that are loaded non-speculatively (not for diagnostics or doc links).
/// Should be used to maintain observable language behavior, for example when collecting lang
/// items or impls from all crates, or collecting libraries to link.
query used_crates(_: ()) -> &'tcx [CrateNum] {
Copy link
Member

Choose a reason for hiding this comment

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

"used" kind of sounds like if I add a crate as a dependency but don't use it, it doesn't show up.

Maybe required_crates?

Also all_crates could maybe need a more scary name, like crates_including_speculative or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

crates_including_speculative looks fine to me.
Not sure about required_crates.

Ideally I'd turn the used bool in crate loader into a 3-variant enum:

enum WhatCrateIsLoadedFor {
  ForRealThings,
  // Need to be encoded into metadata in any case
  ForDocLinksAndDiagnosticsOnly,
  // It may be possible to drop these ones from metadata, if we clean their traces from source map somehow
  ForDiagnosticsOnly, 
}

Maybe even merge it with existing enum CrateDepKind which is sort of similar.

Let's keep its pre-existing naming for now.

eval_always
desc { "fetching `CrateNum`s for all crates loaded non-speculatively" }
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,7 @@ impl<'tcx> TyCtxt<'tcx> {

pub fn all_traits(self) -> impl Iterator<Item = DefId> + 'tcx {
iter::once(LOCAL_CRATE)
.chain(self.crates(()).iter().copied())
.chain(self.used_crates(()).iter().copied())
.flat_map(move |cnum| self.traits(cnum).iter().copied())
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3258,7 +3258,7 @@ fn for_each_def(tcx: TyCtxt<'_>, mut collect_fn: impl for<'b> FnMut(&'b Ident, N
let queue = &mut Vec::new();
let mut seen_defs: DefIdSet = Default::default();

for &cnum in tcx.crates(()).iter() {
for &cnum in tcx.crates_including_speculative(()).iter() {
let def_id = cnum.as_def_id();

// Ignore crates that are not direct dependencies.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait
// Traits defined in the current crate can't have impls in upstream
// crates, so we don't bother querying the cstore.
if !trait_id.is_local() {
for &cnum in tcx.crates(()).iter() {
for &cnum in tcx.used_crates(()).iter() {
for &(impl_def_id, simplified_self_ty) in
tcx.implementations_of_trait((cnum, trait_id)).iter()
{
Expand Down Expand Up @@ -247,7 +247,7 @@ pub(super) fn incoherent_impls_provider(
let mut impls = Vec::new();

let mut res = Ok(());
for cnum in iter::once(LOCAL_CRATE).chain(tcx.crates(()).iter().copied()) {
for cnum in iter::once(LOCAL_CRATE).chain(tcx.used_crates(()).iter().copied()) {
let incoherent_impls = match tcx.crate_incoherent_impls((cnum, simp)) {
Ok(impls) => impls,
Err(e) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/diagnostic_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn all_diagnostic_items(tcx: TyCtxt<'_>, (): ()) -> DiagnosticItems {
let mut items = DiagnosticItems::default();

// Collect diagnostic items in other crates.
for &cnum in tcx.crates(()).iter().chain(std::iter::once(&LOCAL_CRATE)) {
for &cnum in tcx.crates_including_speculative(()).iter().chain(std::iter::once(&LOCAL_CRATE)) {
for (&name, &def_id) in &tcx.diagnostic_items(cnum).name_to_id {
collect_item(tcx, &mut items, name, def_id);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
// stabilization diagnostic, but it can be avoided when there are no
// `remaining_lib_features`.
let mut all_implications = remaining_implications.clone();
for &cnum in tcx.crates(()) {
for &cnum in tcx.used_crates(()) {
all_implications
.extend_unord(tcx.stability_implications(cnum).items().map(|(k, v)| (*k, *v)));
}
Expand All @@ -1033,7 +1033,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
&all_implications,
);

for &cnum in tcx.crates(()) {
for &cnum in tcx.used_crates(()) {
if remaining_lib_features.is_empty() && remaining_implications.is_empty() {
break;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/weak_lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ fn verify(tcx: TyCtxt<'_>, items: &lang_items::LanguageItems) {
}

let mut missing = FxHashSet::default();
for &cnum in tcx.crates(()).iter() {
for &cnum in tcx.used_crates(()).iter() {
for &item in tcx.missing_lang_items(cnum).iter() {
missing.insert(item);
}
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_smir/src/rustc_smir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<'tcx> Context for TablesWrapper<'tcx> {
let mut tables = self.0.borrow_mut();
let tcx = tables.tcx;
iter::once(LOCAL_CRATE)
.chain(tables.tcx.crates(()).iter().copied())
.chain(tables.tcx.used_crates(()).iter().copied())
.flat_map(|cnum| tcx.trait_impls_in_crate(cnum).iter())
.map(|impl_def_id| tables.impl_def(*impl_def_id))
.collect()
Expand Down Expand Up @@ -201,14 +201,19 @@ impl<'tcx> Context for TablesWrapper<'tcx> {

fn external_crates(&self) -> Vec<stable_mir::Crate> {
let tables = self.0.borrow();
tables.tcx.crates(()).iter().map(|crate_num| smir_crate(tables.tcx, *crate_num)).collect()
tables
.tcx
.used_crates(())
.iter()
.map(|crate_num| smir_crate(tables.tcx, *crate_num))
.collect()
}

fn find_crates(&self, name: &str) -> Vec<stable_mir::Crate> {
let tables = self.0.borrow();
let crates: Vec<stable_mir::Crate> = [LOCAL_CRATE]
.iter()
.chain(tables.tcx.crates(()).iter())
.chain(tables.tcx.used_crates(()).iter())
.filter_map(|crate_num| {
let crate_name = tables.tcx.crate_name(*crate_num).to_string();
(name == crate_name).then(|| smir_crate(tables.tcx, *crate_num))
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,7 @@ impl PrimitiveType {
let mut primitive_locations = FxHashMap::default();
// NOTE: technically this misses crates that are only passed with `--extern` and not loaded when checking the crate.
// This is a degenerate case that I don't plan to support.
for &crate_num in tcx.crates(()) {
for &crate_num in tcx.crates_including_speculative(()) {
let e = ExternalCrate { crate_num };
let crate_name = e.name(tcx);
debug!(?crate_num, ?crate_name);
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ pub(crate) fn run_global_ctxt(
show_coverage,
};

for cnum in tcx.crates(()) {
for cnum in tcx.crates_including_speculative(()) {
crate::visit_lib::lib_embargo_visit_item(&mut ctxt, cnum.as_def_id());
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl Cache {

// Cache where all our extern crates are located
// FIXME: this part is specific to HTML so it'd be nice to remove it from the common code
for &crate_num in tcx.crates(()) {
for &crate_num in tcx.crates_including_speculative(()) {
let e = ExternalCrate { crate_num };

let name = e.name(tcx);
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
// External trait impls.
{
let _prof_timer = tcx.sess.prof.generic_activity("build_extern_trait_impls");
for &cnum in tcx.crates(()) {
for &cnum in tcx.crates_including_speculative(()) {
for &impl_def_id in tcx.trait_impls_in_crate(cnum) {
cx.with_param_env(impl_def_id, |cx| {
inline::build_impl(cx, impl_def_id, None, &mut new_items_external);
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/scrape_examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub(crate) fn run(
// Collect CrateIds corresponding to provided target crates
// If two different versions of the crate in the dependency tree, then examples will be collected from both.
let all_crates = tcx
.crates(())
.crates_including_speculative(())
.iter()
.chain([&LOCAL_CRATE])
.map(|crate_num| (crate_num, tcx.crate_name(*crate_num)))
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ fn item_children_by_name(tcx: TyCtxt<'_>, def_id: DefId, name: Symbol) -> Vec<Re
/// This function is expensive and should be used sparingly.
pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Vec<Res> {
fn find_crates(tcx: TyCtxt<'_>, name: Symbol) -> impl Iterator<Item = DefId> + '_ {
tcx.crates(())
tcx.crates_including_speculative(())
.iter()
.copied()
.filter(move |&num| tcx.crate_name(num) == name)
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ fn try_resolve_did(tcx: TyCtxt<'_>, path: &[&str], namespace: Option<Namespace>)
// the one in the sysroot and the one locally built by `cargo test`.)
// FIXME: can we prefer the one from the sysroot?
'crates: for krate in
tcx.crates(()).iter().filter(|&&krate| tcx.crate_name(krate).as_str() == crate_name)
tcx.used_crates(()).iter().filter(|&&krate| tcx.crate_name(krate).as_str() == crate_name)
{
let mut cur_item = DefId { krate: *krate, index: CRATE_DEF_INDEX };
// Go over the modules.
Expand Down Expand Up @@ -1365,7 +1365,7 @@ pub fn get_local_crates(tcx: TyCtxt<'_>) -> Vec<CrateNum> {
.map(|crates| crates.split(',').map(|krate| krate.to_string()).collect::<Vec<_>>())
.unwrap_or_default();
let mut local_crates = Vec::new();
for &crate_num in tcx.crates(()) {
for &crate_num in tcx.crates_including_speculative(()) {
let name = tcx.crate_name(crate_num);
let name = name.as_str();
if local_crate_names.iter().any(|local_name| local_name == name) {
Expand Down
9 changes: 1 addition & 8 deletions tests/ui/extern-flag/empty-extern-arg.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
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 4 previous errors
error: aborting due to 2 previous errors
Copy link
Member

@lqd lqd May 24, 2024

Choose a reason for hiding this comment

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

Was this diagnostics change expected? (I'm analyzing the regressions caused by this PR)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, during my review I assumed this is the weak lang item check that now only looks at crates that are actually used, so we aren't speculatively looking at weak lang items of unused crates anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I see libcore is loaded speculatively in this test (missing_core in fn resolve_crate) and this error happens due to (weak) lang items in core.


Loading