Remove DepKind::CrateMetadata and pre-allocation of DepNodes

Remove much of the special-case handling around crate metadata
dependency tracking by replacing `DepKind::CrateMetadata` and the
pre-allocation of corresponding `DepNodes` with on-demand invocation
of the `crate_hash` query.

Author: tgnottingham

compiler/rustc_incremental/src/lib.rs

@@ -17,7 +17,6 @@ mod persist;
 pub use assert_dep_graph::assert_dep_graph;
 pub use persist::copy_cgu_workproduct_to_incr_comp_cache_dir;
 pub use persist::delete_workproduct_files;
-pub use persist::dep_graph_tcx_init;
 pub use persist::finalize_session_directory;
 pub use persist::garbage_collect_session_directories;
 pub use persist::in_incr_comp_dir;

compiler/rustc_incremental/src/persist/load.rs

@@ -4,7 +4,6 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::definitions::Definitions;
 use rustc_middle::dep_graph::{PreviousDepGraph, SerializedDepGraph, WorkProduct, WorkProductId};
 use rustc_middle::ty::query::OnDiskCache;
-use rustc_middle::ty::TyCtxt;
 use rustc_serialize::opaque::Decoder;
 use rustc_serialize::Decodable as RustcDecodable;
 use rustc_session::Session;
@@ -15,14 +14,6 @@ use super::file_format;
 use super::fs::*;
 use super::work_product;
 
-pub fn dep_graph_tcx_init(tcx: TyCtxt<'_>) {
-    if !tcx.dep_graph.is_fully_enabled() {
-        return;
-    }
-
-    tcx.allocate_metadata_dep_nodes();
-}
-
 type WorkProductMap = FxHashMap<WorkProductId, WorkProduct>;
 
 pub enum LoadResult<T> {

compiler/rustc_incremental/src/persist/mod.rs

@@ -15,7 +15,6 @@ pub use fs::garbage_collect_session_directories;
 pub use fs::in_incr_comp_dir;
 pub use fs::in_incr_comp_dir_sess;
 pub use fs::prepare_session_directory;
-pub use load::dep_graph_tcx_init;
 pub use load::load_query_result_cache;
 pub use load::LoadResult;
 pub use load::{load_dep_graph, DepGraphFuture};

compiler/rustc_interface/src/passes.rs

@@ -797,12 +797,6 @@ pub fn create_global_ctxt<'tcx>(
         })
     });
 
-    // Do some initialization of the DepGraph that can only be done with the tcx available.
-    let icx = ty::tls::ImplicitCtxt::new(&gcx);
-    ty::tls::enter_context(&icx, |_| {
-        icx.tcx.sess.time("dep_graph_tcx_init", || rustc_incremental::dep_graph_tcx_init(icx.tcx));
-    });
-
     QueryContext(gcx)
 }
 

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -10,7 +10,7 @@ use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fingerprint::{Fingerprint, FingerprintDecoder};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::svh::Svh;
-use rustc_data_structures::sync::{AtomicCell, Lock, LockGuard, Lrc, OnceCell};
+use rustc_data_structures::sync::{Lock, LockGuard, Lrc, OnceCell};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_errors::ErrorReported;
 use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
@@ -21,7 +21,6 @@ use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE}
 use rustc_hir::definitions::{DefKey, DefPath, DefPathData, DefPathHash};
 use rustc_hir::lang_items;
 use rustc_index::vec::{Idx, IndexVec};
-use rustc_middle::dep_graph::{self, DepNode, DepNodeExt, DepNodeIndex};
 use rustc_middle::hir::exports::Export;
 use rustc_middle::middle::cstore::{CrateSource, ExternCrate};
 use rustc_middle::middle::cstore::{ForeignModule, LinkagePreference, NativeLib};
