Auto merge of #59068 - ljedrz:kill_off_NodeId_stragglers, r=Zoxc

HirIdification: kill off NodeId stragglers

The final stages of HirIdification (#57578).

This PR, along with #59042, should finalize the HirIdification process (at least the more straightforward bits).

- replace `NodeId` with `HirId` in `trait_impls`
- remove all `NodeId`s from `borrowck`
- remove all `NodeId`s from `typeck`
- remove all `NodeId`s from `mir`
- remove `trait_auto_impl` (unused)

I would be cool to also remove `NodeId` from `hir::def::Def`, `middle::privacy::AccessLevel`  and `hir::ItemId`, but I don't know if this is feasible.

I'll be happy to do more if I've missed anything.

src/librustc/hir/lowering.rs

@@ -89,8 +89,7 @@ pub struct LoweringContext<'a> {
     bodies: BTreeMap<hir::BodyId, hir::Body>,
     exported_macros: Vec<hir::MacroDef>,
 
-    trait_impls: BTreeMap<DefId, Vec<NodeId>>,
-    trait_auto_impl: BTreeMap<DefId, NodeId>,
+    trait_impls: BTreeMap<DefId, Vec<hir::HirId>>,
 
     modules: BTreeMap<NodeId, hir::ModuleItems>,
 
@@ -233,7 +232,6 @@ pub fn lower_crate(
         impl_items: BTreeMap::new(),
         bodies: BTreeMap::new(),
         trait_impls: BTreeMap::new(),
-        trait_auto_impl: BTreeMap::new(),
         modules: BTreeMap::new(),
         exported_macros: Vec::new(),
         catch_scopes: Vec::new(),
@@ -514,7 +512,6 @@ impl<'a> LoweringContext<'a> {
             bodies: self.bodies,
             body_ids,
             trait_impls: self.trait_impls,
-            trait_auto_impl: self.trait_auto_impl,
             modules: self.modules,
         }
     }
@@ -2968,6 +2965,7 @@ impl<'a> LoweringContext<'a> {
                 // method, it will not be considered an in-band
                 // lifetime to be added, but rather a reference to a
                 // parent lifetime.
+                let lowered_trait_impl_id = self.lower_node_id(id).hir_id;
                 let (generics, (trait_ref, lowered_ty)) = self.add_in_band_defs(
                     ast_generics,
                     def_id,
@@ -2979,7 +2977,8 @@ impl<'a> LoweringContext<'a> {
 
                         if let Some(ref trait_ref) = trait_ref {
                             if let Def::Trait(def_id) = trait_ref.path.def {
-                                this.trait_impls.entry(def_id).or_default().push(id);
+                                this.trait_impls.entry(def_id).or_default().push(
+                                    lowered_trait_impl_id);
                             }
                         }
 

src/librustc/hir/map/collector.rs

@@ -121,7 +121,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
                 impl_items: _,
                 bodies: _,
                 trait_impls: _,
-                trait_auto_impl: _,
                 body_ids: _,
                 modules: _,
             } = *krate;

src/librustc/hir/map/mod.rs

@@ -557,26 +557,14 @@ impl<'hir> Map<'hir> {
         }
     }
 
-    pub fn trait_impls(&self, trait_did: DefId) -> &'hir [NodeId] {
+    pub fn trait_impls(&self, trait_did: DefId) -> &'hir [HirId] {
         self.dep_graph.read(DepNode::new_no_params(DepKind::AllLocalTraitImpls));
 
         // N.B., intentionally bypass `self.forest.krate()` so that we
         // do not trigger a read of the whole krate here
         self.forest.krate.trait_impls.get(&trait_did).map_or(&[], |xs| &xs[..])
     }
 
-    pub fn trait_auto_impl(&self, trait_did: DefId) -> Option<NodeId> {
-        self.dep_graph.read(DepNode::new_no_params(DepKind::AllLocalTraitImpls));
-
-        // N.B., intentionally bypass `self.forest.krate()` so that we
-        // do not trigger a read of the whole krate here
-        self.forest.krate.trait_auto_impl.get(&trait_did).cloned()
-    }
-
-    pub fn trait_is_auto(&self, trait_did: DefId) -> bool {
-        self.trait_auto_impl(trait_did).is_some()
-    }
-
     /// Gets the attributes on the crate. This is preferable to
     /// invoking `krate.attrs` because it registers a tighter
     /// dep-graph access.

src/librustc/hir/mod.rs

@@ -727,8 +727,7 @@ pub struct Crate {
     pub trait_items: BTreeMap<TraitItemId, TraitItem>,
     pub impl_items: BTreeMap<ImplItemId, ImplItem>,
     pub bodies: BTreeMap<BodyId, Body>,
-    pub trait_impls: BTreeMap<DefId, Vec<NodeId>>,
-    pub trait_auto_impl: BTreeMap<DefId, NodeId>,
+    pub trait_impls: BTreeMap<DefId, Vec<HirId>>,
 
     /// A list of the body ids written out in the order in which they
     /// appear in the crate. If you're going to process all the bodies

src/librustc/ty/trait_def.rs

@@ -183,8 +183,8 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             }
         }
 
