hir: Make sure all HirIds have corresponding HIR Nodes

petrochenkov · petrochenkov · commit 3ff6796460b1 · 2024-01-31T21:10:31.000+03:00
diff --git a/compiler/rustc_ast_lowering/src/index.rs b/compiler/rustc_ast_lowering/src/index.rs
@@ -15,7 +15,7 @@ struct NodeCollector<'a, 'hir> {
     bodies: &'a SortedMap<ItemLocalId, &'hir Body<'hir>>,
 
     /// Outputs
-    nodes: IndexVec<ItemLocalId, Option<ParentedNode<'hir>>>,
+    nodes: IndexVec<ItemLocalId, ParentedNode<'hir>>,
     parenting: LocalDefIdMap<ItemLocalId>,
 
     /// The parent of this node
@@ -29,16 +29,19 @@ pub(super) fn index_hir<'hir>(
     tcx: TyCtxt<'hir>,
     item: hir::OwnerNode<'hir>,
     bodies: &SortedMap<ItemLocalId, &'hir Body<'hir>>,
-) -> (IndexVec<ItemLocalId, Option<ParentedNode<'hir>>>, LocalDefIdMap<ItemLocalId>) {
-    let mut nodes = IndexVec::new();
+    num_nodes: usize,
+) -> (IndexVec<ItemLocalId, ParentedNode<'hir>>, LocalDefIdMap<ItemLocalId>) {
+    let zero_id = ItemLocalId::new(0);
+    let err_node = ParentedNode { parent: zero_id, node: Node::Err(item.span()) };
+    let mut nodes = IndexVec::from_elem_n(err_node, num_nodes);
     // This node's parent should never be accessed: the owner's parent is computed by the
     // hir_owner_parent query. Make it invalid (= ItemLocalId::MAX) to force an ICE whenever it is
     // used.
-    nodes.push(Some(ParentedNode { parent: ItemLocalId::INVALID, node: item.into() }));
+    nodes[zero_id] = ParentedNode { parent: ItemLocalId::INVALID, node: item.into() };
     let mut collector = NodeCollector {
         tcx,
         owner: item.def_id(),
-        parent_node: ItemLocalId::new(0),
+        parent_node: zero_id,
         nodes,
         bodies,
         parenting: Default::default(),
@@ -54,6 +57,14 @@ pub(super) fn index_hir<'hir>(
         OwnerNode::ForeignItem(item) => collector.visit_foreign_item(item),
     };
 
+    for (local_id, node) in collector.nodes.iter_enumerated() {
+        if let Node::Err(span) = node.node {
+            let hir_id = HirId { owner: item.def_id(), local_id };
+            let msg = format!("ID {hir_id} not encountered when visiting item HIR");
+            tcx.dcx().span_delayed_bug(*span, msg);
+        }
+    }
+
     (collector.nodes, collector.parenting)
 }
 
@@ -88,7 +99,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
             }
         }
 
-        self.nodes.insert(hir_id.local_id, ParentedNode { parent: self.parent_node, node });
+        self.nodes[hir_id.local_id] = ParentedNode { parent: self.parent_node, node };
     }
 
     fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_node_id: HirId, f: F) {
@@ -348,4 +359,23 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
 
         self.visit_nested_foreign_item(id);
     }
+
+    fn visit_where_predicate(&mut self, predicate: &'hir WherePredicate<'hir>) {
+        match predicate {
+            WherePredicate::BoundPredicate(pred) => {
+                self.insert(pred.span, pred.hir_id, Node::WhereBoundPredicate(pred));
+                self.with_parent(pred.hir_id, |this| {
+                    intravisit::walk_where_predicate(this, predicate)
+                })
+            }
+            _ => intravisit::walk_where_predicate(self, predicate),
+        }
+    }
+
+    fn visit_array_length(&mut self, len: &'hir ArrayLen) {
+        match len {
+            ArrayLen::Infer(inf) => self.insert(inf.span, inf.hir_id, Node::ArrayLenInfer(inf)),
+            ArrayLen::Body(..) => intravisit::walk_array_len(self, len),
+        }
+    }
 }
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
@@ -674,7 +674,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         } else {
             (None, None)
         };