@@ -84,11 +83,6 @@ crate struct CrateMetadata {
     def_path_hash_map: OnceCell<UnhashMap<DefPathHash, DefIndex>>,
     /// Used for decoding interpret::AllocIds in a cached & thread-safe manner.
     alloc_decoding_state: AllocDecodingState,
-    /// The `DepNodeIndex` of the `DepNode` representing this upstream crate.
-    /// It is initialized on the first access in `get_crate_dep_node_index()`.
-    /// Do not access the value directly, as it might not have been initialized yet.
-    /// The field must always be initialized to `DepNodeIndex::INVALID`.
-    dep_node_index: AtomicCell<DepNodeIndex>,
     /// Caches decoded `DefKey`s.
     def_key_cache: Lock<FxHashMap<DefIndex, DefKey>>,
     /// Caches decoded `DefPathHash`es.
@@ -1592,31 +1586,6 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         self.def_path_hash_unlocked(index, &mut def_path_hashes)
     }
 
-    /// Get the `DepNodeIndex` corresponding this crate. The result of this
-    /// method is cached in the `dep_node_index` field.
-    fn get_crate_dep_node_index(&self, tcx: TyCtxt<'tcx>) -> DepNodeIndex {
-        let mut dep_node_index = self.dep_node_index.load();
-
-        if unlikely!(dep_node_index == DepNodeIndex::INVALID) {
-            // We have not cached the DepNodeIndex for this upstream crate yet,
-            // so use the dep-graph to find it out and cache it.
-            // Note that multiple threads can enter this block concurrently.
-            // That is fine because the DepNodeIndex remains constant
-            // throughout the whole compilation session, and multiple stores
-            // would always write the same value.
-
-            let def_path_hash = self.def_path_hash(CRATE_DEF_INDEX);
-            let dep_node =
-                DepNode::from_def_path_hash(def_path_hash, dep_graph::DepKind::CrateMetadata);
-
-            dep_node_index = tcx.dep_graph.dep_node_index_of(&dep_node);
-            assert!(dep_node_index != DepNodeIndex::INVALID);
-            self.dep_node_index.store(dep_node_index);
-        }
-
-        dep_node_index
-    }
-
     /// Imports the source_map from an external crate into the source_map of the crate
     /// currently being compiled (the "local crate").
     ///
@@ -1833,7 +1802,6 @@ impl CrateMetadata {
             source_map_import_info: OnceCell::new(),
             def_path_hash_map: Default::default(),
             alloc_decoding_state,
-            dep_node_index: AtomicCell::new(DepNodeIndex::INVALID),
             cnum,
             cnum_map,
             dependencies,

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -44,18 +44,33 @@ macro_rules! provide {
                 let ($def_id, $other) = def_id_arg.into_args();
                 assert!(!$def_id.is_local());
 
-                let $cdata = CStore::from_tcx($tcx).get_crate_data($def_id.krate);
-
                 if $tcx.dep_graph.is_fully_enabled() {
-                    let crate_dep_node_index = $cdata.get_crate_dep_node_index($tcx);
-                    $tcx.dep_graph.read_index(crate_dep_node_index);
+                    $tcx.ensure().crate_hash($def_id.krate);
                 }
 
+                let $cdata = CStore::from_tcx($tcx).get_crate_data($def_id.krate);
+
                 $compute
             })*
 
+            // The other external query providers call `crate_hash` in order to register a
+            // dependency on the crate metadata. The `crate_hash` implementation differs in
+            // that it doesn't need to do this (and can't, as it would cause a query cycle).
+            fn crate_hash<'tcx>(
+                tcx: TyCtxt<'tcx>,
+                def_id_arg: ty::query::query_keys::crate_hash<'tcx>,
+            ) -> ty::query::query_values::crate_hash<'tcx> {
+                let _prof_timer = tcx.prof.generic_activity("metadata_decode_entry_crate_hash");
+
+                let (def_id, _) = def_id_arg.into_args();
+                assert!(!def_id.is_local());
+
+                CStore::from_tcx(tcx).get_crate_data(def_id.krate).root.hash
+            }
+
             *providers = Providers {
                 $($name,)*
+                crate_hash,
                 ..*providers
             };
         }
@@ -191,7 +206,6 @@ provide! { <'tcx> tcx, def_id, other, cdata,
         })
     }
     crate_disambiguator => { cdata.root.disambiguator }
-    crate_hash => { cdata.root.hash }
     crate_host_hash => { cdata.host_hash }
     original_crate_name => { cdata.root.name }
 

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -214,17 +214,6 @@ pub mod dep_kind {
         try_load_from_on_disk_cache: |_, _| {},
     };
 
