Auto merge of #85266 - cjgillot:hir-dep-clean, r=michaelwoerister

Remove obsolete workaround.

The regression test for #62649 appears to pass even without the workaround.

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

@@ -1,6 +1,6 @@
 use crate::arena::Arena;
-use crate::hir::map::{HirOwnerData, Map};
+use crate::hir::map::Map;
-use crate::hir::{IndexedHir, Owner, OwnerNodes, ParentedNode};
+use crate::hir::{IndexedHir, OwnerNodes, ParentedNode};
 use crate::ich::StableHashingContext;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
@@ -28,7 +28,7 @@ pub(super) struct NodeCollector<'a, 'hir> {
     /// Source map
     source_map: &'a SourceMap,
 
-    map: IndexVec<LocalDefId, HirOwnerData<'hir>>,
+    map: IndexVec<LocalDefId, Option<&'hir mut OwnerNodes<'hir>>>,
     parenting: FxHashMap<LocalDefId, HirId>,
 
     /// The parent of this node
@@ -107,9 +107,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
             current_dep_node_owner: LocalDefId { local_def_index: CRATE_DEF_INDEX },
             definitions,
             hcx,
-            map: (0..definitions.def_index_count())
+            map: IndexVec::from_fn_n(|_| None, definitions.def_index_count()),
-                .map(|_| HirOwnerData { signature: None, with_bodies: None })
-                .collect(),
             parenting: FxHashMap::default(),
         };
         collector.insert_entry(
@@ -124,7 +122,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
     pub(super) fn finalize_and_compute_crate_hash(mut self) -> IndexedHir<'hir> {
         // Insert bodies into the map
         for (id, body) in self.krate.bodies.iter() {
-            let bodies = &mut self.map[id.hir_id.owner].with_bodies.as_mut().unwrap().bodies;
+            let bodies = &mut self.map[id.hir_id.owner].as_mut().unwrap().bodies;
             assert!(bodies.insert(id.hir_id.local_id, body).is_none());
         }
         IndexedHir { map: self.map, parenting: self.parenting }
@@ -137,22 +135,13 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
 
         let data = &mut self.map[id.owner];
 
-        if data.with_bodies.is_none() {
+        if i == 0 {
-            data.with_bodies = Some(arena.alloc(OwnerNodes {
+            debug_assert!(data.is_none());
+            *data = Some(arena.alloc(OwnerNodes {
                 hash,
                 nodes: IndexVec::new(),
                 bodies: FxHashMap::default(),
             }));
-        }
-
-        let nodes = data.with_bodies.as_mut().unwrap();
-
-        if i == 0 {
-            // Overwrite the dummy hash with the real HIR owner hash.
-            nodes.hash = hash;
-
-            debug_assert!(data.signature.is_none());
-            data.signature = Some(self.arena.alloc(Owner { node: entry.node }));
 
             let dk_parent = self.definitions.def_key(id.owner).parent;
             if let Some(dk_parent) = dk_parent {
@@ -168,14 +157,17 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
                 debug_assert_eq!(self.parenting.get(&id.owner), Some(&entry.parent));
             }
         } else {
-            assert_eq!(entry.parent.owner, id.owner);
+            debug_assert_eq!(entry.parent.owner, id.owner);
+        }
+
+        let data = data.as_mut().unwrap();
+
         insert_vec_map(
-                &mut nodes.nodes,
+            &mut data.nodes,
             id.local_id,
             ParentedNode { parent: entry.parent.local_id, node: entry.node },
         );
     }
-    }
 
     fn insert(&mut self, span: Span, hir_id: HirId, node: Node<'hir>) {
         self.insert_with_hash(span, hir_id, node, Fingerprint::ZERO)

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

@@ -1,6 +1,6 @@
 use self::collector::NodeCollector;
 
-use crate::hir::{AttributeMap, HirOwnerData, IndexedHir};
+use crate::hir::{AttributeMap, IndexedHir};
 use crate::middle::cstore::CrateStore;
 use crate::ty::TyCtxt;
 use rustc_ast as ast;
@@ -953,7 +953,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
         .filter_map(|(def_id, hod)| {
             let def_path_hash = tcx.definitions.def_path_hash(def_id);
             let mut hasher = StableHasher::new();
-            hod.with_bodies.as_ref()?.hash_stable(&mut hcx, &mut hasher);
+            hod.as_ref()?.hash_stable(&mut hcx, &mut hasher);
             AttributeMap { map: &tcx.untracked_crate.attrs, prefix: def_id }
                 .hash_stable(&mut hcx, &mut hasher);
             Some((def_path_hash, hasher.finish()))

compiler/rustc_middle/src/hir/mod.rs

@@ -15,23 +15,25 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::*;
-use rustc_index::vec::IndexVec;
+use rustc_index::vec::{Idx, IndexVec};
 use rustc_span::DUMMY_SP;
 use std::collections::BTreeMap;
 
-#[derive(Debug)]
+/// Result of HIR indexing.
-struct HirOwnerData<'hir> {
-    signature: Option<&'hir Owner<'hir>>,
-    with_bodies: Option<&'hir mut OwnerNodes<'hir>>,
-}
-
 #[derive(Debug)]
 pub struct IndexedHir<'hir> {
-    map: IndexVec<LocalDefId, HirOwnerData<'hir>>,
+    /// Contents of the HIR owned by each definition. None for definitions that are not HIR owners.
+    // The `mut` comes from construction time, and is harmless since we only ever hand out
+    // immutable refs to IndexedHir.
+    map: IndexVec<LocalDefId, Option<&'hir mut OwnerNodes<'hir>>>,
+    /// Map from each owner to its parent's HirId inside another owner.
+    // This map is separate from `map` to eventually allow for per-owner indexing.
     parenting: FxHashMap<LocalDefId, HirId>,
 }
 
-#[derive(Debug)]
+/// Top-level HIR node for current owner. This only contains the node for which
+/// `HirId::local_id == 0`, and excludes bodies.
+#[derive(Copy, Clone, Debug)]
 pub struct Owner<'tcx> {
     node: Node<'tcx>,
 }
@@ -43,6 +45,9 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for Owner<'tcx> {
     }
 }
 
+/// HIR node coupled with its parent's id in the same HIR owner.
+///
+/// The parent is trash when the node is a HIR owner.
 #[derive(Clone, Debug)]
 pub struct ParentedNode<'tcx> {
     parent: ItemLocalId,
@@ -51,8 +56,12 @@ pub struct ParentedNode<'tcx> {
 
 #[derive(Debug)]
 pub struct OwnerNodes<'tcx> {
+    /// Pre-computed hash of the full HIR.
     hash: Fingerprint,
+    /// Full HIR for the current owner.
+    // The zeroth node's parent is trash, but is never accessed.
     nodes: IndexVec<ItemLocalId, Option<ParentedNode<'tcx>>>,
+    /// Content of local bodies.
     bodies: FxHashMap<ItemLocalId, &'tcx Body<'tcx>>,
 }
 
