Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove LocalDefId <-> HirId global maps #89278

Closed
wants to merge 3 commits into from
Closed

Remove LocalDefId <-> HirId global maps #89278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a0ed1f
Store def_id_to_hir_id as variant in hir_owner.
cjgillot Sep 22, 2021
70ad73b
Make node_id_to_hir_id owner-local.
cjgillot Jul 18, 2021
6805235
Only the query once for `find_by_def_id`.
cjgillot Jan 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
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 @@ -476,10 +475,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
res
} else {
// Associate an HirId to both ids even if there is no resolution.
let _old = self
.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
104 changes: 51 additions & 53 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use rustc_ast::{self as ast, *};
use rustc_ast_pretty::pprust;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sorted_map::SortedMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::Lrc;
Expand All @@ -65,6 +65,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{Span, DUMMY_SP};

use smallvec::SmallVec;
use std::collections::hash_map::Entry;
use tracing::{debug, trace};

macro_rules! arena_vec {
Expand Down Expand Up @@ -98,7 +99,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 @@ -152,10 +153,9 @@ struct LoweringContext<'a, 'hir: 'a> {

current_hir_id_owner: LocalDefId,
item_local_id_counter: hir::ItemLocalId,
node_id_to_hir_id: IndexVec<NodeId, Option<hir::HirId>>,

/// NodeIds that are lowered inside the current HIR owner.
local_node_ids: Vec<NodeId>,
node_id_to_local_id: FxHashMap<NodeId, hir::ItemLocalId>,

allow_try_trait: Option<Lrc<[Symbol]>>,
allow_gen_future: Option<Lrc<[Symbol]>>,
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 All @@ -308,8 +309,7 @@ pub fn lower_crate<'a, 'hir>(
anonymous_lifetime_mode: AnonymousLifetimeMode::PassThrough,
current_hir_id_owner: CRATE_DEF_ID,
item_local_id_counter: hir::ItemLocalId::new(0),
node_id_to_hir_id: IndexVec::new(),
local_node_ids: Vec::new(),
node_id_to_local_id: FxHashMap::default(),
generator_kind: None,
task_context: None,
current_item: None,
Expand Down Expand Up @@ -402,19 +402,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 +414,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 All @@ -449,56 +436,57 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let current_attrs = std::mem::take(&mut self.attrs);
let current_bodies = std::mem::take(&mut self.bodies);
let current_node_ids = std::mem::take(&mut self.local_node_ids);
let current_node_ids = std::mem::take(&mut self.node_id_to_local_id);
let current_owner = std::mem::replace(&mut self.current_hir_id_owner, def_id);
let current_local_counter =
std::mem::replace(&mut self.item_local_id_counter, hir::ItemLocalId::new(1));

// Always allocate the first `HirId` for the owner itself.
let _old = self.node_id_to_hir_id.insert(owner, hir::HirId::make_owner(def_id));
let _old = self.node_id_to_local_id.insert(owner, hir::ItemLocalId::new(0));
debug_assert_eq!(_old, None);
self.local_node_ids.push(owner);

let item = f(self);
debug_assert_eq!(def_id, item.def_id());
let info = self.make_owner_info(item);

self.attrs = current_attrs;
self.bodies = current_bodies;
self.local_node_ids = current_node_ids;
self.node_id_to_local_id = current_node_ids;
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
}

fn make_owner_info(&mut self, node: hir::OwnerNode<'hir>) -> hir::OwnerInfo<'hir> {
let attrs = std::mem::take(&mut self.attrs);
let mut bodies = std::mem::take(&mut self.bodies);
let local_node_ids = std::mem::take(&mut self.local_node_ids);
let node_id_to_local_id = std::mem::take(&mut self.node_id_to_local_id);

let local_id_to_def_id = local_node_ids
let local_id_to_def_id = node_id_to_local_id
.iter()
.filter_map(|&node_id| {
let hir_id = self.node_id_to_hir_id[node_id]?;
if hir_id.local_id == hir::ItemLocalId::new(0) {
None
} else {
let def_id = self.resolver.opt_local_def_id(node_id)?;
Some((hir_id.local_id, def_id))
.filter_map(|(&node_id, &local_id)| {
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.
let hir_id = hir::HirId { owner: self.current_hir_id_owner, local_id };
*o = hir::MaybeOwner::NonOwner(hir_id);
}

if local_id == hir::ItemLocalId::new(0) { None } else { Some((local_id, def_id)) }
})
.collect();

let trait_map = local_node_ids
let trait_map = node_id_to_local_id
.into_iter()
.filter_map(|node_id| {
let hir_id = self.node_id_to_hir_id[node_id]?;
.filter_map(|(node_id, local_id)| {
let traits = self.resolver.take_trait_map(node_id)?;
Some((hir_id.local_id, traits.into_boxed_slice()))
Some((local_id, traits.into_boxed_slice()))
})
.collect();

Expand Down Expand Up @@ -563,14 +551,18 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
fn lower_node_id(&mut self, ast_node_id: NodeId) -> hir::HirId {
assert_ne!(ast_node_id, DUMMY_NODE_ID);

*self.node_id_to_hir_id.get_or_insert_with(ast_node_id, || {
// Generate a new `HirId`.
let owner = self.current_hir_id_owner;
let local_id = self.item_local_id_counter;
self.item_local_id_counter.increment_by(1);
self.local_node_ids.push(ast_node_id);
hir::HirId { owner, local_id }
})
let owner = self.current_hir_id_owner;
let local_id = match self.node_id_to_local_id.entry(ast_node_id) {
Entry::Occupied(o) => *o.get(),
Entry::Vacant(v) => {
// Generate a new `HirId`.
let local_id = self.item_local_id_counter;
self.item_local_id_counter.increment_by(1);
v.insert(local_id);
local_id
}
};
hir::HirId { owner, local_id }
}

fn next_id(&mut self) -> hir::HirId {
Expand All @@ -579,11 +571,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}

fn lower_res(&mut self, res: Res<NodeId>) -> Res {
res.map_id(|id| {
self.node_id_to_hir_id.get(id).copied().flatten().unwrap_or_else(|| {
panic!("expected `NodeId` to be lowered already for res {:#?}", res);
})
})
let res: Result<Res, ()> = res.apply_id(|id| {
let owner = self.current_hir_id_owner;
let local_id = self.node_id_to_local_id.get(&id).copied().ok_or(())?;
Ok(hir::HirId { owner, local_id })
});
// We may fail to find a HirId when the Res points to a Local from an enclosing HIR owner.
// This can happen when trying to lower the return type `x` in erroneous code like
// async fn foo(x: u8) -> x {}
// In that case, `x` is lowered as a function parameter, and the return type is lowered as
// an opaque type as a synthetized HIR owner.
res.unwrap_or(Res::Err)
}

fn expect_full_res(&mut self, id: NodeId) -> Res<NodeId> {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_hir/src/def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,19 @@ impl<Id> Res<Id> {
}
}

pub fn apply_id<R, E>(self, mut map: impl FnMut(Id) -> Result<R, E>) -> Result<Res<R>, E> {
Ok(match self {
Res::Def(kind, id) => Res::Def(kind, id),
Res::SelfCtor(id) => Res::SelfCtor(id),
Res::PrimTy(id) => Res::PrimTy(id),
Res::Local(id) => Res::Local(map(id)?),
Res::SelfTy(a, b) => Res::SelfTy(a, b),
Res::ToolMod => Res::ToolMod,
Res::NonMacroAttr(attr_kind) => Res::NonMacroAttr(attr_kind),
Res::Err => Res::Err,
})
}

#[track_caller]
pub fn expect_non_local<OtherId>(self) -> Res<OtherId> {
self.map_id(|_| panic!("unexpected `Res::Local`"))
Expand Down
31 changes: 1 addition & 30 deletions compiler/rustc_hir/src/definitions.rs
View file
Open in desktop
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
Loading