hir: Make sure all HirIds have corresponding HIR Nodes

Author: petrochenkov

compiler/rustc_ast_lowering/src/index.rs

@@ -29,7 +29,7 @@ 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>) {
+) -> (IndexVec<ItemLocalId, ParentedNode<'hir>>, LocalDefIdMap<ItemLocalId>) {
     let mut nodes = IndexVec::new();
     // 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
@@ -54,7 +54,18 @@ pub(super) fn index_hir<'hir>(
         OwnerNode::ForeignItem(item) => collector.visit_foreign_item(item),
     };
 
-    (collector.nodes, collector.parenting)
+    let err = || {
+        let span = item.span();
+        tcx.dcx().span_delayed_bug(*span, "ID not encountered when visiting item HIR");
+        ParentedNode { parent: ItemLocalId::new(0), node: Node::Err(span) }
+    };
+    let nodes = collector
+        .nodes
+        .into_iter()
+        .map(|parented_node| parented_node.unwrap_or_else(err))
+        .collect();
+
+    (nodes, collector.parenting)
 }
 
 impl<'a, 'hir> NodeCollector<'a, 'hir> {
@@ -348,4 +359,28 @@ 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_let_expr(&mut self, lex: &'hir Let<'hir>) {
+        self.insert(lex.span, lex.hir_id, Node::Let(lex));
+        self.with_parent(lex.hir_id, |this| intravisit::walk_let_expr(this, lex))
+    }
+
+    fn visit_array_length(&mut self, len: &'hir ArrayLen) {
+        match len {
+            ArrayLen::Infer(hir_id, span) => self.insert(*span, *hir_id, Node::ArrayLenInfer(span)),
+            ArrayLen::Body(..) => intravisit::walk_array_len(self, len),
+        }
+    }
 }

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 {

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<_>>(),
             )
@@ -3356,13 +3353,14 @@ impl<'hir> OwnerNode<'hir> {
         }
     }
 
-    pub fn span(&self) -> Span {
+    #[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,
         }
     }
 
@@ -3491,17 +3489,20 @@ 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>),
+    Let(&'hir Let<'hir>),
+    // Span by reference to minimize `Node`'s size
+    #[allow(rustc::pass_by_value)]
+    ArrayLenInfer(&'hir Span),
+    #[allow(rustc::pass_by_value)]
+    Err(&'hir Span),
 }
 
 impl<'hir> Node<'hir> {
@@ -3546,7 +3547,11 @@ impl<'hir> Node<'hir> {
             | Node::Crate(..)
             | Node::Ty(..)
             | Node::TraitRef(..)
-            | Node::Infer(..) => None,
+            | Node::Infer(..)
+            | Node::WhereBoundPredicate(..)
+            | Node::Let(..)
+            | Node::ArrayLenInfer(..)
+            | Node::Err(..) => None,
         }
     }
 

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);
         }

compiler/rustc_hir_pretty/src/lib.rs

@@ -117,6 +117,14 @@ 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::Let(lex) => self.print_let(lex.pat, lex.ty, lex.init),
+            Node::ArrayLenInfer(_) => self.word("_"),
+            Node::Err(_) => self.word("/*ERROR*/"),
         }
     }
 }

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1052,7 +1052,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",

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) {

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

@@ -140,9 +140,8 @@ 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).as_owner()?;
-        let node = owner.nodes[id.local_id].as_ref()?;
-        Some(node.node)
+        let owner_nodes = self.hir_owner_nodes(id.owner).unwrap();
+        Some(owner_nodes.nodes[id.local_id].node)
     }
 
     /// Retrieves the `hir::Node` corresponding to `id`, returning `None` if cannot be found.
@@ -214,7 +213,7 @@ impl<'hir> Map<'hir> {
             Some(self.tcx.hir_owner_parent(id.owner))
         } else {
             let owner = self.tcx.hir_owner_nodes(id.owner).as_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);
@@ -989,6 +988,10 @@ 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::Let(lex) => lex.span,
+            Node::ArrayLenInfer(span) => *span,
+            Node::Err(span) => *span,
         }
     }
 
@@ -1250,6 +1253,10 @@ 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::Let(_)) => node_str("let expr"),
+        Some(Node::ArrayLenInfer(_)) => node_str("array len infer"),
+        Some(Node::Err(_)) => node_str("error"),
         None => format!("{id} (unknown node)"),
     }
 }

src/tools/clippy/clippy_lints/src/min_ident_chars.rs

@@ -96,7 +96,7 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {
             let Some(owner) = cx.tcx.hir_owner_nodes(hir_id.owner).as_owner() else {
                 return;
             };
-            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;

src/tools/clippy/tests/ui/manual_map_option_2.fixed

@@ -42,7 +42,10 @@ fn main() {
 
     // Lint. `s` is captured by reference, so no lifetime issues.
     let s = Some(String::new());
-    let _ = s.as_ref().map(|x| { if let Some(ref s) = s { (x.clone(), s) } else { panic!() } });
+    let _ = match &s {
+        Some(x) => Some({ if let Some(ref s) = s { (x.clone(), s) } else { panic!() } }),
+        None => None,
+    };
 
     // Issue #7820
     unsafe fn f(x: u32) -> u32 {

src/tools/clippy/tests/ui/manual_map_option_2.stderr

@@ -21,16 +21,6 @@ LL +             (x, y.0)
 LL ~         });
    |
 
-error: manual implementation of `Option::map`
-  --> $DIR/manual_map_option_2.rs:48:13
-   |
-LL |       let _ = match &s {
-   |  _____________^
-LL | |         Some(x) => Some({ if let Some(ref s) = s { (x.clone(), s) } else { panic!() } }),
-LL | |         None => None,
-LL | |     };
-   | |_____^ help: try: `s.as_ref().map(|x| { if let Some(ref s) = s { (x.clone(), s) } else { panic!() } })`
-
 error: manual implementation of `Option::map`
   --> $DIR/manual_map_option_2.rs:58:17
    |
@@ -61,5 +51,5 @@ LL | |         None => None,
 LL | |     };
    | |_____^ help: try: `Some(0).map(|x| unsafe { f(x) })`
 
-error: aborting due to 5 previous errors
+error: aborting due to 4 previous errors
 

src/tools/clippy/tests/ui/shadow.stderr

@@ -244,7 +244,7 @@ note: previous binding is here
 LL |     let _ = |[x]: [u32; 1]| {
    |               ^
 
-error: `y` is shadowed
+error: `y` shadows a previous, unrelated binding
   --> $DIR/shadow.rs:76:17
    |
 LL |     if let Some(y) = y {}