@@ -65,6 +74,8 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for OwnerNodes<'tcx> {
     }
 }
 
+/// Attributes owner by a HIR owner. It is build as a slice inside the attributes map, restricted
+/// to the nodes whose `HirId::owner` is `prefix`.
 #[derive(Copy, Clone)]
 pub struct AttributeMap<'tcx> {
     map: &'tcx BTreeMap<HirId, &'tcx [Attribute]>,
@@ -127,8 +138,12 @@ pub fn provide(providers: &mut Providers) {
     providers.index_hir = map::index_hir;
     providers.crate_hash = map::crate_hash;
     providers.hir_module_items = |tcx, id| &tcx.untracked_crate.modules[&id];
-    providers.hir_owner = |tcx, id| tcx.index_hir(()).map[id].signature;
+    providers.hir_owner = |tcx, id| {
-    providers.hir_owner_nodes = |tcx, id| tcx.index_hir(()).map[id].with_bodies.as_deref();
+        let owner = tcx.index_hir(()).map[id].as_ref()?;
+        let node = owner.nodes[ItemLocalId::new(0)].as_ref()?.node;
+        Some(Owner { node })
+    };
+    providers.hir_owner_nodes = |tcx, id| tcx.index_hir(()).map[id].as_deref();
     providers.hir_owner_parent = |tcx, id| {
         let index = tcx.index_hir(());
         index.parenting.get(&id).copied().unwrap_or(CRATE_HIR_ID)

compiler/rustc_middle/src/query/mod.rs

@@ -47,7 +47,7 @@ rustc_queries! {
     ///
     /// This can be conveniently accessed by methods on `tcx.hir()`.
     /// Avoid calling this query directly.
-    query hir_owner(key: LocalDefId) -> Option<&'tcx crate::hir::Owner<'tcx>> {
+    query hir_owner(key: LocalDefId) -> Option<crate::hir::Owner<'tcx>> {
         eval_always
         desc { |tcx| "HIR owner of `{}`", tcx.def_path_str(key.to_def_id()) }
     }

compiler/rustc_query_impl/src/plumbing.rs

@@ -3,7 +3,7 @@
 //! manage the caches, and so forth.
 
 use super::queries;
-use rustc_middle::dep_graph::{DepKind, DepNode, DepNodeExt, DepNodeIndex, SerializedDepNodeIndex};
+use rustc_middle::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex};
 use rustc_middle::ty::query::on_disk_cache;
 use rustc_middle::ty::tls::{self, ImplicitCtxt};
 use rustc_middle::ty::{self, TyCtxt};
@@ -57,39 +57,6 @@ impl QueryContext for QueryCtxt<'tcx> {
     }
 
     fn try_force_from_dep_node(&self, dep_node: &DepNode) -> bool {
-        // FIXME: This match is just a workaround for incremental bugs and should
-        // 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 => {
-                if let Some(def_id) = dep_node.extract_def_id(**self) {
-                    let def_id = def_id.expect_local();
-                    let hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id);
-                    if def_id != hir_id.owner {
-                        // This `DefPath` does not have a
-                        // corresponding `DepNode` (e.g. a
-                        // struct field), and the ` DefPath`
-                        // collided with the `DefPath` of a
-                        // proper item that existed in the
-                        // previous compilation session.
-                        //
-                        // Since the given `DefPath` does not
-                        // denote the item that previously
-                        // existed, we just fail to mark green.
-                        return false;
-                    }
-                } else {
-                    // If the node does not exist anymore, we
-                    // just fail to mark green.
-                    return false;
-                }
-            }
-            _ => {
-                // For other kinds of nodes it's OK to be
-                // forced.
-            }
-        }
-
         debug!("try_force_from_dep_node({:?}) --- trying to force", dep_node);
 
         // We must avoid ever having to call `force_from_dep_node()` for a