Skip to content

Commit

Permalink
Auto merge of #93373 - spastorino:def_id_to_hir_id_refactor, r=oli-obk
Browse files Browse the repository at this point in the history
Store def_id_to_hir_id as variant in hir_owner.

If hir_owner is Owner(_), the LocalDefId is pointing to an owner, so the ItemLocalId is 0.
If the HIR node does not exist, we store Phantom.
Otherwise, we store the HirId associated to the LocalDefId.

Related to #89278

r? `@oli-obk`
  • Loading branch information
bors committed Jan 31, 2022
2 parents 86f5e17 + bf1ca2e commit 24b8bb1
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 95 deletions.
17 changes: 11 additions & 6 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> T {
let old_len = self.in_scope_lifetimes.len();

let parent_generics =
match self.owners[parent_hir_id].as_ref().unwrap().node().expect_item().kind {
hir::ItemKind::Impl(hir::Impl { ref generics, .. })
| hir::ItemKind::Trait(_, _, ref generics, ..) => generics.params,
_ => &[],
};
let parent_generics = match self.owners[parent_hir_id].unwrap().node().expect_item().kind {
hir::ItemKind::Impl(hir::Impl { ref generics, .. })
| hir::ItemKind::Trait(_, _, ref generics, ..) => generics.params,
_ => &[],
};
let lt_def_names = parent_generics.iter().filter_map(|param| match param.kind {
hir::GenericParamKind::Lifetime { .. } => Some(param.name.normalize_to_macros_2_0()),
_ => None,
Expand Down Expand Up @@ -480,6 +479,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
.node_id_to_hir_id
.insert(new_node_id, hir::HirId::make_owner(new_id));
debug_assert!(_old.is_none());
self.owners.ensure_contains_elem(new_id, || hir::MaybeOwner::Phantom);
let _old = std::mem::replace(
&mut self.owners[new_id],
hir::MaybeOwner::NonOwner(hir::HirId::make_owner(new_id)),
);
debug_assert!(matches!(_old, hir::MaybeOwner::Phantom));
continue;
};
let ident = *ident;
Expand Down
29 changes: 11 additions & 18 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ struct LoweringContext<'a, 'hir: 'a> {
arena: &'hir Arena<'hir>,

/// The items being lowered are collected here.
owners: IndexVec<LocalDefId, Option<hir::OwnerInfo<'hir>>>,
owners: IndexVec<LocalDefId, hir::MaybeOwner<&'hir hir::OwnerInfo<'hir>>>,
/// Bodies inside the owner being lowered.
bodies: Vec<(hir::ItemLocalId, &'hir hir::Body<'hir>)>,
/// Attributes inside the owner being lowered.
Expand Down Expand Up @@ -291,7 +291,8 @@ pub fn lower_crate<'a, 'hir>(
) -> &'hir hir::Crate<'hir> {
let _prof_timer = sess.prof.verbose_generic_activity("hir_lowering");

let owners = IndexVec::from_fn_n(|_| None, resolver.definitions().def_index_count());
let owners =
IndexVec::from_fn_n(|_| hir::MaybeOwner::Phantom, resolver.definitions().def_index_count());
LoweringContext {
sess,
resolver,
Expand Down Expand Up @@ -402,19 +403,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let hir_hash = self.compute_hir_hash();

let mut def_id_to_hir_id = IndexVec::default();

for (node_id, hir_id) in self.node_id_to_hir_id.into_iter_enumerated() {
if let Some(def_id) = self.resolver.opt_local_def_id(node_id) {
if def_id_to_hir_id.len() <= def_id.index() {
def_id_to_hir_id.resize(def_id.index() + 1, None);
}
def_id_to_hir_id[def_id] = hir_id;
}
}

self.resolver.definitions().init_def_id_to_hir_id_mapping(def_id_to_hir_id);

let krate = hir::Crate { owners: self.owners, hir_hash };
self.arena.alloc(krate)
}
Expand All @@ -427,7 +415,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.owners
.iter_enumerated()
.filter_map(|(def_id, info)| {
let info = info.as_ref()?;
let info = info.as_owner()?;
let def_path_hash = definitions.def_path_hash(def_id);
Some((def_path_hash, info))
})
Expand Down Expand Up @@ -469,8 +457,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.current_hir_id_owner = current_owner;
self.item_local_id_counter = current_local_counter;

let _old = self.owners.insert(def_id, info);
debug_assert!(_old.is_none());
self.owners.ensure_contains_elem(def_id, || hir::MaybeOwner::Phantom);
self.owners[def_id] = hir::MaybeOwner::Owner(self.arena.alloc(info));

def_id
}
Expand All @@ -488,6 +476,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
None
} else {
let def_id = self.resolver.opt_local_def_id(node_id)?;
self.owners.ensure_contains_elem(def_id, || hir::MaybeOwner::Phantom);
if let o @ hir::MaybeOwner::Phantom = &mut self.owners[def_id] {
// Do not override a `MaybeOwner::Owner` that may already here.
*o = hir::MaybeOwner::NonOwner(hir_id);
}
Some((hir_id.local_id, def_id))
}
})
Expand Down
31 changes: 1 addition & 30 deletions compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
pub use crate::def_id::DefPathHash;
use crate::def_id::{CrateNum, DefIndex, LocalDefId, StableCrateId, CRATE_DEF_INDEX, LOCAL_CRATE};
use crate::def_path_hash_map::DefPathHashMap;
use crate::hir;

