From 78b53db2868eaad41ac884989f7b470f352c65b2 Mon Sep 17 00:00:00 2001 From: Fabian Drinck Date: Sat, 27 Apr 2019 19:23:00 +0200 Subject: [PATCH 1/3] Remove obsolete comment --- src/librustc/hir/map/mod.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 9c895198ddde9..fbdb0e07ac1c7 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -174,15 +174,6 @@ pub struct Map<'hir> { /// The SVH of the local crate. pub crate_hash: Svh, - /// `NodeId`s are sequential integers from 0, so we can be - /// super-compact by storing them in a vector. Not everything with - /// a `NodeId` is in the map, but empirically the occupancy is about - /// 75-80%, so there's not too much overhead (certainly less than - /// a hashmap, since they (at the time of writing) have a maximum - /// of 75% occupancy). - /// - /// Also, indexing is pretty quick when you've got a vector and - /// plain old integers. map: FxHashMap>, definitions: &'hir Definitions, From fa7582d1dd37d0bdb08033bcb810274084bc26ca Mon Sep 17 00:00:00 2001 From: Fabian Drinck Date: Sat, 27 Apr 2019 19:24:45 +0200 Subject: [PATCH 2/3] Replace `NodeId` variant of `read` --- src/librustc/hir/map/mod.rs | 30 +++++++++++--------------- src/librustc_metadata/index_builder.rs | 4 ++-- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index fbdb0e07ac1c7..56c14faef2302 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -190,13 +190,7 @@ impl<'hir> Map<'hir> { /// otherwise have had access to those contents, and hence needs a /// read recorded). If the function just returns a DefId or /// NodeId, no actual content was returned, so no read is needed. - pub fn read(&self, id: NodeId) { - let hir_id = self.node_to_hir_id(id); - self.read_by_hir_id(hir_id); - } - - // FIXME(@ljedrz): replace the NodeId variant - pub fn read_by_hir_id(&self, hir_id: HirId) { + pub fn read(&self, hir_id: HirId) { if let Some(entry) = self.map.get(&hir_id) { self.dep_graph.read_index(entry.dep_node); } else { @@ -402,7 +396,7 @@ impl<'hir> Map<'hir> { } pub fn trait_item(&self, id: TraitItemId) -> &'hir TraitItem { - self.read_by_hir_id(id.hir_id); + self.read(id.hir_id); // N.B., intentionally bypass `self.forest.krate()` so that we // do not trigger a read of the whole krate here @@ -410,7 +404,7 @@ impl<'hir> Map<'hir> { } pub fn impl_item(&self, id: ImplItemId) -> &'hir ImplItem { - self.read_by_hir_id(id.hir_id); + self.read(id.hir_id); // N.B., intentionally bypass `self.forest.krate()` so that we // do not trigger a read of the whole krate here @@ -418,7 +412,7 @@ impl<'hir> Map<'hir> { } pub fn body(&self, id: BodyId) -> &'hir Body { - self.read_by_hir_id(id.hir_id); + self.read(id.hir_id); // N.B., intentionally bypass `self.forest.krate()` so that we // do not trigger a read of the whole krate here @@ -551,7 +545,7 @@ impl<'hir> Map<'hir> { pub fn get_module(&self, module: DefId) -> (&'hir Mod, Span, HirId) { let hir_id = self.as_local_hir_id(module).unwrap(); - self.read_by_hir_id(hir_id); + self.read(hir_id); match self.find_entry(hir_id).unwrap().node { Node::Item(&Item { span, @@ -566,13 +560,15 @@ impl<'hir> Map<'hir> { pub fn visit_item_likes_in_module(&self, module: DefId, visitor: &mut V) where V: ItemLikeVisitor<'hir> { - let node_id = self.as_local_node_id(module).unwrap(); + let hir_id = self.as_local_hir_id(module).unwrap(); // Read the module so we'll be re-executed if new items // appear immediately under in the module. If some new item appears // in some nested item in the module, we'll be re-executed due to reads // in the expect_* calls the loops below - self.read(node_id); + self.read(hir_id); + + let node_id = self.hir_to_node_id[&hir_id]; let module = &self.forest.krate.modules[&node_id]; @@ -650,7 +646,7 @@ impl<'hir> Map<'hir> { } }); if result.is_some() { - self.read_by_hir_id(hir_id); + self.read(hir_id); } result } @@ -884,7 +880,7 @@ impl<'hir> Map<'hir> { if let Entry { node: Node::Item(Item { node: ItemKind::ForeignMod(ref nm), .. }), .. } = entry { - self.read_by_hir_id(hir_id); // reveals some of the content of a node + self.read(hir_id); // reveals some of the content of a node return nm.abi; } } @@ -992,7 +988,7 @@ impl<'hir> Map<'hir> { // FIXME(@ljedrz): replace the NodeId variant pub fn attrs_by_hir_id(&self, id: HirId) -> &'hir [ast::Attribute] { - self.read_by_hir_id(id); // reveals attributes on the node + self.read(id); // reveals attributes on the node let attrs = match self.find_entry(id).map(|entry| entry.node) { Some(Node::Local(l)) => Some(&l.attrs[..]), Some(Node::Item(i)) => Some(&i.attrs[..]), @@ -1037,7 +1033,7 @@ impl<'hir> Map<'hir> { // FIXME(@ljedrz): replace the NodeId variant pub fn span_by_hir_id(&self, hir_id: HirId) -> Span { - self.read_by_hir_id(hir_id); // reveals span from node + self.read(hir_id); // reveals span from node match self.find_entry(hir_id).map(|entry| entry.node) { Some(Node::Item(item)) => item.span, Some(Node::ForeignItem(foreign_item)) => foreign_item.span, diff --git a/src/librustc_metadata/index_builder.rs b/src/librustc_metadata/index_builder.rs index 8343171b99f4b..b77feeee06f57 100644 --- a/src/librustc_metadata/index_builder.rs +++ b/src/librustc_metadata/index_builder.rs @@ -185,7 +185,7 @@ macro_rules! read_hir { ($t:ty) => { impl<'tcx> DepGraphRead for &'tcx $t { fn read(&self, tcx: TyCtxt<'_, '_, '_>) { - tcx.hir().read_by_hir_id(self.hir_id); + tcx.hir().read(self.hir_id); } } } @@ -219,6 +219,6 @@ pub struct FromId(pub hir::HirId, pub T); impl DepGraphRead for FromId { fn read(&self, tcx: TyCtxt<'_, '_, '_>) { - tcx.hir().read_by_hir_id(self.0); + tcx.hir().read(self.0); } } From 164988c1297c4c28a8f38561cbf1481c25a1c5b8 Mon Sep 17 00:00:00 2001 From: Fabian Drinck Date: Sat, 27 Apr 2019 22:39:17 +0200 Subject: [PATCH 3/3] Remove `def_path_from_id`, `node_id_to_string` --- src/librustc/hir/map/mod.rs | 51 ++++++++++++++--------------------- src/librustc_driver/pretty.rs | 5 +++- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 56c14faef2302..a59170e96669c 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -208,17 +208,12 @@ impl<'hir> Map<'hir> { self.definitions.def_key(def_id.index) } - pub fn def_path_from_id(&self, id: NodeId) -> Option { - self.opt_local_def_id(id).map(|def_id| { + pub fn def_path_from_hir_id(&self, id: HirId) -> Option { + self.opt_local_def_id_from_hir_id(id).map(|def_id| { self.def_path(def_id) }) } - // FIXME(@ljedrz): replace the NodeId variant - pub fn def_path_from_hir_id(&self, id: HirId) -> DefPath { - self.def_path(self.local_def_id_from_hir_id(id)) - } - pub fn def_path(&self, def_id: DefId) -> DefPath { assert!(def_id.is_local()); self.definitions.def_path(def_id.index) @@ -1075,7 +1070,7 @@ impl<'hir> Map<'hir> { } pub fn node_to_string(&self, id: NodeId) -> String { - node_id_to_string(self, id, true) + hir_id_to_string(self, self.node_to_hir_id(id), true) } // FIXME(@ljedrz): replace the NodeId variant @@ -1084,7 +1079,7 @@ impl<'hir> Map<'hir> { } pub fn node_to_user_string(&self, id: NodeId) -> String { - node_id_to_string(self, id, false) + hir_id_to_string(self, self.node_to_hir_id(id), false) } // FIXME(@ljedrz): replace the NodeId variant @@ -1303,8 +1298,8 @@ impl<'a> print::State<'a> { } } -fn node_id_to_string(map: &Map<'_>, id: NodeId, include_id: bool) -> String { - let id_str = format!(" (id={})", id); +fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String { + let id_str = format!(" (hir_id={})", id); let id_str = if include_id { &id_str[..] } else { "" }; let path_str = || { @@ -1312,9 +1307,9 @@ fn node_id_to_string(map: &Map<'_>, id: NodeId, include_id: bool) -> String { // the user-friendly path, otherwise fall back to stringifying DefPath. crate::ty::tls::with_opt(|tcx| { if let Some(tcx) = tcx { - let def_id = map.local_def_id(id); + let def_id = map.local_def_id_from_hir_id(id); tcx.def_path_str(def_id) - } else if let Some(path) = map.def_path_from_id(id) { + } else if let Some(path) = map.def_path_from_hir_id(id) { path.data.into_iter().map(|elem| { elem.data.to_string() }).collect::>().join("::") @@ -1324,7 +1319,7 @@ fn node_id_to_string(map: &Map<'_>, id: NodeId, include_id: bool) -> String { }) }; - match map.find(id) { + match map.find_by_hir_id(id) { Some(Node::Item(item)) => { let item_str = match item.node { ItemKind::ExternCrate(..) => "extern crate", @@ -1385,40 +1380,40 @@ fn node_id_to_string(map: &Map<'_>, id: NodeId, include_id: bool) -> String { path_str(), id_str) } Some(Node::AnonConst(_)) => { - format!("const {}{}", map.node_to_pretty_string(id), id_str) + format!("const {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Expr(_)) => { - format!("expr {}{}", map.node_to_pretty_string(id), id_str) + format!("expr {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Stmt(_)) => { - format!("stmt {}{}", map.node_to_pretty_string(id), id_str) + format!("stmt {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::PathSegment(_)) => { - format!("path segment {}{}", map.node_to_pretty_string(id), id_str) + format!("path segment {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Ty(_)) => { - format!("type {}{}", map.node_to_pretty_string(id), id_str) + format!("type {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::TraitRef(_)) => { - format!("trait_ref {}{}", map.node_to_pretty_string(id), id_str) + format!("trait_ref {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Binding(_)) => { - format!("local {}{}", map.node_to_pretty_string(id), id_str) + format!("local {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Pat(_)) => { - format!("pat {}{}", map.node_to_pretty_string(id), id_str) + format!("pat {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Block(_)) => { - format!("block {}{}", map.node_to_pretty_string(id), id_str) + format!("block {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Local(_)) => { - format!("local {}{}", map.node_to_pretty_string(id), id_str) + format!("local {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::Ctor(..)) => { format!("ctor {}{}", path_str(), id_str) } Some(Node::Lifetime(_)) => { - format!("lifetime {}{}", map.node_to_pretty_string(id), id_str) + format!("lifetime {}{}", map.hir_to_pretty_string(id), id_str) } Some(Node::GenericParam(ref param)) => { format!("generic_param {:?}{}", param, id_str) @@ -1434,12 +1429,6 @@ fn node_id_to_string(map: &Map<'_>, id: NodeId, include_id: bool) -> String { } } -// FIXME(@ljedrz): replace the NodeId variant -fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String { - let node_id = map.hir_to_node_id(id); - node_id_to_string(map, node_id, include_id) -} - pub fn def_kind(tcx: TyCtxt<'_, '_, '_>, def_id: DefId) -> Option { if let Some(node_id) = tcx.hir().as_local_node_id(def_id) { tcx.hir().def_kind(node_id) diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index 5cefc35607db0..4fdcdafcab86c 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -255,7 +255,10 @@ trait HirPrinterSupport<'hir>: pprust_hir::PpAnn { /// Computes an user-readable representation of a path, if possible. fn node_path(&self, id: ast::NodeId) -> Option { - self.hir_map().and_then(|map| map.def_path_from_id(id)).map(|path| { + self.hir_map().and_then(|map| { + let hir_id = map.node_to_hir_id(id); + map.def_path_from_hir_id(hir_id) + }).map(|path| { path.data .into_iter() .map(|elem| elem.data.to_string())