rustc: rewrite the HirId validator to only check HIR map compactness.

src/librustc/hir/intravisit.rs

@@ -803,11 +803,9 @@ pub fn walk_where_predicate<'v, V: Visitor<'v>>(
             visitor.visit_lifetime(lifetime);
             walk_list!(visitor, visit_param_bound, bounds);
         }
-        &WherePredicate::EqPredicate(WhereEqPredicate{hir_id,
-                                                      ref lhs_ty,
+        &WherePredicate::EqPredicate(WhereEqPredicate{ref lhs_ty,
                                                       ref rhs_ty,
                                                       ..}) => {
-            visitor.visit_id(hir_id);
             visitor.visit_ty(lhs_ty);
             visitor.visit_ty(rhs_ty);
         }

src/librustc/hir/lowering/item.rs

@@ -1442,13 +1442,11 @@ impl LoweringContext<'_> {
                 bounds: self.lower_param_bounds(bounds, ImplTraitContext::disallowed()),
             }),
             WherePredicate::EqPredicate(WhereEqPredicate {
-                id,
                 ref lhs_ty,
                 ref rhs_ty,
                 span,
             }) => {
                 hir::WherePredicate::EqPredicate(hir::WhereEqPredicate {
-                    hir_id: self.lower_node_id(id),
                     lhs_ty: self.lower_ty(lhs_ty, ImplTraitContext::disallowed()),
                     rhs_ty: self.lower_ty(rhs_ty, ImplTraitContext::disallowed()),
                     span,

src/librustc/hir/map/collector.rs

@@ -433,6 +433,19 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         self.with_parent(pat.hir_id, |this| {
             intravisit::walk_pat(this, pat);
         });
+        self.with_parent(pat.hir_id, |this| match pat.kind {
+            // FIXME(eddyb) add `visit_field_pat` to `intravisit`.
+            PatKind::Struct(ref qpath, ref fields, _) => {
+                this.visit_qpath(qpath, pat.hir_id, pat.span);
+                for field in fields {
+                    this.insert(field.span, field.hir_id, Node::FieldPat(field));
+                    this.with_parent(field.hir_id, |this| {
+                        this.visit_pat(&field.pat);
+                    });
+                }
+            }
+            _ => intravisit::walk_pat(this, pat),
+        });
     }
 
     fn visit_arm(&mut self, arm: &'hir Arm) {
@@ -456,8 +469,21 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     fn visit_expr(&mut self, expr: &'hir Expr) {
         self.insert(expr.span, expr.hir_id, Node::Expr(expr));
 
-        self.with_parent(expr.hir_id, |this| {
-            intravisit::walk_expr(this, expr);
+        self.with_parent(expr.hir_id, |this| match expr.kind {
+            // FIXME(eddyb) add `visit_field` to `intravisit`.
+            ExprKind::Struct(ref qpath, ref fields, ref optional_base) => {
+                this.visit_qpath(qpath, expr.hir_id, expr.span);
+                for field in fields {
+                    this.insert(field.span, field.hir_id, Node::Field(field));
+                    this.with_parent(field.hir_id, |this| {
+                        this.visit_expr(&field.expr);
+                    });
+                }
+                if let Some(base) = optional_base {
+                    this.visit_expr(base);
+                }
+            }
+            _ => intravisit::walk_expr(this, expr),
         });
     }
 
@@ -484,6 +510,14 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         });
     }
 
+    fn visit_assoc_type_binding(&mut self, type_binding: &'hir TypeBinding) {
+        self.insert(type_binding.span, type_binding.hir_id, Node::TypeBinding(type_binding));
+
+        self.with_parent(type_binding.hir_id, |this| {
+            intravisit::walk_assoc_type_binding(this, type_binding);
+        });
+    }
+
     fn visit_trait_ref(&mut self, tr: &'hir TraitRef) {
         self.insert(tr.path.span, tr.hir_ref_id, Node::TraitRef(tr));
 
@@ -551,7 +585,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     }
 
     fn visit_struct_field(&mut self, field: &'hir StructField) {
-        self.insert(field.span, field.hir_id, Node::Field(field));
+        self.insert(field.span, field.hir_id, Node::StructField(field));
         self.with_parent(field.hir_id, |this| {
             intravisit::walk_struct_field(this, field);
         });

src/librustc/hir/map/hir_id_validator.rs

@@ -1,170 +1,44 @@
-use crate::hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX};
-use crate::hir::{self, intravisit, HirId, ItemLocalId};
-use crate::hir::itemlikevisit::ItemLikeVisitor;
-use rustc_data_structures::fx::FxHashSet;
+use crate::hir::def_id::{DefId, DefIndex};
+use crate::hir::{self, HirId};
 use rustc_data_structures::sync::{Lock, ParallelIterator, par_iter};
+use std::collections::BTreeMap;
 
 pub fn check_crate(hir_map: &hir::map::Map<'_>) {
     hir_map.dep_graph.assert_ignored();
 
     let errors = Lock::new(Vec::new());
 
-    par_iter(&hir_map.krate().modules).for_each(|(module_id, _)| {
-        let local_def_id = hir_map.local_def_id(*module_id);
-        hir_map.visit_item_likes_in_module(local_def_id, &mut OuterVisitor {
-            hir_map,
-            errors: &errors,
-        });
+    par_iter(0..hir_map.map.len()).for_each(|owner| {
+        let owner = DefIndex::from(owner);
+        let local_map = &hir_map.map[owner];
+
+        // Collect the missing `ItemLocalId`s.
+        let missing: Vec<_> = local_map
+            .iter_enumerated()
+            .filter(|(_, entry)| entry.is_none())
+            .map(|(local_id, _)| local_id)
+            .collect();
+
+        if !missing.is_empty() {
+            let present: BTreeMap<_, _> = local_map
+                .iter_enumerated()
+                .filter(|(_, entry)| entry.is_some())
+                .map(|(local_id, _)| {
+                    (local_id, hir_map.node_to_string(HirId { owner, local_id }))
+                })
+                .collect();
+
+            errors.lock().push(format!(
+                "{}:\n  missing IDs = {:?}\n  present IDs = {:#?}",
+                hir_map.def_path(DefId::local(owner)).to_string_no_crate(),
+                missing,
+                present,
+            ));
+        }
     });
 
     let errors = errors.into_inner();
-
     if !errors.is_empty() {
-        let message = errors
-            .iter()
-            .fold(String::new(), |s1, s2| s1 + "\n" + s2);
-        bug!("{}", message);
-    }
-}
-
-struct HirIdValidator<'a, 'hir> {
-    hir_map: &'a hir::map::Map<'hir>,
-    owner_def_index: Option<DefIndex>,
-    hir_ids_seen: FxHashSet<ItemLocalId>,
-    errors: &'a Lock<Vec<String>>,
-}
-
-struct OuterVisitor<'a, 'hir> {
-    hir_map: &'a hir::map::Map<'hir>,
-    errors: &'a Lock<Vec<String>>,
-}
-
-impl<'a, 'hir> OuterVisitor<'a, 'hir> {
-    fn new_inner_visitor(&self,
-                         hir_map: &'a hir::map::Map<'hir>)
-                         -> HirIdValidator<'a, 'hir> {
-        HirIdValidator {
-            hir_map,
-            owner_def_index: None,
-            hir_ids_seen: Default::default(),
-            errors: self.errors,
-        }
-    }
-}
-
-impl<'a, 'hir> ItemLikeVisitor<'hir> for OuterVisitor<'a, 'hir> {
-    fn visit_item(&mut self, i: &'hir hir::Item) {
-        let mut inner_visitor = self.new_inner_visitor(self.hir_map);
-        inner_visitor.check(i.hir_id, |this| intravisit::walk_item(this, i));
-    }
-
-    fn visit_trait_item(&mut self, i: &'hir hir::TraitItem) {
-        let mut inner_visitor = self.new_inner_visitor(self.hir_map);
-        inner_visitor.check(i.hir_id, |this| intravisit::walk_trait_item(this, i));
-    }
-
-    fn visit_impl_item(&mut self, i: &'hir hir::ImplItem) {
-        let mut inner_visitor = self.new_inner_visitor(self.hir_map);
-        inner_visitor.check(i.hir_id, |this| intravisit::walk_impl_item(this, i));
-    }
-}
-
-impl<'a, 'hir> HirIdValidator<'a, 'hir> {
-    #[cold]
-    #[inline(never)]
-    fn error(&self, f: impl FnOnce() -> String) {
-        self.errors.lock().push(f());
-    }
-
-    fn check<F: FnOnce(&mut HirIdValidator<'a, 'hir>)>(&mut self,
-                                                       hir_id: HirId,
-                                                       walk: F) {
-        assert!(self.owner_def_index.is_none());
-        let owner_def_index = self.hir_map.local_def_id(hir_id).index;
-        self.owner_def_index = Some(owner_def_index);
-        walk(self);
-
-        if owner_def_index == CRATE_DEF_INDEX {
-            return;
-        }
-
-        // There's always at least one entry for the owning item itself
-        let max = self.hir_ids_seen
-                      .iter()
-                      .map(|local_id| local_id.as_usize())
-                      .max()
-                      .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());
-
-            for local_id in missing {
-                let hir_id = HirId {
-                    owner: owner_def_index,
-                    local_id: ItemLocalId::from_u32(local_id),
-                };
-
-                trace!("missing hir id {:#?}", hir_id);
-
-                missing_items.push(format!("[local_id: {}, node:{}]",
-                                           local_id,
-                                           self.hir_map.node_to_string(hir_id)));
-            }
-            self.error(|| format!(
-                "ItemLocalIds not assigned densely in {}. \
-                Max ItemLocalId = {}, missing IDs = {:?}; seens IDs = {:?}",
-                self.hir_map.def_path(DefId::local(owner_def_index)).to_string_no_crate(),
-                max,
-                missing_items,
-                self.hir_ids_seen
-                    .iter()
-                    .map(|&local_id| HirId {
-                        owner: owner_def_index,
-                        local_id,
-                    })
-                    .map(|h| format!("({:?} {})", h, self.hir_map.node_to_string(h)))
-                    .collect::<Vec<_>>()));
-        }
-    }
-}
-
-impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
-
-    fn nested_visit_map<'this>(&'this mut self)
-                               -> intravisit::NestedVisitorMap<'this, 'hir> {
-        intravisit::NestedVisitorMap::OnlyBodies(self.hir_map)
-    }
-
-    fn visit_id(&mut self, hir_id: HirId) {
-        let owner = self.owner_def_index.expect("no owner_def_index");
-
-        if hir_id == hir::DUMMY_HIR_ID {
-            self.error(|| format!("HirIdValidator: HirId {:?} is invalid",
-                                  self.hir_map.node_to_string(hir_id)));
-            return;
-        }
-
-        if owner != hir_id.owner {
-            self.error(|| format!(
-                "HirIdValidator: The recorded owner of {} is {} instead of {}",
-                self.hir_map.node_to_string(hir_id),
-                self.hir_map.def_path(DefId::local(hir_id.owner)).to_string_no_crate(),
-                self.hir_map.def_path(DefId::local(owner)).to_string_no_crate()));
-        }
-
-        self.hir_ids_seen.insert(hir_id.local_id);
-    }
-
-    fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) {
-        // Explicitly do nothing here. ImplItemRefs contain hir::Visibility
-        // values that actually belong to an ImplItem instead of the ItemKind::Impl
-        // we are currently in. So for those it's correct that they have a
-        // different owner.
+        bug!("`ItemLocalId`s not assigned densely in:\n{}", errors.join("\n"));
     }
 }