use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::StableHasher;
Expand Down Expand Up @@ -101,13 +100,6 @@ impl DefPathTable {
pub struct Definitions {
table: DefPathTable,

/// Only [`LocalDefId`]s for items and item-like are HIR owners.
/// The associated `HirId` has a `local_id` of `0`.
/// Generic parameters and closures are also assigned a `LocalDefId` but are not HIR owners.
/// Their `HirId`s are defined by their position while lowering the enclosing owner.
// FIXME(cjgillot) Some `LocalDefId`s from `use` items are dropped during lowering and lack a `HirId`.
pub(super) def_id_to_hir_id: IndexVec<LocalDefId, Option<hir::HirId>>,

/// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`.
expansions_that_defined: FxHashMap<LocalDefId, ExpnId>,

Expand Down Expand Up @@ -322,12 +314,6 @@ impl Definitions {
})
}

#[inline]
#[track_caller]
pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId {
self.def_id_to_hir_id[id].unwrap()
}

/// Adds a root definition (no parent) and a few other reserved definitions.
pub fn new(stable_crate_id: StableCrateId, crate_span: Span) -> Definitions {
let key = DefKey {
Expand All @@ -354,7 +340,6 @@ impl Definitions {

Definitions {
table,
def_id_to_hir_id: Default::default(),
expansions_that_defined: Default::default(),
def_id_to_span,
stable_crate_id,
Expand Down Expand Up @@ -406,20 +391,6 @@ impl Definitions {
def_id
}

/// Initializes the `LocalDefId` to `HirId` mapping once it has been generated during
/// AST to HIR lowering.
pub fn init_def_id_to_hir_id_mapping(
&mut self,
mapping: IndexVec<LocalDefId, Option<hir::HirId>>,
) {
assert!(
self.def_id_to_hir_id.is_empty(),
"trying to initialize `LocalDefId` <-> `HirId` mappings twice"
);

self.def_id_to_hir_id = mapping;
}

pub fn expansion_that_defined(&self, id: LocalDefId) -> ExpnId {
self.expansions_that_defined.get(&id).copied().unwrap_or_else(ExpnId::root)
}
Expand All @@ -431,7 +402,7 @@ impl Definitions {
}

pub fn iter_local_def_id(&self) -> impl Iterator<Item = LocalDefId> + '_ {
self.def_id_to_hir_id.iter_enumerated().map(|(k, _)| k)
self.table.def_path_hashes.indices().map(|local_def_index| LocalDefId { local_def_index })
}

#[inline(always)]
Expand Down
48 changes: 43 additions & 5 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,15 @@ pub struct OwnerNodes<'tcx> {
pub local_id_to_def_id: SortedMap<ItemLocalId, LocalDefId>,
}

impl<'tcx> OwnerNodes<'tcx> {
pub fn node(&self) -> OwnerNode<'tcx> {
use rustc_index::vec::Idx;
let node = self.nodes[ItemLocalId::new(0)].as_ref().unwrap().node;
let node = node.as_owner().unwrap(); // Indexing must ensure it is an OwnerNode.
node
}
}

/// Full information resulting from lowering an AST node.
#[derive(Debug, HashStable_Generic)]
pub struct OwnerInfo<'hir> {
Expand All @@ -728,10 +737,39 @@ pub struct OwnerInfo<'hir> {
impl<'tcx> OwnerInfo<'tcx> {
#[inline]
pub fn node(&self) -> OwnerNode<'tcx> {
use rustc_index::vec::Idx;
let node = self.nodes.nodes[ItemLocalId::new(0)].as_ref().unwrap().node;
let node = node.as_owner().unwrap(); // Indexing must ensure it is an OwnerNode.
node
self.nodes.node()
}
}

