hir: HirId-ify intravisit

Author: ljedrz

src/librustc/hir/intravisit.rs

@@ -3301,7 +3301,9 @@ impl<'a> LoweringContext<'a> {
         let mut path = path.clone();
         for seg in path.segments.iter_mut() {
             if seg.id.is_some() {
-                seg.id = Some(self.next_id().node_id);
+                let next_id = self.next_id();
+                seg.id = Some(next_id.node_id);
+                seg.hir_id = Some(next_id.hir_id);
             }
         }
         path

src/librustc/hir/lowering.rs

@@ -228,7 +228,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
         self.map[id.as_usize()] = Some(entry);
     }
 
-    fn insert(&mut self, span: Span, id: NodeId, node: Node<'hir>) {
+    // FIXME(ljedrz): devise a way to get rid of this NodeId
+    fn insert(&mut self, span: Span, node_id: NodeId, hir_id: HirId, node: Node<'hir>) {
         let entry = Entry {
             parent: self.parent_node,
             parent_hir: self.parent_hir,
@@ -243,18 +244,18 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
         // Make sure that the DepNode of some node coincides with the HirId
         // owner of that node.
         if cfg!(debug_assertions) {
-            let hir_id = self.definitions.node_to_hir_id(id);
+           assert_eq!(self.definitions.node_to_hir_id(node_id), hir_id);
 
             if hir_id.owner != self.current_dep_node_owner {
-                let node_str = match self.definitions.opt_def_index(id) {
+                let node_str = match self.definitions.opt_def_index(node_id) {
                     Some(def_index) => {
                         self.definitions.def_path(def_index).to_string_no_crate()
                     }
                     None => format!("{:?}", node)
                 };
 
                 let forgot_str = if hir_id == crate::hir::DUMMY_HIR_ID {
-                    format!("\nMaybe you forgot to lower the node id {:?}?", id)
+                    format!("\nMaybe you forgot to lower the node id {:?}?", node_id)
                 } else {
                     String::new()
                 };
@@ -276,13 +277,21 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
             }
         }
 
-        self.insert_entry(id, entry);
+        self.insert_entry(node_id, entry);
     }
 
-    fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_id: NodeId, f: F) {
+    fn with_parent<F: FnOnce(&mut Self)>(
+        &mut self,
+        parent_node_id: NodeId,
+        parent_hir_id: HirId,
+        f: F,
+    ) {
         let parent_node = self.parent_node;
-        self.parent_node = parent_id;
+        self.parent_node = parent_node_id;
+        let parent_hir = self.parent_hir;
+        self.parent_hir = parent_hir_id;
         f(self);
+        self.parent_hir = parent_hir;
         self.parent_node = parent_node;
     }
 
@@ -352,12 +361,13 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         debug_assert_eq!(i.hir_id.owner,
                          self.definitions.opt_def_index(i.id).unwrap());
         self.with_dep_node_owner(i.hir_id.owner, i, |this| {
-            this.insert(i.span, i.id, Node::Item(i));
-            this.with_parent(i.id, |this| {
+            this.insert(i.span, i.id, i.hir_id, Node::Item(i));
+            this.with_parent(i.id, i.hir_id, |this| {
                 if let ItemKind::Struct(ref struct_def, _) = i.node {
                     // If this is a tuple-like struct, register the constructor.
                     if !struct_def.is_struct() {
-                        this.insert(i.span, struct_def.id(), Node::StructCtor(struct_def));
+                        this.insert(i.span, struct_def.id(), struct_def.hir_id(),
+                            Node::StructCtor(struct_def));
                     }
                 }
                 intravisit::walk_item(this, i);
@@ -366,25 +376,26 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     }
 
     fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem) {
-        self.insert(foreign_item.span, foreign_item.id, Node::ForeignItem(foreign_item));
+        self.insert(foreign_item.span, foreign_item.id, foreign_item.hir_id,
+            Node::ForeignItem(foreign_item));
 
-        self.with_parent(foreign_item.id, |this| {
+        self.with_parent(foreign_item.id, foreign_item.hir_id, |this| {
             intravisit::walk_foreign_item(this, foreign_item);
         });
     }
 
     fn visit_generic_param(&mut self, param: &'hir GenericParam) {
-        self.insert(param.span, param.id, Node::GenericParam(param));
+        self.insert(param.span, param.id, param.hir_id, Node::GenericParam(param));
         intravisit::walk_generic_param(self, param);
     }
 
     fn visit_trait_item(&mut self, ti: &'hir TraitItem) {
         debug_assert_eq!(ti.hir_id.owner,
                          self.definitions.opt_def_index(ti.id).unwrap());
         self.with_dep_node_owner(ti.hir_id.owner, ti, |this| {
-            this.insert(ti.span, ti.id, Node::TraitItem(ti));
+            this.insert(ti.span, ti.id, ti.hir_id, Node::TraitItem(ti));
 
-            this.with_parent(ti.id, |this| {
+            this.with_parent(ti.id, ti.hir_id, |this| {
                 intravisit::walk_trait_item(this, ti);
             });
         });
@@ -394,9 +405,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         debug_assert_eq!(ii.hir_id.owner,
                          self.definitions.opt_def_index(ii.id).unwrap());
         self.with_dep_node_owner(ii.hir_id.owner, ii, |this| {
-            this.insert(ii.span, ii.id, Node::ImplItem(ii));
+            this.insert(ii.span, ii.id, ii.hir_id, Node::ImplItem(ii));
 
-            this.with_parent(ii.id, |this| {
+            this.with_parent(ii.id, ii.hir_id, |this| {
                 intravisit::walk_impl_item(this, ii);
             });
         });
@@ -408,93 +419,93 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         } else {
             Node::Pat(pat)
         };
-        self.insert(pat.span, pat.id, node);
+        self.insert(pat.span, pat.id, pat.hir_id, node);
 
-        self.with_parent(pat.id, |this| {
+        self.with_parent(pat.id, pat.hir_id, |this| {
             intravisit::walk_pat(this, pat);
         });
     }
 
     fn visit_anon_const(&mut self, constant: &'hir AnonConst) {
-        self.insert(DUMMY_SP, constant.id, Node::AnonConst(constant));
+        self.insert(DUMMY_SP, constant.id, constant.hir_id, Node::AnonConst(constant));
 
-        self.with_parent(constant.id, |this| {
+        self.with_parent(constant.id, constant.hir_id, |this| {
             intravisit::walk_anon_const(this, constant);
         });
     }
 
     fn visit_expr(&mut self, expr: &'hir Expr) {
-        self.insert(expr.span, expr.id, Node::Expr(expr));
+        self.insert(expr.span, expr.id, expr.hir_id, Node::Expr(expr));
 
-        self.with_parent(expr.id, |this| {
+        self.with_parent(expr.id, expr.hir_id, |this| {
             intravisit::walk_expr(this, expr);
         });
     }
 
     fn visit_stmt(&mut self, stmt: &'hir Stmt) {
-        let id = stmt.id;
-        self.insert(stmt.span, id, Node::Stmt(stmt));
+        self.insert(stmt.span, stmt.id, stmt.hir_id, Node::Stmt(stmt));
 
-        self.with_parent(id, |this| {
+        self.with_parent(stmt.id, stmt.hir_id, |this| {
             intravisit::walk_stmt(this, stmt);
         });
     }
 
     fn visit_path_segment(&mut self, path_span: Span, path_segment: &'hir PathSegment) {
-        if let Some(id) = path_segment.id {
-            self.insert(path_span, id, Node::PathSegment(path_segment));
+        if let Some(node_id) = path_segment.id {
+            let hir_id = path_segment.hir_id.unwrap();
+            self.insert(path_span, node_id, hir_id, Node::PathSegment(path_segment));
         }
         intravisit::walk_path_segment(self, path_span, path_segment);
     }
 
     fn visit_ty(&mut self, ty: &'hir Ty) {
-        self.insert(ty.span, ty.id, Node::Ty(ty));
+        self.insert(ty.span, ty.id, ty.hir_id, Node::Ty(ty));
 
-        self.with_parent(ty.id, |this| {
+        self.with_parent(ty.id, ty.hir_id, |this| {
             intravisit::walk_ty(this, ty);
         });
     }
 
     fn visit_trait_ref(&mut self, tr: &'hir TraitRef) {
-        self.insert(tr.path.span, tr.ref_id, Node::TraitRef(tr));
+        self.insert(tr.path.span, tr.ref_id, tr.hir_ref_id, Node::TraitRef(tr));
 
-        self.with_parent(tr.ref_id, |this| {
+        self.with_parent(tr.ref_id, tr.hir_ref_id, |this| {
             intravisit::walk_trait_ref(this, tr);
         });
     }
 
     fn visit_fn(&mut self, fk: intravisit::FnKind<'hir>, fd: &'hir FnDecl,
-                b: BodyId, s: Span, id: NodeId) {
-        assert_eq!(self.parent_node, id);
+                b: BodyId, s: Span, id: HirId) {
+        assert_eq!(self.parent_hir, id);
         intravisit::walk_fn(self, fk, fd, b, s, id);
     }
 
     fn visit_block(&mut self, block: &'hir Block) {
-        self.insert(block.span, block.id, Node::Block(block));
-        self.with_parent(block.id, |this| {
+        self.insert(block.span, block.id, block.hir_id, Node::Block(block));
+        self.with_parent(block.id, block.hir_id, |this| {
             intravisit::walk_block(this, block);
         });
     }
 
     fn visit_local(&mut self, l: &'hir Local) {
-        self.insert(l.span, l.id, Node::Local(l));
-        self.with_parent(l.id, |this| {
+        self.insert(l.span, l.id, l.hir_id, Node::Local(l));
+        self.with_parent(l.id, l.hir_id, |this| {
             intravisit::walk_local(this, l)
         })
     }
 
     fn visit_lifetime(&mut self, lifetime: &'hir Lifetime) {
-        self.insert(lifetime.span, lifetime.id, Node::Lifetime(lifetime));
+        self.insert(lifetime.span, lifetime.id, lifetime.hir_id, Node::Lifetime(lifetime));
     }
 
     fn visit_vis(&mut self, visibility: &'hir Visibility) {
         match visibility.node {
             VisibilityKind::Public |
             VisibilityKind::Crate(_) |
             VisibilityKind::Inherited => {}
-            VisibilityKind::Restricted { id, .. } => {
-                self.insert(visibility.span, id, Node::Visibility(visibility));
-                self.with_parent(id, |this| {
+            VisibilityKind::Restricted { id, hir_id, .. } => {
+                self.insert(visibility.span, id, hir_id, Node::Visibility(visibility));
+                self.with_parent(id, hir_id, |this| {
                     intravisit::walk_vis(this, visibility);
                 });
             }
@@ -505,21 +516,20 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         let def_index = self.definitions.opt_def_index(macro_def.id).unwrap();
 
         self.with_dep_node_owner(def_index, macro_def, |this| {
-            this.insert(macro_def.span, macro_def.id, Node::MacroDef(macro_def));
+            this.insert(macro_def.span, macro_def.id, macro_def.hir_id, Node::MacroDef(macro_def));
         });
     }
 
-    fn visit_variant(&mut self, v: &'hir Variant, g: &'hir Generics, item_id: NodeId) {
-        let id = v.node.data.id();
-        self.insert(v.span, id, Node::Variant(v));
-        self.with_parent(id, |this| {
+    fn visit_variant(&mut self, v: &'hir Variant, g: &'hir Generics, item_id: HirId) {
+        self.insert(v.span, v.node.data.id(), v.node.data.hir_id(), Node::Variant(v));
+        self.with_parent(v.node.data.id(), v.node.data.hir_id(), |this| {
             intravisit::walk_variant(this, v, g, item_id);
         });
     }
 
     fn visit_struct_field(&mut self, field: &'hir StructField) {
-        self.insert(field.span, field.id, Node::Field(field));
-        self.with_parent(field.id, |this| {
+        self.insert(field.span, field.id, field.hir_id, Node::Field(field));
+        self.with_parent(field.id, field.hir_id, |this| {
             intravisit::walk_struct_field(this, field);
         });
     }

src/librustc/hir/map/collector.rs

@@ -55,17 +55,17 @@ impl<'a, 'hir: 'a> OuterVisitor<'a, 'hir> {
 impl<'a, 'hir: 'a> 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.id, |this| intravisit::walk_item(this, i));
+        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.id, |this| intravisit::walk_trait_item(this, i));
+        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.id, |this| intravisit::walk_impl_item(this, i));
+        inner_visitor.check(i.hir_id, |this| intravisit::walk_impl_item(this, i));
     }
 }
 
@@ -77,10 +77,10 @@ impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> {
     }
 
     fn check<F: FnOnce(&mut HirIdValidator<'a, 'hir>)>(&mut self,
-                                                       node_id: NodeId,
+                                                       hir_id: HirId,
                                                        walk: F) {
         assert!(self.owner_def_index.is_none());
-        let owner_def_index = self.hir_map.local_def_id(node_id).index;
+        let owner_def_index = self.hir_map.local_def_id_from_hir_id(hir_id).index;
         self.owner_def_index = Some(owner_def_index);
         walk(self);
 
@@ -147,35 +147,26 @@ impl<'a, 'hir: 'a> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
         intravisit::NestedVisitorMap::OnlyBodies(self.hir_map)
     }
 
-    fn visit_id(&mut self, node_id: NodeId) {
+    fn visit_id(&mut self, hir_id: HirId) {
         let owner = self.owner_def_index.expect("no owner_def_index");
-        let stable_id = self.hir_map.definitions().node_to_hir_id[node_id];
 
-        if stable_id == hir::DUMMY_HIR_ID {
-            self.error(|| format!("HirIdValidator: No HirId assigned for NodeId {}: {:?}",
-                                     node_id,
-                                     self.hir_map.node_to_string(node_id)));
+        if hir_id == hir::DUMMY_HIR_ID {
+            self.error(|| format!("HirIdValidator: HirId {:?} is invalid",
+                                  self.hir_map.hir_to_string(hir_id)));
             return;
         }
 
-        if owner != stable_id.owner {
+        if owner != hir_id.owner {
             self.error(|| format!(
                 "HirIdValidator: The recorded owner of {} is {} instead of {}",
-                self.hir_map.node_to_string(node_id),
-                self.hir_map.def_path(DefId::local(stable_id.owner)).to_string_no_crate(),
+                self.hir_map.hir_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()));
         }
 
-        if let Some(prev) = self.hir_ids_seen.insert(stable_id.local_id, node_id) {
-            if prev != node_id {
-                self.error(|| format!(
-                    "HirIdValidator: Same HirId {}/{} assigned for nodes {} and {}",
-                    self.hir_map.def_path(DefId::local(stable_id.owner)).to_string_no_crate(),
-                    stable_id.local_id.as_usize(),
-                    self.hir_map.node_to_string(prev),
-                    self.hir_map.node_to_string(node_id)));
-            }
-        }
+        let node_id = self.hir_map.hir_to_node_id(hir_id);
+
+        self.hir_ids_seen.insert(hir_id.local_id, node_id);
     }
 
     fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) {