-        for &node_id in tcx.hir().trait_impls(trait_id) {
-            add_impl(tcx.hir().local_def_id(node_id));
+        for &hir_id in tcx.hir().trait_impls(trait_id) {
+            add_impl(tcx.hir().local_def_id_from_hir_id(hir_id));
         }
     }
 

src/librustc_borrowck/borrowck/gather_loans/lifetime.rs

@@ -2,13 +2,13 @@
 //! does not exceed the lifetime of the value being borrowed.
 
 use crate::borrowck::*;
+use rustc::hir::HirId;
 use rustc::middle::expr_use_visitor as euv;
 use rustc::middle::mem_categorization as mc;
 use rustc::middle::mem_categorization::Categorization;
 use rustc::middle::region;
 use rustc::ty;
 
-use syntax::ast;
 use syntax_pos::Span;
 use log::debug;
 
@@ -51,7 +51,7 @@ struct GuaranteeLifetimeContext<'a, 'tcx: 'a> {
 }
 
 impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
-    fn check(&self, cmt: &mc::cmt_<'tcx>, discr_scope: Option<ast::NodeId>) -> R {
+    fn check(&self, cmt: &mc::cmt_<'tcx>, discr_scope: Option<HirId>) -> R {
         //! Main routine. Walks down `cmt` until we find the
         //! "guarantor". Reports an error if `self.loan_region` is
         //! larger than scope of `cmt`.

src/librustc_borrowck/borrowck/gather_loans/mod.rs

@@ -14,7 +14,6 @@ use rustc::middle::mem_categorization::Categorization;
 use rustc::middle::region;
 use rustc::ty::{self, TyCtxt};
 
-use syntax::ast;
 use syntax_pos::Span;
 use rustc::hir;
 use log::debug;
@@ -141,8 +140,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> {
               assignee_cmt: &mc::cmt_<'tcx>,
               _: euv::MutateMode)
     {
-        let node_id = self.bccx.tcx.hir().hir_to_node_id(assignment_id);
-        self.guarantee_assignment_valid(node_id,
+        self.guarantee_assignment_valid(assignment_id,
                                         assignment_span,
                                         assignee_cmt);
     }
@@ -256,7 +254,7 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
 
     /// Guarantees that `cmt` is assignable, or reports an error.
     fn guarantee_assignment_valid(&mut self,
-                                  assignment_id: ast::NodeId,
+                                  assignment_id: hir::HirId,
                                   assignment_span: Span,
                                   cmt: &mc::cmt_<'tcx>) {
 
@@ -290,8 +288,7 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
                     self.mark_loan_path_as_mutated(&lp);
                 }
                 gather_moves::gather_assignment(self.bccx, &self.move_data,
-                                                self.bccx.tcx.hir().node_to_hir_id(assignment_id)
-                                                    .local_id,
+                                                assignment_id.local_id,
                                                 assignment_span,
                                                 lp);
             }

src/librustc_borrowck/borrowck/mod.rs

@@ -34,7 +34,6 @@ use std::fmt;
 use std::rc::Rc;
 use rustc_data_structures::sync::Lrc;
 use std::hash::{Hash, Hasher};
-use syntax::ast;
 use syntax_pos::{MultiSpan, Span};
 use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
 use log::debug;
@@ -399,12 +398,12 @@ pub enum LoanPathElem<'tcx> {
 }
 
 fn closure_to_block(closure_id: LocalDefId,
-                    tcx: TyCtxt<'_, '_, '_>) -> ast::NodeId {
+                    tcx: TyCtxt<'_, '_, '_>) -> HirId {
     let closure_id = tcx.hir().local_def_id_to_node_id(closure_id);
     match tcx.hir().get(closure_id) {
         Node::Expr(expr) => match expr.node {
             hir::ExprKind::Closure(.., body_id, _, _) => {
-                tcx.hir().hir_to_node_id(body_id.hir_id)
+                body_id.hir_id
             }
             _ => {
                 bug!("encountered non-closure id: {}", closure_id)
@@ -422,8 +421,7 @@ impl<'a, 'tcx> LoanPath<'tcx> {
             }
             LpUpvar(upvar_id) => {
                 let block_id = closure_to_block(upvar_id.closure_expr_id, bccx.tcx);
-                let hir_id = bccx.tcx.hir().node_to_hir_id(block_id);
-                region::Scope { id: hir_id.local_id, data: region::ScopeData::Node }
+                region::Scope { id: block_id.local_id, data: region::ScopeData::Node }
             }
             LpDowncast(ref base, _) |
             LpExtend(ref base, ..) => base.kill_scope(bccx),

src/librustc_mir/borrow_check/nll/universal_regions.rs

@@ -23,7 +23,6 @@ use rustc::util::nodemap::FxHashMap;
 use rustc_data_structures::indexed_vec::{Idx, IndexVec};
 use rustc_errors::DiagnosticBuilder;
 use std::iter;
-use syntax::ast;
 
 use super::ToRegionVid;
 
@@ -200,12 +199,10 @@ impl<'tcx> UniversalRegions<'tcx> {
         param_env: ty::ParamEnv<'tcx>,
     ) -> Self {
         let tcx = infcx.tcx;
-        let mir_node_id = tcx.hir().as_local_node_id(mir_def_id).unwrap();
-        let mir_hir_id = tcx.hir().node_to_hir_id(mir_node_id);
+        let mir_hir_id = tcx.hir().as_local_hir_id(mir_def_id).unwrap();
         UniversalRegionsBuilder {
             infcx,
             mir_def_id,
-            mir_node_id,
             mir_hir_id,
             param_env,
         }.build()
@@ -370,7 +367,6 @@ struct UniversalRegionsBuilder<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
     infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>,
     mir_def_id: DefId,
     mir_hir_id: HirId,
-    mir_node_id: ast::NodeId,
     param_env: ty::ParamEnv<'tcx>,
 }
 
@@ -475,7 +471,7 @@ impl<'cx, 'gcx, 'tcx> UniversalRegionsBuilder<'cx, 'gcx, 'tcx> {
         let tcx = self.infcx.tcx;
         let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id);
 
-        match tcx.hir().body_owner_kind(self.mir_node_id) {
+        match tcx.hir().body_owner_kind_by_hir_id(self.mir_hir_id) {
             BodyOwnerKind::Closure |
             BodyOwnerKind::Fn => {
                 let defining_ty = if self.mir_def_id == closure_base_def_id {

src/librustc_mir/build/mod.rs

@@ -373,7 +373,7 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     /// finish building it.
     guard_context: Vec<GuardFrame>,
 
-    /// Maps `NodeId`s of variable bindings to the `Local`s created for them.
+    /// Maps `HirId`s of variable bindings to the `Local`s created for them.
     /// (A match binding can have two locals; the 2nd is for the arm's guard.)
     var_indices: HirIdMap<LocalsForNode>,
     local_decls: IndexVec<Local, LocalDecl<'tcx>>,
@@ -451,7 +451,7 @@ impl BlockContext {
 
 #[derive(Debug)]
 enum LocalsForNode {
-    /// In the usual case, a `NodeId` for an identifier maps to at most
+    /// In the usual case, a `HirId` for an identifier maps to at most
     /// one `Local` declaration.
     One(Local),
 

src/librustc_typeck/check/method/mod.rs

@@ -196,7 +196,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         )?;
 
         if let Some(import_id) = pick.import_id {
-            let import_def_id = self.tcx.hir().local_def_id(import_id);
+            let import_def_id = self.tcx.hir().local_def_id_from_hir_id(import_id);
             debug!("used_trait_import: {:?}", import_def_id);
             Lrc::get_mut(&mut self.tables.borrow_mut().used_trait_imports)
                 .unwrap().insert(import_def_id);
@@ -428,7 +428,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                                        self_ty, expr_id, ProbeScope::TraitsInScope)?;
         debug!("resolve_ufcs: pick={:?}", pick);
         if let Some(import_id) = pick.import_id {
-            let import_def_id = tcx.hir().local_def_id(import_id);
+            let import_def_id = tcx.hir().local_def_id_from_hir_id(import_id);
             debug!("resolve_ufcs: used_trait_import: {:?}", import_def_id);
             Lrc::get_mut(&mut self.tables.borrow_mut().used_trait_imports)
                 .unwrap().insert(import_def_id);

src/librustc_typeck/check/method/probe.rs

@@ -120,7 +120,7 @@ struct Candidate<'tcx> {
     xform_ret_ty: Option<Ty<'tcx>>,
     item: ty::AssociatedItem,
     kind: CandidateKind<'tcx>,
-    import_id: Option<ast::NodeId>,
+    import_id: Option<hir::HirId>,
 }
 
 #[derive(Debug)]
@@ -145,7 +145,7 @@ enum ProbeResult {
 pub struct Pick<'tcx> {
     pub item: ty::AssociatedItem,
     pub kind: PickKind<'tcx>,
-    pub import_id: Option<ast::NodeId>,
+    pub import_id: Option<hir::HirId>,
 
     // Indicates that the source expression should be autoderef'd N times
     //
@@ -836,7 +836,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
             for trait_candidate in applicable_traits.iter() {
                 let trait_did = trait_candidate.def_id;
                 if duplicates.insert(trait_did) {
-                    let import_id = trait_candidate.import_id;
+                    let import_id = trait_candidate.import_id.map(|node_id|
+                        self.fcx.tcx.hir().node_to_hir_id(node_id));
                     let result = self.assemble_extension_candidates_for_trait(import_id, trait_did);
                     result?;
                 }
@@ -887,7 +888,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
     }
 
     fn assemble_extension_candidates_for_trait(&mut self,
-                                               import_id: Option<ast::NodeId>,
+                                               import_id: Option<hir::HirId>,
                                                trait_def_id: DefId)
                                                -> Result<(), MethodError<'tcx>> {
         debug!("assemble_extension_candidates_for_trait(trait_def_id={:?})",

src/librustc_typeck/check/mod.rs

@@ -5016,10 +5016,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                     // that highlight errors inline.
                     let mut sp = blk.span;
                     let mut fn_span = None;
-                    let blk_node_id = self.tcx.hir().hir_to_node_id(blk.hir_id);
-                    if let Some((decl, ident)) = self.get_parent_fn_decl(blk_node_id) {
+                    if let Some((decl, ident)) = self.get_parent_fn_decl(blk.hir_id) {
                         let ret_sp = decl.output.span();
-                        if let Some(block_sp) = self.parent_item_span(blk_node_id) {
+                        if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
                             // HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
                             // output would otherwise be incorrect and even misleading. Make sure
                             // the span we're aiming at correspond to a `fn` body.
@@ -5059,8 +5058,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         ty
     }
 
-    fn parent_item_span(&self, id: ast::NodeId) -> Option<Span> {
-        let node = self.tcx.hir().get(self.tcx.hir().get_parent(id));
+    fn parent_item_span(&self, id: hir::HirId) -> Option<Span> {
+        let node = self.tcx.hir().get_by_hir_id(self.tcx.hir().get_parent_item(id));
         match node {
             Node::Item(&hir::Item {
                 node: hir::ItemKind::Fn(_, _, _, body_id), ..
@@ -5078,9 +5077,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         None
     }
 
-    /// Given a function block's `NodeId`, returns its `FnDecl` if it exists, or `None` otherwise.
-    fn get_parent_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, ast::Ident)> {
-        let parent = self.tcx.hir().get(self.tcx.hir().get_parent(blk_id));
+    /// Given a function block's `HirId`, returns its `FnDecl` if it exists, or `None` otherwise.
+    fn get_parent_fn_decl(&self, blk_id: hir::HirId) -> Option<(hir::FnDecl, ast::Ident)> {
+        let parent = self.tcx.hir().get_by_hir_id(self.tcx.hir().get_parent_item(blk_id));
         self.get_node_fn_decl(parent).map(|(fn_decl, ident, _)| (fn_decl, ident))
     }
 

src/librustc_typeck/check/writeback.rs

@@ -15,7 +15,6 @@ use rustc::ty::{self, Ty, TyCtxt};
 use rustc::util::nodemap::DefIdSet;
 use rustc_data_structures::sync::Lrc;
 use std::mem;
-use syntax::ast;
 use syntax_pos::Span;
 
 ///////////////////////////////////////////////////////////////////////////
@@ -444,8 +443,8 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
 
     fn visit_opaque_types(&mut self, span: Span) {
         for (&def_id, opaque_defn) in self.fcx.opaque_types.borrow().iter() {
-            let node_id = self.tcx().hir().as_local_node_id(def_id).unwrap();
-            let instantiated_ty = self.resolve(&opaque_defn.concrete_ty, &node_id);
+            let hir_id = self.tcx().hir().as_local_hir_id(def_id).unwrap();
+            let instantiated_ty = self.resolve(&opaque_defn.concrete_ty, &hir_id);
 
             let generics = self.tcx().generics_of(def_id);
 
@@ -731,12 +730,6 @@ impl Locatable for Span {
     }
 }
 
-impl Locatable for ast::NodeId {
-    fn to_span(&self, tcx: &TyCtxt<'_, '_, '_>) -> Span {
-        tcx.hir().span(*self)
-    }
-}
-
 impl Locatable for DefIndex {
     fn to_span(&self, tcx: &TyCtxt<'_, '_, '_>) -> Span {
         let hir_id = tcx.hir().def_index_to_hir_id(*self);

src/librustc_typeck/coherence/builtin.rs

@@ -37,7 +37,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
     {
         if Some(self.trait_def_id) == trait_def_id {
             for &impl_id in self.tcx.hir().trait_impls(self.trait_def_id) {
-                let impl_def_id = self.tcx.hir().local_def_id(impl_id);
+                let impl_def_id = self.tcx.hir().local_def_id_from_hir_id(impl_id);
                 f(self.tcx, impl_def_id);
             }
         }