From 8b3fb35e7fb48cdde954de6578c9e4f653b7d55c Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 22 Jun 2020 11:31:04 +0100 Subject: [PATCH 1/5] Revert "Pre-compute `def_id_to_hir_id` table" This reverts commit 13104ef1c5c7e8ebd973b07b33ac3794fc8fed59. --- src/librustc_hir/definitions.rs | 37 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index 5755a3db92ac1..5e03395c6933e 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -81,12 +81,13 @@ pub struct Definitions { def_id_to_span: IndexVec, + // FIXME(eddyb) don't go through `ast::NodeId` to convert between `HirId` + // and `LocalDefId` - ideally all `LocalDefId`s would be HIR owners. node_id_to_def_id: FxHashMap, def_id_to_node_id: IndexVec, - // FIXME(eddyb) ideally all `LocalDefId`s would be HIR owners. - pub(super) def_id_to_hir_id: IndexVec>, - /// The reverse mapping of `def_id_to_hir_id`. + pub(super) node_id_to_hir_id: IndexVec>, + /// The pre-computed mapping of `hir_id_to_node_id` -> `node_id_to_def_id`. pub(super) hir_id_to_def_id: FxHashMap, /// If `ExpnId` is an ID of some macro expansion, @@ -326,7 +327,9 @@ impl Definitions { #[inline] pub fn local_def_id(&self, node: ast::NodeId) -> LocalDefId { - self.opt_local_def_id(node).unwrap_or_else(|| panic!("no entry for node id: `{:?}`", node)) + self.opt_local_def_id(node).unwrap_or_else(|| { + panic!("no entry for node id: `{:?}` / `{:?}`", node, self.node_id_to_hir_id.get(node)) + }) } #[inline] @@ -336,12 +339,14 @@ impl Definitions { #[inline] pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId { - self.def_id_to_hir_id[id].unwrap() + let node_id = self.def_id_to_node_id[id]; + self.node_id_to_hir_id[node_id].unwrap() } #[inline] pub fn opt_local_def_id_to_hir_id(&self, id: LocalDefId) -> Option { - self.def_id_to_hir_id[id] + let node_id = self.def_id_to_node_id[id]; + self.node_id_to_hir_id[node_id] } #[inline] @@ -456,20 +461,16 @@ impl Definitions { mapping: IndexVec>, ) { assert!( - self.def_id_to_hir_id.is_empty(), - "trying to initialize `LocalDefId` <-> `HirId` mappings twice" + self.node_id_to_hir_id.is_empty(), + "trying to initialize `NodeId` -> `HirId` mapping twice" ); + self.node_id_to_hir_id = mapping; - self.def_id_to_hir_id = self - .def_id_to_node_id - .iter() - .map(|&node_id| mapping.get(node_id).and_then(|&hir_id| hir_id)) - .collect(); - - // Build the reverse mapping of `def_id_to_hir_id`. - self.hir_id_to_def_id = mapping - .into_iter_enumerated() - .filter_map(|(node_id, hir_id)| { + // Build the pre-computed mapping of `hir_id_to_node_id` -> `node_id_to_def_id`. + self.hir_id_to_def_id = self + .node_id_to_hir_id + .iter_enumerated() + .filter_map(|(node_id, &hir_id)| { hir_id.and_then(|hir_id| { self.node_id_to_def_id.get(&node_id).map(|&def_id| (hir_id, def_id)) }) From 3c94dd15dced6b9dae5b3626097b607d3e157e78 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 22 Jun 2020 11:31:09 +0100 Subject: [PATCH 2/5] Revert "Pre-compute `hir_id_to_def_id` mapping" This reverts commit 94817e38e14b89747c6d6d5af0d45267fcbf765e. --- src/librustc_hir/definitions.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index 5e03395c6933e..9913f92d2d98e 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -87,8 +87,8 @@ pub struct Definitions { def_id_to_node_id: IndexVec, pub(super) node_id_to_hir_id: IndexVec>, - /// The pre-computed mapping of `hir_id_to_node_id` -> `node_id_to_def_id`. - pub(super) hir_id_to_def_id: FxHashMap, + /// The reverse mapping of `node_id_to_hir_id`. + pub(super) hir_id_to_node_id: FxHashMap, /// If `ExpnId` is an ID of some macro expansion, /// then `DefId` is the normal module (`mod`) in which the expanded macro was defined. @@ -351,7 +351,8 @@ impl Definitions { #[inline] pub fn opt_hir_id_to_local_def_id(&self, hir_id: hir::HirId) -> Option { - self.hir_id_to_def_id.get(&hir_id).copied() + let node_id = self.hir_id_to_node_id[&hir_id]; + self.opt_local_def_id(node_id) } /// Retrieves the span of the given `DefId` if `DefId` is in the local crate. @@ -466,15 +467,11 @@ impl Definitions { ); self.node_id_to_hir_id = mapping; - // Build the pre-computed mapping of `hir_id_to_node_id` -> `node_id_to_def_id`. - self.hir_id_to_def_id = self + // Build the reverse mapping of `node_id_to_hir_id`. + self.hir_id_to_node_id = self .node_id_to_hir_id .iter_enumerated() - .filter_map(|(node_id, &hir_id)| { - hir_id.and_then(|hir_id| { - self.node_id_to_def_id.get(&node_id).map(|&def_id| (hir_id, def_id)) - }) - }) + .filter_map(|(node_id, &hir_id)| hir_id.map(|hir_id| (hir_id, node_id))) .collect(); } From a95a0b9d2ec13d115a85c36a118f869b9a396e49 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 22 Jun 2020 11:31:10 +0100 Subject: [PATCH 3/5] Revert "Remove `HirId` to `NodeId` conversion APIs" This reverts commit 6a0f1af19d504635ea36efdf4ea68b1df935fc1b. --- src/librustc_hir/definitions.rs | 7 ++++++- src/librustc_middle/hir/map/collector.rs | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index 9913f92d2d98e..679fd1668dde1 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -337,6 +337,11 @@ impl Definitions { self.local_def_id_to_hir_id(def_id) } + #[inline] + pub fn hir_id_to_node_id(&self, hir_id: hir::HirId) -> ast::NodeId { + self.hir_id_to_node_id[&hir_id] + } + #[inline] pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId { let node_id = self.def_id_to_node_id[id]; @@ -351,7 +356,7 @@ impl Definitions { #[inline] pub fn opt_hir_id_to_local_def_id(&self, hir_id: hir::HirId) -> Option { - let node_id = self.hir_id_to_node_id[&hir_id]; + let node_id = self.hir_id_to_node_id(hir_id); self.opt_local_def_id(node_id) } diff --git a/src/librustc_middle/hir/map/collector.rs b/src/librustc_middle/hir/map/collector.rs index dce06a5f7eeec..ae169976698cc 100644 --- a/src/librustc_middle/hir/map/collector.rs +++ b/src/librustc_middle/hir/map/collector.rs @@ -242,8 +242,10 @@ 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 node_id = self.definitions.hir_id_to_node_id(hir_id); + if hir_id.owner != self.current_dep_node_owner { - let node_str = match self.definitions.opt_hir_id_to_local_def_id(hir_id) { + let node_str = match self.definitions.opt_local_def_id(node_id) { Some(def_id) => self.definitions.def_path(def_id).to_string_no_crate(), None => format!("{:?}", node), }; @@ -333,7 +335,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { debug!("visit_item: {:?}", i); debug_assert_eq!( i.hir_id.owner, - self.definitions.opt_hir_id_to_local_def_id(i.hir_id).unwrap() + self.definitions + .opt_local_def_id(self.definitions.hir_id_to_node_id(i.hir_id)) + .unwrap() ); self.with_dep_node_owner(i.hir_id.owner, i, |this, hash| { this.insert_with_hash(i.span, i.hir_id, Node::Item(i), hash); @@ -365,7 +369,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { fn visit_trait_item(&mut self, ti: &'hir TraitItem<'hir>) { debug_assert_eq!( ti.hir_id.owner, - self.definitions.opt_hir_id_to_local_def_id(ti.hir_id).unwrap() + self.definitions + .opt_local_def_id(self.definitions.hir_id_to_node_id(ti.hir_id)) + .unwrap() ); self.with_dep_node_owner(ti.hir_id.owner, ti, |this, hash| { this.insert_with_hash(ti.span, ti.hir_id, Node::TraitItem(ti), hash); @@ -379,7 +385,9 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { fn visit_impl_item(&mut self, ii: &'hir ImplItem<'hir>) { debug_assert_eq!( ii.hir_id.owner, - self.definitions.opt_hir_id_to_local_def_id(ii.hir_id).unwrap() + self.definitions + .opt_local_def_id(self.definitions.hir_id_to_node_id(ii.hir_id)) + .unwrap() ); self.with_dep_node_owner(ii.hir_id.owner, ii, |this, hash| { this.insert_with_hash(ii.span, ii.hir_id, Node::ImplItem(ii), hash); From 879ac1dd7f8b513680b067d6551da2ca579551db Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 22 Jun 2020 11:31:11 +0100 Subject: [PATCH 4/5] Revert "Remove `NodeId` to `HirId` conversion APIs" This reverts commit a98f35f503ad4df9c6083d01447700c57436a0bc. --- src/librustc_hir/definitions.rs | 12 +++++++++++- src/librustc_middle/hir/map/collector.rs | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index 679fd1668dde1..b63dd653c4dd8 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -328,7 +328,7 @@ impl Definitions { #[inline] pub fn local_def_id(&self, node: ast::NodeId) -> LocalDefId { self.opt_local_def_id(node).unwrap_or_else(|| { - panic!("no entry for node id: `{:?}` / `{:?}`", node, self.node_id_to_hir_id.get(node)) + panic!("no entry for node id: `{:?}` / `{:?}`", node, self.opt_node_id_to_hir_id(node)) }) } @@ -342,6 +342,16 @@ impl Definitions { self.hir_id_to_node_id[&hir_id] } + #[inline] + pub fn node_id_to_hir_id(&self, node_id: ast::NodeId) -> hir::HirId { + self.node_id_to_hir_id[node_id].unwrap() + } + + #[inline] + pub fn opt_node_id_to_hir_id(&self, node_id: ast::NodeId) -> Option { + self.node_id_to_hir_id[node_id] + } + #[inline] pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId { let node_id = self.def_id_to_node_id[id]; diff --git a/src/librustc_middle/hir/map/collector.rs b/src/librustc_middle/hir/map/collector.rs index ae169976698cc..2b3c21daa4635 100644 --- a/src/librustc_middle/hir/map/collector.rs +++ b/src/librustc_middle/hir/map/collector.rs @@ -243,6 +243,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { // owner of that node. if cfg!(debug_assertions) { let node_id = self.definitions.hir_id_to_node_id(hir_id); + assert_eq!(self.definitions.node_id_to_hir_id(node_id), hir_id); if hir_id.owner != self.current_dep_node_owner { let node_str = match self.definitions.opt_local_def_id(node_id) { From c8905eca91260724a9b716b5afc9b7639f8cc08b Mon Sep 17 00:00:00 2001 From: marmeladema Date: Mon, 22 Jun 2020 11:31:12 +0100 Subject: [PATCH 5/5] Revert "Move `trait_map` into `hir::Crate`" This reverts commit 936b6bfa646c90b0533cc33bbbc03d890d0780f1. --- src/librustc_ast_lowering/lib.rs | 10 ---------- src/librustc_hir/hir.rs | 4 +--- src/librustc_middle/hir/map/collector.rs | 1 - src/librustc_middle/ty/context.rs | 4 ++-- src/librustc_middle/ty/mod.rs | 1 + src/librustc_resolve/lib.rs | 15 +++++++++++---- 6 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 335cc3e61040d..af1e33a049bfb 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -203,8 +203,6 @@ pub trait Resolver { fn lint_buffer(&mut self) -> &mut LintBuffer; fn next_node_id(&mut self) -> NodeId; - - fn trait_map(&self) -> &NodeMap>; } type NtToTokenstream = fn(&Nonterminal, &ParseSess, Span) -> TokenStream; @@ -557,13 +555,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let proc_macros = c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id].unwrap()).collect(); - let trait_map = self - .resolver - .trait_map() - .iter() - .map(|(&k, v)| (self.node_id_to_hir_id[k].unwrap(), v.clone())) - .collect(); - self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id); hir::Crate { @@ -578,7 +569,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { trait_impls: self.trait_impls, modules: self.modules, proc_macros, - trait_map, } } diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 7d1cb7738c35e..eb1db46fd2bda 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -639,8 +639,6 @@ pub struct Crate<'hir> { /// A list of proc macro HirIds, written out in the order in which /// they are declared in the static array generated by proc_macro_harness. pub proc_macros: Vec, - - pub trait_map: BTreeMap>, } impl Crate<'hir> { @@ -2653,7 +2651,7 @@ pub type CaptureModeMap = NodeMap; // The TraitCandidate's import_ids is empty if the trait is defined in the same module, and // has length > 0 if the trait is found through an chain of imports, starting with the // import/use statement in the scope where the trait is used. -#[derive(RustcEncodable, RustcDecodable, Clone, Debug)] +#[derive(Clone, Debug)] pub struct TraitCandidate { pub def_id: DefId, pub import_ids: SmallVec<[LocalDefId; 1]>, diff --git a/src/librustc_middle/hir/map/collector.rs b/src/librustc_middle/hir/map/collector.rs index 2b3c21daa4635..2906da437abac 100644 --- a/src/librustc_middle/hir/map/collector.rs +++ b/src/librustc_middle/hir/map/collector.rs @@ -117,7 +117,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { body_ids: _, modules: _, proc_macros: _, - trait_map: _, } = *krate; hash_body(&mut hcx, root_mod_def_path_hash, item, &mut hir_body_nodes) diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index 56f4ae9e9848b..77d27e4d31357 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -1120,9 +1120,9 @@ impl<'tcx> TyCtxt<'tcx> { }; let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default(); - for (hir_id, v) in krate.trait_map.iter() { + for (hir_id, v) in resolutions.trait_map.into_iter() { let map = trait_map.entry(hir_id.owner).or_default(); - map.insert(hir_id.local_id, StableVec::new(v.to_vec())); + map.insert(hir_id.local_id, StableVec::new(v)); } GlobalCtxt { diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 6b7940ed7abcc..89713d42fe154 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -121,6 +121,7 @@ pub struct ResolverOutputs { pub definitions: rustc_hir::definitions::Definitions, pub cstore: Box, pub extern_crate_map: FxHashMap, + pub trait_map: FxHashMap>, pub maybe_unused_trait_imports: FxHashSet, pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>, pub export_map: ExportMap, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 91bd155614178..66e5612b627b4 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1109,10 +1109,6 @@ impl rustc_ast_lowering::Resolver for Resolver<'_> { fn next_node_id(&mut self) -> NodeId { self.next_node_id() } - - fn trait_map(&self) -> &NodeMap> { - &self.trait_map - } } impl<'a> Resolver<'a> { @@ -1288,6 +1284,11 @@ impl<'a> Resolver<'a> { let definitions = self.definitions; let extern_crate_map = self.extern_crate_map; let export_map = self.export_map; + let trait_map = self + .trait_map + .into_iter() + .map(|(k, v)| (definitions.node_id_to_hir_id(k), v)) + .collect(); let maybe_unused_trait_imports = self.maybe_unused_trait_imports; let maybe_unused_extern_crates = self.maybe_unused_extern_crates; let glob_map = self.glob_map; @@ -1296,6 +1297,7 @@ impl<'a> Resolver<'a> { cstore: Box::new(self.crate_loader.into_cstore()), extern_crate_map, export_map, + trait_map, glob_map, maybe_unused_trait_imports, maybe_unused_extern_crates, @@ -1313,6 +1315,11 @@ impl<'a> Resolver<'a> { cstore: Box::new(self.cstore().clone()), extern_crate_map: self.extern_crate_map.clone(), export_map: self.export_map.clone(), + trait_map: self + .trait_map + .iter() + .map(|(&k, v)| (self.definitions.node_id_to_hir_id(k), v.clone())) + .collect(), glob_map: self.glob_map.clone(), maybe_unused_trait_imports: self.maybe_unused_trait_imports.clone(), maybe_unused_extern_crates: self.maybe_unused_extern_crates.clone(),