Auto merge of rust-lang#85941 - cjgillot:qresolve, r=Aaron1011

Reduce the amount of untracked state in TyCtxt -- Take 2

Main part of rust-lang#85153

The offending line (rust-lang#85153 (comment)) is replaced by a FIXME until the possible bug and the perf concern are both resolved.

r? `@Aaron1011`

compiler/rustc_hir/src/definitions.rs

@@ -23,7 +23,7 @@ use tracing::debug;
 /// Internally the `DefPathTable` holds a tree of `DefKey`s, where each `DefKey`
 /// stores the `DefIndex` of its parent.
 /// There is one `DefPathTable` for each crate.
-#[derive(Clone, Default)]
+#[derive(Clone, Default, Debug)]
 pub struct DefPathTable {
     index_to_key: IndexVec<DefIndex, DefKey>,
     def_path_hashes: IndexVec<DefIndex, DefPathHash>,
@@ -96,7 +96,7 @@ impl DefPathTable {
 /// The definition table containing node definitions.
 /// It holds the `DefPathTable` for `LocalDefId`s/`DefPath`s.
 /// It also stores mappings to convert `LocalDefId`s to/from `HirId`s.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct Definitions {
     table: DefPathTable,
 

compiler/rustc_metadata/src/creader.rs

@@ -51,6 +51,12 @@ pub struct CStore {
     unused_externs: Vec<Symbol>,
 }
 
+impl std::fmt::Debug for CStore {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("CStore").finish_non_exhaustive()
+    }
+}
+
 pub struct CrateLoader<'a> {
     // Immutable configuration.
     sess: &'a Session,

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -445,7 +445,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
     }
 
     fn encode_def_path_table(&mut self) {
-        let table = self.tcx.hir().definitions().def_path_table();
+        let table = self.tcx.resolutions(()).definitions.def_path_table();
         if self.is_proc_macro {
             for def_index in std::iter::once(CRATE_DEF_INDEX)
                 .chain(self.tcx.hir().krate().proc_macros.iter().map(|p| p.owner.local_def_index))
@@ -1062,7 +1062,7 @@ impl EncodeContext<'a, 'tcx> {
 
         let data = ModData {
             reexports,
-            expansion: tcx.hir().definitions().expansion_that_defined(local_def_id),
+            expansion: tcx.resolutions(()).definitions.expansion_that_defined(local_def_id),
         };
 
         record!(self.tables.kind[def_id] <- EntryKind::Mod(self.lazy(data)));
@@ -1759,7 +1759,7 @@ impl EncodeContext<'a, 'tcx> {
             .map(|(trait_def_id, mut impls)| {
                 // Bring everything into deterministic order for hashing
                 impls.sort_by_cached_key(|&(index, _)| {
-                    tcx.hir().definitions().def_path_hash(LocalDefId { local_def_index: index })
+                    tcx.hir().def_path_hash(LocalDefId { local_def_index: index })
                 });
 
                 TraitImpls {

compiler/rustc_middle/src/hir/map/mod.rs

@@ -9,7 +9,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::svh::Svh;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
-use rustc_hir::definitions::{DefKey, DefPath, Definitions};
+use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
 use rustc_hir::intravisit;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::itemlikevisit::ItemLikeVisitor;
@@ -154,21 +154,24 @@ impl<'hir> Map<'hir> {
         self.tcx.hir_crate(())
     }
 
-    #[inline]
-    pub fn definitions(&self) -> &'hir Definitions {
-        &self.tcx.definitions
-    }
-
     pub fn def_key(&self, def_id: LocalDefId) -> DefKey {
-        self.tcx.definitions.def_key(def_id)
+        // Accessing the DefKey is ok, since it is part of DefPathHash.
+        self.tcx.untracked_resolutions.definitions.def_key(def_id)
     }
 
     pub fn def_path_from_hir_id(&self, id: HirId) -> Option<DefPath> {
         self.opt_local_def_id(id).map(|def_id| self.def_path(def_id))
     }
 
     pub fn def_path(&self, def_id: LocalDefId) -> DefPath {
-        self.tcx.definitions.def_path(def_id)
+        // Accessing the DefPath is ok, since it is part of DefPathHash.
+        self.tcx.untracked_resolutions.definitions.def_path(def_id)
+    }
+
+    #[inline]
+    pub fn def_path_hash(self, def_id: LocalDefId) -> DefPathHash {
+        // Accessing the DefPathHash is ok, it is incr. comp. stable.
+        self.tcx.untracked_resolutions.definitions.def_path_hash(def_id)
     }
 
     #[inline]
@@ -184,16 +187,21 @@ impl<'hir> Map<'hir> {
 
     #[inline]
     pub fn opt_local_def_id(&self, hir_id: HirId) -> Option<LocalDefId> {
-        self.tcx.definitions.opt_hir_id_to_local_def_id(hir_id)
+        // FIXME(#85914) is this access safe for incr. comp.?
+        self.tcx.untracked_resolutions.definitions.opt_hir_id_to_local_def_id(hir_id)
     }
 
     #[inline]
     pub fn local_def_id_to_hir_id(&self, def_id: LocalDefId) -> HirId {
-        self.tcx.definitions.local_def_id_to_hir_id(def_id)
+        // FIXME(#85914) is this access safe for incr. comp.?
+        self.tcx.untracked_resolutions.definitions.local_def_id_to_hir_id(def_id)
     }
 
     pub fn iter_local_def_id(&self) -> impl Iterator<Item = LocalDefId> + '_ {
-        self.tcx.definitions.iter_local_def_id()
+        // Create a dependency to the crate to be sure we reexcute this when the amount of
+        // definitions change.
+        self.tcx.ensure().hir_crate(());
+        self.tcx.untracked_resolutions.definitions.iter_local_def_id()
     }
 
     pub fn opt_def_kind(&self, local_def_id: LocalDefId) -> Option<DefKind> {
@@ -932,9 +940,15 @@ impl<'hir> intravisit::Map<'hir> for Map<'hir> {
 pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> &'tcx IndexedHir<'tcx> {
     let _prof_timer = tcx.sess.prof.generic_activity("build_hir_map");
 
+    // We can access untracked state since we are an eval_always query.
     let hcx = tcx.create_stable_hashing_context();
-    let mut collector =
-        NodeCollector::root(tcx.sess, &**tcx.arena, tcx.untracked_crate, &tcx.definitions, hcx);
+    let mut collector = NodeCollector::root(
+        tcx.sess,
+        &**tcx.arena,
+        tcx.untracked_crate,
+        &tcx.untracked_resolutions.definitions,
+        hcx,
+    );
     intravisit::walk_crate(&mut collector, tcx.untracked_crate);
 
     let map = collector.finalize_and_compute_crate_hash();
@@ -944,14 +958,15 @@ pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> &'tcx IndexedHir<'tc
 pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
     assert_eq!(crate_num, LOCAL_CRATE);
 
+    // We can access untracked state since we are an eval_always query.
     let mut hcx = tcx.create_stable_hashing_context();
 
     let mut hir_body_nodes: Vec<_> = tcx
         .index_hir(())
         .map
         .iter_enumerated()
         .filter_map(|(def_id, hod)| {
-            let def_path_hash = tcx.definitions.def_path_hash(def_id);
+            let def_path_hash = tcx.untracked_resolutions.definitions.def_path_hash(def_id);
             let mut hasher = StableHasher::new();
             hod.as_ref()?.hash_stable(&mut hcx, &mut hasher);
             AttributeMap { map: &tcx.untracked_crate.attrs, prefix: def_id }
@@ -968,7 +983,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
         },
     );
 
-    let upstream_crates = upstream_crates(&*tcx.cstore);
+    let upstream_crates = upstream_crates(&*tcx.untracked_resolutions.cstore);
 
     // We hash the final, remapped names of all local source files so we
     // don't have to include the path prefix remapping commandline args.

compiler/rustc_middle/src/hir/mod.rs

@@ -169,6 +169,6 @@ pub fn provide(providers: &mut Providers) {
     providers.all_local_trait_impls = |tcx, ()| &tcx.hir_crate(()).trait_impls;
     providers.expn_that_defined = |tcx, id| {
         let id = id.expect_local();
-        tcx.definitions.expansion_that_defined(id)
+        tcx.resolutions(()).definitions.expansion_that_defined(id)
     };
 }

compiler/rustc_middle/src/middle/cstore.rs

@@ -187,7 +187,7 @@ pub type MetadataLoaderDyn = dyn MetadataLoader + Sync;
 /// that it's *not* tracked for dependency information throughout compilation
 /// (it'd break incremental compilation) and should only be called pre-HIR (e.g.
 /// during resolve)
-pub trait CrateStore {
+pub trait CrateStore: std::fmt::Debug {
     fn as_any(&self) -> &dyn Any;
 
     // resolve

compiler/rustc_middle/src/query/mod.rs

@@ -14,6 +14,12 @@ rustc_queries! {
         desc { "trigger a delay span bug" }
     }
 
+    query resolutions(_: ()) -> &'tcx ty::ResolverOutputs {
+        eval_always
+        no_hash
+        desc { "get the resolver outputs" }
+    }
+
     /// Represents crate as a whole (as distinct from the top-level crate module).
     /// If you call `hir_crate` (e.g., indirectly by calling `tcx.hir().krate()`),
     /// we will have to assume that any change means that you need to be recompiled.
@@ -207,7 +213,6 @@ rustc_queries! {
     }
 
     query expn_that_defined(key: DefId) -> rustc_span::ExpnId {
-        eval_always
         desc { |tcx| "expansion that defined `{}`", tcx.def_path_str(key) }
     }
 
@@ -1133,7 +1138,6 @@ rustc_queries! {
 
     query module_exports(def_id: LocalDefId) -> Option<&'tcx [Export<LocalDefId>]> {
         desc { |tcx| "looking up items exported by `{}`", tcx.def_path_str(def_id.to_def_id()) }
-        eval_always
     }
 
     query impl_defaultness(def_id: DefId) -> hir::Defaultness {
@@ -1323,7 +1327,6 @@ rustc_queries! {
     }
 
     query visibility(def_id: DefId) -> ty::Visibility {
-        eval_always
         desc { |tcx| "computing visibility of `{}`", tcx.def_path_str(def_id) }
     }
 
@@ -1348,8 +1351,6 @@ rustc_queries! {
         desc { |tcx| "collecting child items of `{}`", tcx.def_path_str(def_id) }
     }
     query extern_mod_stmt_cnum(def_id: LocalDefId) -> Option<CrateNum> {
-        // This depends on untracked global state (`tcx.extern_crate_map`)
-        eval_always
         desc { |tcx| "computing crate imported by `{}`", tcx.def_path_str(def_id.to_def_id()) }
     }
 
@@ -1426,16 +1427,12 @@ rustc_queries! {
         eval_always
     }
     query maybe_unused_trait_import(def_id: LocalDefId) -> bool {
-        eval_always
         desc { |tcx| "maybe_unused_trait_import for `{}`", tcx.def_path_str(def_id.to_def_id()) }
     }
     query maybe_unused_extern_crates(_: ()) -> &'tcx [(LocalDefId, Span)] {
-        eval_always
         desc { "looking up all possibly unused extern crates" }
     }
-    query names_imported_by_glob_use(def_id: LocalDefId)
-        -> &'tcx FxHashSet<Symbol> {
-        eval_always
+    query names_imported_by_glob_use(def_id: LocalDefId) -> &'tcx FxHashSet<Symbol> {
         desc { |tcx| "names_imported_by_glob_use for `{}`", tcx.def_path_str(def_id.to_def_id()) }
     }
 
@@ -1445,7 +1442,6 @@ rustc_queries! {
         desc { "calculating the stability index for the local crate" }
     }
     query crates(_: ()) -> &'tcx [CrateNum] {
-        eval_always
         desc { "fetching all foreign CrateNum instances" }
     }