-        let (nodes, parenting) = index::index_hir(self.tcx, node, &bodies);
+        let num_nodes = self.item_local_id_counter.as_usize();
+        let (nodes, parenting) = index::index_hir(self.tcx, node, &bodies, num_nodes);
         let nodes = hir::OwnerNodes { opt_hash_including_bodies, nodes, bodies };
         let attrs = hir::AttributeMap { map: attrs, opt_hash: attrs_hash };
 
diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs
@@ -216,7 +216,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 if let Some(id) = placeholder.bound.kind.get_id()
                     && let Some(placeholder_id) = id.as_local()
                     && let gat_hir_id = self.infcx.tcx.local_def_id_to_hir_id(placeholder_id)
-                    && let Some(generics_impl) = hir.get_parent(gat_hir_id).generics()
+                    && let Some(generics_impl) =
+                        hir.get_parent(hir.parent_id(gat_hir_id)).generics()
                 {
                     Some((gat_hir_id, generics_impl))
                 } else {
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -835,17 +835,16 @@ pub struct OwnerNodes<'tcx> {
     // The zeroth node's parent should never be accessed: the owner's parent is computed by the
     // hir_owner_parent query. It is set to `ItemLocalId::INVALID` to force an ICE if accidentally
     // used.
-    pub nodes: IndexVec<ItemLocalId, Option<ParentedNode<'tcx>>>,
+    pub nodes: IndexVec<ItemLocalId, ParentedNode<'tcx>>,
     /// Content of local bodies.
     pub bodies: SortedMap<ItemLocalId, &'tcx Body<'tcx>>,
 }
 
 impl<'tcx> OwnerNodes<'tcx> {
     pub fn node(&self) -> OwnerNode<'tcx> {
         use rustc_index::Idx;
-        let node = self.nodes[ItemLocalId::new(0)].as_ref().unwrap().node;
-        let node = node.as_owner().unwrap(); // Indexing must ensure it is an OwnerNode.
-        node
+        // Indexing must ensure it is an OwnerNode.
+        self.nodes[ItemLocalId::new(0)].node.as_owner().unwrap()
     }
 }
 
@@ -860,9 +859,7 @@ impl fmt::Debug for OwnerNodes<'_> {
                     .nodes
                     .iter_enumerated()
                     .map(|(id, parented_node)| {
-                        let parented_node = parented_node.as_ref().map(|node| node.parent);
-
-                        debug_fn(move |f| write!(f, "({id:?}, {parented_node:?})"))
+                        debug_fn(move |f| write!(f, "({id:?}, {:?})", parented_node.parent))
                     })
                     .collect::<Vec<_>>(),
             )
@@ -3357,13 +3354,15 @@ impl<'hir> OwnerNode<'hir> {
         }
     }
 
-    pub fn span(&self) -> Span {
+    // Span by reference to pass to `Node::Err`.
+    #[allow(rustc::pass_by_value)]
+    pub fn span(&self) -> &'hir Span {
         match self {
             OwnerNode::Item(Item { span, .. })
             | OwnerNode::ForeignItem(ForeignItem { span, .. })
             | OwnerNode::ImplItem(ImplItem { span, .. })
-            | OwnerNode::TraitItem(TraitItem { span, .. }) => *span,
-            OwnerNode::Crate(Mod { spans: ModSpans { inner_span, .. }, .. }) => *inner_span,
+            | OwnerNode::TraitItem(TraitItem { span, .. }) => span,
+            OwnerNode::Crate(Mod { spans: ModSpans { inner_span, .. }, .. }) => inner_span,
         }
     }
 
@@ -3492,17 +3491,18 @@ pub enum Node<'hir> {
     Arm(&'hir Arm<'hir>),
     Block(&'hir Block<'hir>),
     Local(&'hir Local<'hir>),
-
     /// `Ctor` refers to the constructor of an enum variant or struct. Only tuple or unit variants
     /// with synthesized constructors.
     Ctor(&'hir VariantData<'hir>),