#[derive(Copy, Clone, Debug, HashStable_Generic)]
pub enum MaybeOwner<T> {
Owner(T),
NonOwner(HirId),
/// Used as a placeholder for unused LocalDefId.
Phantom,
}

impl<T> MaybeOwner<T> {
pub fn as_owner(self) -> Option<T> {
match self {
MaybeOwner::Owner(i) => Some(i),
MaybeOwner::NonOwner(_) | MaybeOwner::Phantom => None,
}
}

pub fn map<U>(self, f: impl FnOnce(T) -> U) -> MaybeOwner<U> {
match self {
MaybeOwner::Owner(i) => MaybeOwner::Owner(f(i)),
MaybeOwner::NonOwner(hir_id) => MaybeOwner::NonOwner(hir_id),
MaybeOwner::Phantom => MaybeOwner::Phantom,
}
}

pub fn unwrap(self) -> T {
match self {
MaybeOwner::Owner(i) => i,
MaybeOwner::NonOwner(_) | MaybeOwner::Phantom => panic!("Not a HIR owner"),
}
}
}

Expand All @@ -743,7 +781,7 @@ impl<'tcx> OwnerInfo<'tcx> {
/// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/hir.html
#[derive(Debug)]
pub struct Crate<'hir> {
pub owners: IndexVec<LocalDefId, Option<OwnerInfo<'hir>>>,
pub owners: IndexVec<LocalDefId, MaybeOwner<&'hir OwnerInfo<'hir>>>,
pub hir_hash: Fingerprint,
}

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_hir/src/hir_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ impl HirId {
if self.local_id.index() == 0 { Some(self.owner) } else { None }
}

#[inline]
pub fn is_owner(self) -> bool {
self.local_id.index() == 0
}

