Rollup merge of #107515 - Swatinem:hirvalidator, r=compiler-errors

Improve pretty-printing of `HirIdValidator` errors

This now uses `node_to_string` for both missing and seen Ids, which includes the snippet of code for which the Id was allocated. Also removes the duplicated printing of `HirId`, as `node_to_string` also includes that.

7 files changed, 32 insertions, 49 deletions

diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
index a355a54d695..237142acca6 100644
--- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
+++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
@@ -454,8 +454,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             None if let Some(e) = self.tainted_by_errors() => self.tcx.ty_error_with_guaranteed(e),
             None => {
                 bug!(
-                    "no type for node {}: {} in fcx {}",
-                    id,
+                    "no type for node {} in fcx {}",
                     self.tcx.hir().node_to_string(id),
                     self.tag()
                 );
diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs
index 48c75cde9a5..92240b66eb1 100644
--- a/compiler/rustc_hir_typeck/src/mem_categorization.rs
+++ b/compiler/rustc_hir_typeck/src/mem_categorization.rs
@@ -155,8 +155,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> {
             None if self.is_tainted_by_errors() => Err(()),
             None => {
                 bug!(
-                    "no type for node {}: {} in mem_categorization",
-                    id,
+                    "no type for node {} in mem_categorization",
                     self.tcx().hir().node_to_string(id)
                 );
             }
diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
index 5bd6b070442..7f2994fd79b 100644
--- a/compiler/rustc_middle/src/hir/map/mod.rs
+++ b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -290,7 +290,7 @@ impl<'hir> Map<'hir> {
     #[track_caller]
     pub fn parent_id(self, hir_id: HirId) -> HirId {
         self.opt_parent_id(hir_id)
-            .unwrap_or_else(|| bug!("No parent for node {:?}", self.node_to_string(hir_id)))
+            .unwrap_or_else(|| bug!("No parent for node {}", self.node_to_string(hir_id)))
     }
 
     pub fn get_parent(self, hir_id: HirId) -> Node<'hir> {
@@ -1191,12 +1191,10 @@ fn upstream_crates(tcx: TyCtxt<'_>) -> Vec<(StableCrateId, Svh)> {
 }
 
 fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {
-    let id_str = format!(" (hir_id={})", id);
-
     let path_str = |def_id: LocalDefId| map.tcx.def_path_str(def_id.to_def_id());
 
     let span_str = || map.tcx.sess.source_map().span_to_snippet(map.span(id)).unwrap_or_default();
-    let node_str = |prefix| format!("{} {}{}", prefix, span_str(), id_str);
+    let node_str = |prefix| format!("{id} ({prefix} `{}`)", span_str());
 
     match map.find(id) {
         Some(Node::Item(item)) => {
@@ -1225,10 +1223,10 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {
                 ItemKind::TraitAlias(..) => "trait alias",
                 ItemKind::Impl { .. } => "impl",
             };
-            format!("{} {}{}", item_str, path_str(item.owner_id.def_id), id_str)
+            format!("{id} ({item_str} {})", path_str(item.owner_id.def_id))
         }
         Some(Node::ForeignItem(item)) => {
-            format!("foreign item {}{}", path_str(item.owner_id.def_id), id_str)
+            format!("{id} (foreign item {})", path_str(item.owner_id.def_id))
         }
         Some(Node::ImplItem(ii)) => {
             let kind = match ii.kind {
@@ -1236,7 +1234,7 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {
                 ImplItemKind::Fn(..) => "method",
                 ImplItemKind::Type(_) => "assoc type",
             };
-            format!("{} {} in {}{}", kind, ii.ident, path_str(ii.owner_id.def_id), id_str)
+            format!("{id} ({kind} `{}` in {})", ii.ident, path_str(ii.owner_id.def_id))
         }
         Some(Node::TraitItem(ti)) => {
             let kind = match ti.kind {
@@ -1245,13 +1243,13 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {
                 TraitItemKind::Type(..) => "assoc type",
             };
 
-            format!("{} {} in {}{}", kind, ti.ident, path_str(ti.owner_id.def_id), id_str)
+            format!("{id} ({kind} `{}` in {})", ti.ident, path_str(ti.owner_id.def_id))
         }
         Some(Node::Variant(ref variant)) => {
-            format!("variant {} in {}{}", variant.ident, path_str(variant.def_id), id_str)
+            format!("{id} (variant `{}` in {})", variant.ident, path_str(variant.def_id))
         }
         Some(Node::Field(ref field)) => {
-            format!("field {} in {}{}", field.ident, path_str(field.def_id), id_str)
+            format!("{id} (field `{}` in {})", field.ident, path_str(field.def_id))
         }
         Some(Node::AnonConst(_)) => node_str("const"),
         Some(Node::Expr(_)) => node_str("expr"),
@@ -1269,16 +1267,15 @@ fn hir_id_to_string(map: Map<'_>, id: HirId) -> String {
         Some(Node::Infer(_)) => node_str("infer"),
         Some(Node::Local(_)) => node_str("local"),
         Some(Node::Ctor(ctor)) => format!(
-            "ctor {}{}",
+            "{id} (ctor {})",
             ctor.ctor_def_id().map_or("<missing path>".into(), |def_id| path_str(def_id)),
-            id_str
         ),
         Some(Node::Lifetime(_)) => node_str("lifetime"),
         Some(Node::GenericParam(ref param)) => {
-            format!("generic_param {}{}", path_str(param.def_id), id_str)
+            format!("{id} (generic_param {})", path_str(param.def_id))
         }
-        Some(Node::Crate(..)) => String::from("root_crate"),
-        None => format!("unknown node{}", id_str),
+        Some(Node::Crate(..)) => String::from("(root_crate)"),
+        None => format!("{id} (unknown node)"),
     }
 }
 
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
index b63b9e754cf..f98172e4201 100644
--- a/compiler/rustc_middle/src/ty/context.rs
+++ b/compiler/rustc_middle/src/ty/context.rs
@@ -2171,7 +2171,7 @@ impl<'tcx> TyCtxt<'tcx> {
             self.late_bound_vars_map(id.owner)
                 .and_then(|map| map.get(&id.local_id).cloned())
                 .unwrap_or_else(|| {
-                    bug!("No bound vars found for {:?} ({:?})", self.hir().node_to_string(id), id)
+                    bug!("No bound vars found for {}", self.hir().node_to_string(id))
                 })
                 .iter(),
         )
diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs
index 79a6c730d71..9beaac87183 100644
--- a/compiler/rustc_middle/src/ty/typeck_results.rs
+++ b/compiler/rustc_middle/src/ty/typeck_results.rs
@@ -372,7 +372,7 @@ impl<'tcx> TypeckResults<'tcx> {
 
     pub fn node_type(&self, id: hir::HirId) -> Ty<'tcx> {
         self.node_type_opt(id).unwrap_or_else(|| {
-            bug!("node_type: no type for node `{}`", tls::with(|tcx| tcx.hir().node_to_string(id)))
+            bug!("node_type: no type for node {}", tls::with(|tcx| tcx.hir().node_to_string(id)))
         })
     }
 
@@ -551,9 +551,8 @@ fn validate_hir_id_for_typeck_results(hir_owner: OwnerId, hir_id: hir::HirId) {
 fn invalid_hir_id_for_typeck_results(hir_owner: OwnerId, hir_id: hir::HirId) {
     ty::tls::with(|tcx| {
         bug!(
-            "node {} with HirId::owner {:?} cannot be placed in TypeckResults with hir_owner {:?}",
+            "node {} cannot be placed in TypeckResults with hir_owner {:?}",
             tcx.hir().node_to_string(hir_id),
-            hir_id.owner,
             hir_owner
         )
     });
diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs
index d143adb2eb9..de0e50a65de 100644
--- a/compiler/rustc_passes/src/hir_id_validator.rs
+++ b/compiler/rustc_passes/src/hir_id_validator.rs
@@ -74,37 +74,26 @@ impl<'a, 'hir> HirIdValidator<'a, 'hir> {
             .expect("owning item has no entry");
 
         if max != self.hir_ids_seen.len() - 1 {
-            // Collect the missing ItemLocalIds
-            let missing: Vec<_> = (0..=max as u32)
-                .filter(|&i| !self.hir_ids_seen.contains(ItemLocalId::from_u32(i)))
-                .collect();
-
-            // Try to map those to something more useful
-            let mut missing_items = Vec::with_capacity(missing.len());
+            let hir = self.tcx.hir();
+            let pretty_owner = hir.def_path(owner.def_id).to_string_no_crate_verbose();
 
-            for local_id in missing {
-                let hir_id = HirId { owner, local_id: ItemLocalId::from_u32(local_id) };
+            let missing_items: Vec<_> = (0..=max as u32)
+                .map(|i| ItemLocalId::from_u32(i))
+                .filter(|&local_id| !self.hir_ids_seen.contains(local_id))
+                .map(|local_id| hir.node_to_string(HirId { owner, local_id }))
+                .collect();
 
-                trace!("missing hir id {:#?}", hir_id);
+            let seen_items: Vec<_> = self
+                .hir_ids_seen
+                .iter()
+                .map(|local_id| hir.node_to_string(HirId { owner, local_id }))
+                .collect();
 
-                missing_items.push(format!(
-                    "[local_id: {}, owner: {}]",
-                    local_id,
-                    self.tcx.hir().def_path(owner.def_id).to_string_no_crate_verbose()
-                ));
-            }
             self.error(|| {
                 format!(
                     "ItemLocalIds not assigned densely in {}. \
-                Max ItemLocalId = {}, missing IDs = {:#?}; seens IDs = {:#?}",
-                    self.tcx.hir().def_path(owner.def_id).to_string_no_crate_verbose(),
-                    max,
-                    missing_items,
-                    self.hir_ids_seen
-                        .iter()
-                        .map(|local_id| HirId { owner, local_id })
-                        .map(|h| format!("({:?} {})", h, self.tcx.hir().node_to_string(h)))
-                        .collect::<Vec<_>>()
+                Max ItemLocalId = {}, missing IDs = {:#?}; seen IDs = {:#?}",
+                    pretty_owner, max, missing_items, seen_items
                 )
             });
         }
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
index b35b9d62759..87dbf7c3fd6 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -133,7 +133,7 @@ impl<'tcx, 'a> GeneratorData<'tcx, 'a> {
                                         .cloned()
                                         .unwrap_or_else(|| {
                                             bug!(
-                                                "node_type: no type for node `{}`",
+                                                "node_type: no type for node {}",
                                                 ty::tls::with(|tcx| tcx
                                                     .hir()
                                                     .node_to_string(await_expr.hir_id))