-
     Lifetime(&'hir Lifetime),
     GenericParam(&'hir GenericParam<'hir>),
-
     Crate(&'hir Mod<'hir>),
-
     Infer(&'hir InferArg),
+    WhereBoundPredicate(&'hir WhereBoundPredicate<'hir>),
+    ArrayLenInfer(&'hir InferArg),
+    // Span by reference to minimize `Node`'s size
+    #[allow(rustc::pass_by_value)]
+    Err(&'hir Span),
 }
 
 impl<'hir> Node<'hir> {
@@ -3547,7 +3547,10 @@ impl<'hir> Node<'hir> {
             | Node::Crate(..)
             | Node::Ty(..)
             | Node::TraitRef(..)
-            | Node::Infer(..) => None,
+            | Node::Infer(..)
+            | Node::WhereBoundPredicate(..)
+            | Node::ArrayLenInfer(..)
+            | Node::Err(..) => None,
         }
     }
 
diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs
@@ -509,6 +509,8 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<Ty
             x => bug!("unexpected non-type Node::GenericParam: {:?}", x),
         },
 
+        Node::ArrayLenInfer(_) => tcx.types.usize,
+
         x => {
             bug!("unexpected sort of node in type_of(): {:?}", x);
         }
diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs
@@ -117,6 +117,13 @@ impl<'a> State<'a> {
             Node::Ctor(..) => panic!("cannot print isolated Ctor"),
             Node::Local(a) => self.print_local_decl(a),
             Node::Crate(..) => panic!("cannot print Crate"),
+            Node::WhereBoundPredicate(pred) => {
+                self.print_formal_generic_params(pred.bound_generic_params);
+                self.print_type(pred.bounded_ty);
+                self.print_bounds(":", pred.bounds);
+            }
+            Node::ArrayLenInfer(_) => self.word("_"),
+            Node::Err(_) => self.word("/*ERROR*/"),
         }
     }
 }
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
@@ -1053,7 +1053,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             if self.can_coerce(found, ty) {
                 if let Some(node) = self.tcx.opt_hir_node(fn_id)
                     && let Some(owner_node) = node.as_owner()
-                    && let Some(span) = expr.span.find_ancestor_inside(owner_node.span())
+                    && let Some(span) = expr.span.find_ancestor_inside(*owner_node.span())
                 {
                     err.multipart_suggestion(
                         "you might have meant to return this value",
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -1388,13 +1388,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             if should_encode_fn_sig(def_kind) {
                 record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
             }
-            // FIXME: Some anonymous constants produced by `#[rustc_legacy_const_generics]`
-            // do not have corresponding HIR nodes, so some queries usually making sense for
-            // anonymous constants will not work on them and panic. It's not clear whether it
-            // can cause any observable issues or not.
-            let anon_const_without_hir = def_kind == DefKind::AnonConst
-                && tcx.opt_hir_node(tcx.local_def_id_to_hir_id(local_id)).is_none();
-            if should_encode_generics(def_kind) && !anon_const_without_hir {
+            if should_encode_generics(def_kind) {
                 let g = tcx.generics_of(def_id);
                 record!(self.tables.generics_of[def_id] <- g);
                 record!(self.tables.explicit_predicates_of[def_id] <- self.tcx.explicit_predicates_of(def_id));
@@ -1408,7 +1402,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                     }
                 }
             }
-            if should_encode_type(tcx, local_id, def_kind) && !anon_const_without_hir {
+            if should_encode_type(tcx, local_id, def_kind) {
                 record!(self.tables.type_of[def_id] <- self.tcx.type_of(def_id));
             }
             if should_encode_constness(def_kind) {
diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -159,9 +159,7 @@ impl<'tcx> TyCtxt<'tcx> {
 
     /// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
     pub fn opt_hir_node(self, id: HirId) -> Option<Node<'tcx>> {
-        let owner = self.hir_owner_nodes(id.owner);
-        let node = owner.nodes[id.local_id].as_ref()?;
-        Some(node.node)
+        Some(self.hir_owner_nodes(id.owner).nodes[id.local_id].node)
     }
 
     /// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
@@ -233,7 +231,7 @@ impl<'hir> Map<'hir> {
             Some(self.tcx.hir_owner_parent(id.owner))
         } else {
             let owner = self.tcx.hir_owner_nodes(id.owner);
-            let node = owner.nodes[id.local_id].as_ref()?;
+            let node = &owner.nodes[id.local_id];
             let hir_id = HirId { owner: id.owner, local_id: node.parent };
             // HIR indexing should have checked that.
             debug_assert_ne!(id.local_id, node.parent);
@@ -994,6 +992,9 @@ impl<'hir> Map<'hir> {
             Node::Infer(i) => i.span,
             Node::Local(local) => local.span,
             Node::Crate(item) => item.spans.inner_span,
+            Node::WhereBoundPredicate(pred) => pred.span,
+            Node::ArrayLenInfer(inf) => inf.span,
+            Node::Err(span) => *span,
         }
     }
 
@@ -1255,6 +1256,9 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {
             format!("{id} (generic_param {})", path_str(param.def_id))
         }
         Some(Node::Crate(..)) => String::from("(root_crate)"),
+        Some(Node::WhereBoundPredicate(_)) => node_str("where bound predicate"),
+        Some(Node::ArrayLenInfer(_)) => node_str("array len infer"),
+        Some(Node::Err(_)) => node_str("error"),
         None => format!("{id} (unknown node)"),
     }
 }
diff --git a/src/tools/clippy/clippy_lints/src/min_ident_chars.rs b/src/tools/clippy/clippy_lints/src/min_ident_chars.rs
@@ -94,7 +94,7 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {
             cx.tcx.opt_hir_node(hir_id)
         } else {
             let owner = cx.tcx.hir_owner_nodes(hir_id.owner);
-            owner.nodes.get(hir_id.local_id).copied().flatten().map(|p| p.node)
+            owner.nodes.get(hir_id.local_id).copied().map(|p| p.node)
         };
         let Some(node) = node else {
             return;

Original file line number	Diff line number	Diff line change
`@@ -509,6 +509,8 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<Ty`
`509`	`509`	`x => bug!("unexpected non-type Node::GenericParam: {:?}", x),`
`510`	`510`	`},`
`511`	`511`
	`512`	`+ Node::ArrayLenInfer(_) => tcx.types.usize,`
	`513`	`+`
`512`	`514`	`x => {`
`513`	`515`	`bug!("unexpected sort of node in type_of(): {:?}", x);`
`514`	`516`	`}`
Original file line number	Diff line number	Diff line change
`@@ -117,6 +117,13 @@ impl<'a> State<'a> {`
`117`	`117`	`Node::Ctor(..) => panic!("cannot print isolated Ctor"),`
`118`	`118`	`Node::Local(a) => self.print_local_decl(a),`
`119`	`119`	`Node::Crate(..) => panic!("cannot print Crate"),`
	`120`	`+ Node::WhereBoundPredicate(pred) => {`
	`121`	`+ self.print_formal_generic_params(pred.bound_generic_params);`
	`122`	`+ self.print_type(pred.bounded_ty);`
	`123`	`+ self.print_bounds(":", pred.bounds);`
	`124`	`+ }`
	`125`	`+ Node::ArrayLenInfer(_) => self.word("_"),`
	`126`	`+ Node::Err(_) => self.word("/ERROR/"),`
`120`	`127`	`}`
`121`	`128`	`}`
`122`	`129`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1053,7 +1053,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {`
`1053`	`1053`	`if self.can_coerce(found, ty) {`
`1054`	`1054`	`if let Some(node) = self.tcx.opt_hir_node(fn_id)`
`1055`	`1055`	`&& let Some(owner_node) = node.as_owner()`
`1056`		`- && let Some(span) = expr.span.find_ancestor_inside(owner_node.span())`
	`1056`	`+ && let Some(span) = expr.span.find_ancestor_inside(*owner_node.span())`
`1057`	`1057`	`{`
`1058`	`1058`	`err.multipart_suggestion(`
`1059`	`1059`	`"you might have meant to return this value",`
Original file line number	Diff line number	Diff line change
`@@ -1388,13 +1388,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {`
`1388`	`1388`	`if should_encode_fn_sig(def_kind) {`
`1389`	`1389`	`record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));`
`1390`	`1390`	`}`
`1391`		- // FIXME: Some anonymous constants produced by `#[rustc_legacy_const_generics]`
`1392`		`- // do not have corresponding HIR nodes, so some queries usually making sense for`
`1393`		`- // anonymous constants will not work on them and panic. It's not clear whether it`
`1394`		`- // can cause any observable issues or not.`
`1395`		`- let anon_const_without_hir = def_kind == DefKind::AnonConst`
`1396`		`- && tcx.opt_hir_node(tcx.local_def_id_to_hir_id(local_id)).is_none();`
`1397`		`- if should_encode_generics(def_kind) && !anon_const_without_hir {`
	`1391`	`+ if should_encode_generics(def_kind) {`
`1398`	`1392`	`let g = tcx.generics_of(def_id);`
`1399`	`1393`	`record!(self.tables.generics_of[def_id] <- g);`
`1400`	`1394`	`record!(self.tables.explicit_predicates_of[def_id] <- self.tcx.explicit_predicates_of(def_id));`
`@@ -1408,7 +1402,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {`
`1408`	`1402`	`}`
`1409`	`1403`	`}`
`1410`	`1404`	`}`
`1411`		`- if should_encode_type(tcx, local_id, def_kind) && !anon_const_without_hir {`
	`1405`	`+ if should_encode_type(tcx, local_id, def_kind) {`
`1412`	`1406`	`record!(self.tables.type_of[def_id] <- self.tcx.type_of(def_id));`
`1413`	`1407`	`}`
`1414`	`1408`	`if should_encode_constness(def_kind) {`
Original file line number	Diff line number	Diff line change
`@@ -159,9 +159,7 @@ impl<'tcx> TyCtxt<'tcx> {`
`159`	`159`
`160`	`160`	/// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
`161`	`161`	`pub fn opt_hir_node(self, id: HirId) -> Option<Node<'tcx>> {`
`162`		`- let owner = self.hir_owner_nodes(id.owner);`
`163`		`- let node = owner.nodes[id.local_id].as_ref()?;`
`164`		`- Some(node.node)`
	`162`	`+ Some(self.hir_owner_nodes(id.owner).nodes[id.local_id].node)`
`165`	`163`	`}`
`166`	`164`
`167`	`165`	/// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
`@@ -233,7 +231,7 @@ impl<'hir> Map<'hir> {`
`233`	`231`	`Some(self.tcx.hir_owner_parent(id.owner))`
`234`	`232`	`} else {`
`235`	`233`	`let owner = self.tcx.hir_owner_nodes(id.owner);`
`236`		`- let node = owner.nodes[id.local_id].as_ref()?;`
	`234`	`+ let node = &owner.nodes[id.local_id];`
`237`	`235`	`let hir_id = HirId { owner: id.owner, local_id: node.parent };`
`238`	`236`	`// HIR indexing should have checked that.`
`239`	`237`	`debug_assert_ne!(id.local_id, node.parent);`
`@@ -994,6 +992,9 @@ impl<'hir> Map<'hir> {`
`994`	`992`	`Node::Infer(i) => i.span,`
`995`	`993`	`Node::Local(local) => local.span,`
`996`	`994`	`Node::Crate(item) => item.spans.inner_span,`
	`995`	`+ Node::WhereBoundPredicate(pred) => pred.span,`
	`996`	`+ Node::ArrayLenInfer(inf) => inf.span,`
	`997`	`+ Node::Err(span) => *span,`
`997`	`998`	`}`
`998`	`999`	`}`
`999`	`1000`
`@@ -1255,6 +1256,9 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {`
`1255`	`1256`	`format!("{id} (generic_param {})", path_str(param.def_id))`
`1256`	`1257`	`}`
`1257`	`1258`	`Some(Node::Crate(..)) => String::from("(root_crate)"),`
	`1259`	`+ Some(Node::WhereBoundPredicate(_)) => node_str("where bound predicate"),`
	`1260`	`+ Some(Node::ArrayLenInfer(_)) => node_str("array len infer"),`
	`1261`	`+ Some(Node::Err(_)) => node_str("error"),`
`1258`	`1262`	`None => format!("{id} (unknown node)"),`
`1259`	`1263`	`}`
`1260`	`1264`	`}`