#[inline]
pub fn make_owner(owner: LocalDefId) -> Self {
Self { owner, local_id: ItemLocalId::from_u32(0) }
Expand Down
36 changes: 19 additions & 17 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl<'hir> Map<'hir> {

pub fn items(&self) -> impl Iterator<Item = &'hir Item<'hir>> + 'hir {
let krate = self.krate();
krate.owners.iter().filter_map(|owner| match owner.as_ref()?.node() {
krate.owners.iter().filter_map(|owner| match owner.as_owner()?.node() {
OwnerNode::Item(item) => Some(item),
_ => None,
})
Expand Down Expand Up @@ -205,7 +205,8 @@ impl<'hir> Map<'hir> {
Some(hir_id.owner)
} else {
self.tcx
.hir_owner_nodes(hir_id.owner)?
.hir_owner_nodes(hir_id.owner)
.as_owner()?
.local_id_to_def_id
.get(&hir_id.local_id)
.copied()
Expand All @@ -214,8 +215,7 @@ impl<'hir> Map<'hir> {

#[inline]
pub fn local_def_id_to_hir_id(&self, def_id: LocalDefId) -> HirId {
// FIXME(#85914) is this access safe for incr. comp.?
self.tcx.untracked_resolutions.definitions.local_def_id_to_hir_id(def_id)
self.tcx.local_def_id_to_hir_id(def_id)
}

pub fn iter_local_def_id(&self) -> impl Iterator<Item = LocalDefId> + '_ {
Expand Down Expand Up @@ -321,7 +321,7 @@ impl<'hir> Map<'hir> {
if id.local_id == ItemLocalId::from_u32(0) {
Some(self.tcx.hir_owner_parent(id.owner))
} else {
let owner = self.tcx.hir_owner_nodes(id.owner)?;
let owner = self.tcx.hir_owner_nodes(id.owner).as_owner()?;
let node = owner.nodes[id.local_id].as_ref()?;
let hir_id = HirId { owner: id.owner, local_id: node.parent };
Some(hir_id)
Expand All @@ -338,7 +338,7 @@ impl<'hir> Map<'hir> {
let owner = self.tcx.hir_owner(id.owner)?;
Some(owner.node.into())
} else {
let owner = self.tcx.hir_owner_nodes(id.owner)?;
let owner = self.tcx.hir_owner_nodes(id.owner).as_owner()?;
let node = owner.nodes[id.local_id].as_ref()?;
Some(node.node)
}
Expand Down Expand Up @@ -522,7 +522,7 @@ impl<'hir> Map<'hir> {
.owners
.iter_enumerated()
.flat_map(move |(owner, owner_info)| {
let bodies = &owner_info.as_ref()?.nodes.bodies;
let bodies = &owner_info.as_owner()?.nodes.bodies;
Some(bodies.iter().map(move |&(local_id, _)| {
let hir_id = HirId { owner, local_id };
let body_id = BodyId { hir_id };
Expand All @@ -539,7 +539,7 @@ impl<'hir> Map<'hir> {

par_iter(&self.krate().owners.raw).enumerate().for_each(|(owner, owner_info)| {
let owner = LocalDefId::new(owner);
if let Some(owner_info) = owner_info {
if let MaybeOwner::Owner(owner_info) = owner_info {
par_iter(owner_info.nodes.bodies.range(..)).for_each(|(local_id, _)| {
let hir_id = HirId { owner, local_id: *local_id };
let body_id = BodyId { hir_id };
Expand Down Expand Up @@ -601,7 +601,7 @@ impl<'hir> Map<'hir> {
pub fn walk_attributes(self, visitor: &mut impl Visitor<'hir>) {
let krate = self.krate();
for (owner, info) in krate.owners.iter_enumerated() {
if let Some(info) = info {
if let MaybeOwner::Owner(info) = info {
for (local_id, attrs) in info.attrs.map.iter() {
let id = HirId { owner, local_id: *local_id };
for a in *attrs {
Expand All @@ -625,7 +625,7 @@ impl<'hir> Map<'hir> {
V: itemlikevisit::ItemLikeVisitor<'hir>,
{
let krate = self.krate();
for owner in krate.owners.iter().filter_map(Option::as_ref) {
for owner in krate.owners.iter().filter_map(|i| i.as_owner()) {
match owner.node() {
OwnerNode::Item(item) => visitor.visit_item(item),
OwnerNode::ForeignItem(item) => visitor.visit_foreign_item(item),
Expand All @@ -642,12 +642,14 @@ impl<'hir> Map<'hir> {
V: itemlikevisit::ParItemLikeVisitor<'hir> + Sync + Send,
{
let krate = self.krate();
par_for_each_in(&krate.owners.raw, |owner| match owner.as_ref().map(OwnerInfo::node) {
Some(OwnerNode::Item(item)) => visitor.visit_item(item),
Some(OwnerNode::ForeignItem(item)) => visitor.visit_foreign_item(item),
Some(OwnerNode::ImplItem(item)) => visitor.visit_impl_item(item),
Some(OwnerNode::TraitItem(item)) => visitor.visit_trait_item(item),
Some(OwnerNode::Crate(_)) | None => {}
par_for_each_in(&krate.owners.raw, |owner| match owner.map(OwnerInfo::node) {
MaybeOwner::Owner(OwnerNode::Item(item)) => visitor.visit_item(item),
MaybeOwner::Owner(OwnerNode::ForeignItem(item)) => visitor.visit_foreign_item(item),
MaybeOwner::Owner(OwnerNode::ImplItem(item)) => visitor.visit_impl_item(item),
MaybeOwner::Owner(OwnerNode::TraitItem(item)) => visitor.visit_trait_item(item),
MaybeOwner::Owner(OwnerNode::Crate(_))
| MaybeOwner::NonOwner(_)
| MaybeOwner::Phantom => {}
})
}

Expand Down Expand Up @@ -1121,7 +1123,7 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
.owners
.iter_enumerated()
.filter_map(|(def_id, info)| {
let _ = info.as_ref()?;
let _ = info.as_owner()?;
let def_path_hash = definitions.def_path_hash(def_id);
let span = definitions.def_span(def_id);
debug_assert_eq!(span.parent(), None);
Expand Down

0 comments on commit 24b8bb1

Please sign in to comment.