-    // Represents metadata from an extern crate.
-    pub const CrateMetadata: DepKindStruct = DepKindStruct {
-        has_params: true,
-        is_anon: false,
-        is_eval_always: true,
-
-        can_reconstruct_query_key: || true,
-        force_from_dep_node: |_, dep_node| bug!("force_from_dep_node: encountered {:?}", dep_node),
-        try_load_from_on_disk_cache: |_, _| {},
-    };
-
     pub const TraitSelect: DepKindStruct = DepKindStruct {
         has_params: false,
         is_anon: true,
@@ -379,9 +368,6 @@ rustc_dep_node_append!([define_dep_nodes!][ <'tcx>
     // We use this for most things when incr. comp. is turned off.
     [] Null,
 
-    // Represents metadata from an extern crate.
-    [eval_always] CrateMetadata(CrateNum),
-
     [anon] TraitSelect,
 
     [] CompileCodegenUnit(Symbol),

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -115,20 +115,9 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {
         // be removed. https://github.com/rust-lang/rust/issues/62649 is one such
         // bug that must be fixed before removing this.
         match dep_node.kind {
-            DepKind::hir_owner | DepKind::hir_owner_nodes | DepKind::CrateMetadata => {
+            DepKind::hir_owner | DepKind::hir_owner_nodes => {
                 if let Some(def_id) = dep_node.extract_def_id(*self) {
-                    if def_id_corresponds_to_hir_dep_node(*self, def_id.expect_local()) {
-                        if dep_node.kind == DepKind::CrateMetadata {
-                            // The `DefPath` has corresponding node,
-                            // and that node should have been marked
-                            // either red or green in `data.colors`.
-                            bug!(
-                                "DepNode {:?} should have been \
-                             pre-marked as red or green but wasn't.",
-                                dep_node
-                            );
-                        }
-                    } else {
+                    if !def_id_corresponds_to_hir_dep_node(*self, def_id.expect_local()) {
                         // This `DefPath` does not have a
                         // corresponding `DepNode` (e.g. a
                         // struct field), and the ` DefPath`

compiler/rustc_middle/src/ty/context.rs

@@ -1,7 +1,7 @@
 //! Type context book-keeping.
 
 use crate::arena::Arena;
-use crate::dep_graph::{self, DepGraph, DepKind, DepNode, DepNodeExt};
+use crate::dep_graph::DepGraph;
 use crate::hir::exports::ExportMap;
 use crate::ich::{NodeIdHashingMode, StableHashingContext};
 use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};
@@ -37,8 +37,7 @@ use rustc_data_structures::sync::{self, Lock, Lrc, WorkerLocal};
 use rustc_errors::ErrorReported;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId};
-use rustc_hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE};
+use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, LOCAL_CRATE};
 use rustc_hir::definitions::Definitions;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::lang_items::LangItem;
@@ -1315,28 +1314,6 @@ impl<'tcx> TyCtxt<'tcx> {
         StableHashingContext::ignore_spans(self.sess, krate, self.definitions, &*self.cstore)
     }
 
-    // This method makes sure that we have a DepNode and a Fingerprint for
-    // every upstream crate. It needs to be called once right after the tcx is
-    // created.
-    // With full-fledged red/green, the method will probably become unnecessary
-    // as this will be done on-demand.
-    pub fn allocate_metadata_dep_nodes(self) {
-        // We cannot use the query versions of crates() and crate_hash(), since
-        // those would need the DepNodes that we are allocating here.
-        for cnum in self.cstore.crates_untracked() {
-            let def_path_hash = self.def_path_hash(DefId { krate: cnum, index: CRATE_DEF_INDEX });
-            let dep_node = DepNode::from_def_path_hash(def_path_hash, DepKind::CrateMetadata);
-            let crate_hash = self.cstore.crate_hash_untracked(cnum);
-            self.dep_graph.with_task(
-                dep_node,
-                self,
-                crate_hash,
-                |_, x| x, // No transformation needed
-                dep_graph::hash_result,
-            );
-        }
-    }
-
     pub fn serialize_query_result_cache(self, encoder: &mut FileEncoder) -> FileEncodeResult {
         self.queries.on_disk_cache.as_ref().map(|c| c.serialize(self, encoder)).unwrap_or(Ok(()))
     }