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.

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

Replace HIR's ItemId structs with aliases #67999

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub trait Visitor<'v>: Sized {
/// but cannot supply a `Map`; see `nested_visit_map` for advice.
#[allow(unused_variables)]
fn visit_nested_item(&mut self, id: ItemId) {
let opt_item = self.nested_visit_map().inter().map(|map| map.expect_item(id.id));
let opt_item = self.nested_visit_map().inter().map(|map| map.expect_item(id));
if let Some(item) = opt_item {
self.visit_item(item);
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {

fn visit_nested_item(&mut self, item: ItemId) {
debug!("visit_nested_item: {:?}", item);
self.visit_item(self.krate.item(item.id));
self.visit_item(self.krate.item(item));
}

fn visit_nested_trait_item(&mut self, item_id: TraitItemId) {
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,15 +406,15 @@ impl<'hir> Map<'hir> {
}

pub fn trait_item(&self, id: TraitItemId) -> &'hir TraitItem<'hir> {
self.read(id.hir_id);
self.read(id);

// N.B., intentionally bypass `self.forest.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.trait_item(id)
}

pub fn impl_item(&self, id: ImplItemId) -> &'hir ImplItem<'hir> {
self.read(id.hir_id);
self.read(id);

// N.B., intentionally bypass `self.forest.krate()` so that we
// do not trigger a read of the whole krate here
Expand Down Expand Up @@ -566,11 +566,11 @@ impl<'hir> Map<'hir> {
}

for id in &module.trait_items {
visitor.visit_trait_item(self.expect_trait_item(id.hir_id));
visitor.visit_trait_item(self.expect_trait_item(*id));
}

for id in &module.impl_items {
visitor.visit_impl_item(self.expect_impl_item(id.hir_id));
visitor.visit_impl_item(self.expect_impl_item(*id));
}
}

Expand Down Expand Up @@ -1256,7 +1256,7 @@ pub fn map_crate<'hir>(
impl<'hir> print::PpAnn for Map<'hir> {
fn nested(&self, state: &mut print::State<'_>, nested: print::Nested) {
match nested {
Nested::Item(id) => state.print_item(self.expect_item(id.id)),
Nested::Item(id) => state.print_item(self.expect_item(id)),
Nested::TraitItem(id) => state.print_trait_item(self.trait_item(id)),
Nested::ImplItem(id) => state.print_impl_item(self.impl_item(id)),
Nested::Body(id) => state.print_expr(&self.body(id).value),
Expand Down
36 changes: 1 addition & 35 deletions src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,40 +40,6 @@ impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
}
}

// The following implementations of HashStable for `ItemId`, `TraitItemId`, and
// `ImplItemId` deserve special attention. Normally we do not hash `NodeId`s within
// the HIR, since they just signify a HIR nodes own path. But `ItemId` et al
// are used when another item in the HIR is *referenced* and we certainly
// want to pick up on a reference changing its target, so we hash the NodeIds
// in "DefPath Mode".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types do have different impls than HirId. Why can this now be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing that comment I thought about how much the NodeIds have become deprecated after the lowering and wondered if we could get away with it. If there's a reason we should stick to this solution, though, we can probably still merge all these 3 types into 1, because their impls are identical.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we should either merge them, or keep them separate if there's a good reason for them to remain separate. I'm not confident about the reason for their being separate in the first place, though, so I'd prefer if someone else clarified this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging the 3 types into 1 should be fine. Using an alias would be a change in semantics, leading to things being ignored during hashing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestion on the name? Would ItemId be fine?


fn hash_item_id(&mut self, id: hir::ItemId, hasher: &mut StableHasher) {
let hcx = self;
let hir::ItemId { id } = id;

hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
id.hash_stable(hcx, hasher);
})
}

fn hash_impl_item_id(&mut self, id: hir::ImplItemId, hasher: &mut StableHasher) {
let hcx = self;
let hir::ImplItemId { hir_id } = id;

hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
hir_id.hash_stable(hcx, hasher);
})
}

fn hash_trait_item_id(&mut self, id: hir::TraitItemId, hasher: &mut StableHasher) {
let hcx = self;
let hir::TraitItemId { hir_id } = id;

hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
hir_id.hash_stable(hcx, hasher);
})
}

fn hash_hir_mod(&mut self, module: &hir::Mod<'_>, hasher: &mut StableHasher) {
let hcx = self;
let hir::Mod { inner: ref inner_span, ref item_ids } = *module;
Expand All @@ -86,7 +52,7 @@ impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> {
let item_ids_hash = item_ids
.iter()
.map(|id| {
let (def_path_hash, local_id) = id.id.to_stable_hash_key(hcx);
let (def_path_hash, local_id) = id.to_stable_hash_key(hcx);
debug_assert_eq!(local_id, hir::ItemLocalId::from_u32(0));
def_path_hash.0
})
Expand Down
16 changes: 8 additions & 8 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2756,7 +2756,7 @@ impl<'tcx> TyCtxt<'tcx> {
parent_vis: &hir::Visibility<'_>,
trait_item_ref: &hir::TraitItemRef,
) -> AssocItem {
let def_id = self.hir().local_def_id(trait_item_ref.id.hir_id);
let def_id = self.hir().local_def_id(trait_item_ref.id);
let (kind, has_self) = match trait_item_ref.kind {
hir::AssocItemKind::Const => (ty::AssocKind::Const, false),
hir::AssocItemKind::Method { has_self } => (ty::AssocKind::Method, has_self),
Expand All @@ -2768,7 +2768,7 @@ impl<'tcx> TyCtxt<'tcx> {
ident: trait_item_ref.ident,
kind,
// Visibility of trait items is inherited from their traits.
vis: Visibility::from_hir(parent_vis, trait_item_ref.id.hir_id, self),
vis: Visibility::from_hir(parent_vis, trait_item_ref.id, self),
defaultness: trait_item_ref.defaultness,
def_id,
container: TraitContainer(parent_def_id),
Expand All @@ -2781,7 +2781,7 @@ impl<'tcx> TyCtxt<'tcx> {
parent_def_id: DefId,
impl_item_ref: &hir::ImplItemRef<'_>,
) -> AssocItem {
let def_id = self.hir().local_def_id(impl_item_ref.id.hir_id);
let def_id = self.hir().local_def_id(impl_item_ref.id);
let (kind, has_self) = match impl_item_ref.kind {
hir::AssocItemKind::Const => (ty::AssocKind::Const, false),
hir::AssocItemKind::Method { has_self } => (ty::AssocKind::Method, has_self),
Expand All @@ -2793,7 +2793,7 @@ impl<'tcx> TyCtxt<'tcx> {
ident: impl_item_ref.ident,
kind,
// Visibility of trait impl items doesn't matter.
vis: ty::Visibility::from_hir(&impl_item_ref.vis, impl_item_ref.id.hir_id, self),
vis: ty::Visibility::from_hir(&impl_item_ref.vis, impl_item_ref.id, self),
defaultness: impl_item_ref.defaultness,
def_id,
container: ImplContainer(parent_def_id),
Expand Down Expand Up @@ -3085,7 +3085,7 @@ fn associated_item(tcx: TyCtxt<'_>, def_id: DefId) -> AssocItem {
let parent_item = tcx.hir().expect_item(parent_id);
match parent_item.kind {
hir::ItemKind::Impl(.., ref impl_item_refs) => {
if let Some(impl_item_ref) = impl_item_refs.iter().find(|i| i.id.hir_id == id) {
if let Some(impl_item_ref) = impl_item_refs.iter().find(|i| i.id == id) {
let assoc_item =
tcx.associated_item_from_impl_item_ref(parent_def_id, impl_item_ref);
debug_assert_eq!(assoc_item.def_id, def_id);
Expand All @@ -3094,7 +3094,7 @@ fn associated_item(tcx: TyCtxt<'_>, def_id: DefId) -> AssocItem {
}

hir::ItemKind::Trait(.., ref trait_item_refs) => {
if let Some(trait_item_ref) = trait_item_refs.iter().find(|i| i.id.hir_id == id) {
if let Some(trait_item_ref) = trait_item_refs.iter().find(|i| i.id == id) {
let assoc_item = tcx.associated_item_from_trait_item_ref(
parent_def_id,
&parent_item.vis,
Expand Down Expand Up @@ -3150,13 +3150,13 @@ fn associated_item_def_ids(tcx: TyCtxt<'_>, def_id: DefId) -> &[DefId] {
trait_item_refs
.iter()
.map(|trait_item_ref| trait_item_ref.id)
.map(|id| tcx.hir().local_def_id(id.hir_id)),
.map(|id| tcx.hir().local_def_id(id)),
),
hir::ItemKind::Impl(.., ref impl_item_refs) => tcx.arena.alloc_from_iter(
impl_item_refs
.iter()
.map(|impl_item_ref| impl_item_ref.id)
.map(|id| tcx.hir().local_def_id(id.hir_id)),
.map(|id| tcx.hir().local_def_id(id)),
),
hir::ItemKind::TraitAlias(..) => &[],
_ => span_bug!(item.span, "associated_item_def_ids: not impl or trait"),
Expand Down
13 changes: 5 additions & 8 deletions src/librustc_ast_lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl<'a, 'lowering, 'hir> Visitor<'a> for ItemLowerer<'a, 'lowering, 'hir> {
fn visit_trait_item(&mut self, item: &'a AssocItem) {
self.lctx.with_hir_id_owner(item.id, |lctx| {
let hir_item = lctx.lower_trait_item(item);
let id = hir::TraitItemId { hir_id: hir_item.hir_id };
let id = hir_item.hir_id;
lctx.trait_items.insert(id, hir_item);
lctx.modules.get_mut(&lctx.current_module).unwrap().trait_items.insert(id);
});
Expand All @@ -93,7 +93,7 @@ impl<'a, 'lowering, 'hir> Visitor<'a> for ItemLowerer<'a, 'lowering, 'hir> {
fn visit_impl_item(&mut self, item: &'a AssocItem) {
self.lctx.with_hir_id_owner(item.id, |lctx| {
let hir_item = lctx.lower_impl_item(item);
let id = hir::ImplItemId { hir_id: hir_item.hir_id };
let id = hir_item.hir_id;
lctx.impl_items.insert(id, hir_item);
lctx.modules.get_mut(&lctx.current_module).unwrap().impl_items.insert(id);
});
Expand Down Expand Up @@ -190,10 +190,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
_ => smallvec![i.id],
};

node_ids
.into_iter()
.map(|node_id| hir::ItemId { id: self.allocate_hir_id_counter(node_id) })
.collect()
node_ids.into_iter().map(|node_id| self.allocate_hir_id_counter(node_id)).collect()
}

fn lower_item_id_use_tree(
Expand Down Expand Up @@ -807,7 +804,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
AssocItemKind::Macro(..) => unimplemented!(),
};
hir::TraitItemRef {
id: hir::TraitItemId { hir_id: self.lower_node_id(i.id) },
id: self.lower_node_id(i.id),
ident: i.ident,
span: i.span,
defaultness: self.lower_defaultness(Defaultness::Default, has_default),
Expand Down Expand Up @@ -890,7 +887,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef<'hir> {
hir::ImplItemRef {
id: hir::ImplItemId { hir_id: self.lower_node_id(i.id) },
id: self.lower_node_id(i.id),
ident: i.ident,
span: i.span,
vis: self.lower_visibility(&i.vis, Some(i.id)),
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
lctx.generate_opaque_type(opaque_ty_node_id, opaque_ty_item, span, opaque_ty_span);

// `impl Trait` now just becomes `Foo<'a, 'b, ..>`.
hir::TyKind::Def(hir::ItemId { id: opaque_ty_id }, lifetimes)
hir::TyKind::Def(opaque_ty_id, lifetimes)
})
}

Expand Down Expand Up @@ -2364,7 +2364,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// Foo = impl Trait` is, internally, created as a child of the
// async fn, so the *type parameters* are inherited. It's
// only the lifetime parameters that we must supply.
let opaque_ty_ref = hir::TyKind::Def(hir::ItemId { id: opaque_ty_id }, generic_args);
let opaque_ty_ref = hir::TyKind::Def(opaque_ty_id, generic_args);
let opaque_ty = self.ty(opaque_ty_span, opaque_ty_ref);
hir::FunctionRetTy::Return(self.arena.alloc(opaque_ty))
}
Expand Down Expand Up @@ -2904,7 +2904,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let mut ids: SmallVec<[hir::Stmt<'hir>; 1]> = item_ids
.into_iter()
.map(|item_id| {
let item_id = hir::ItemId { id: self.lower_node_id(item_id) };
let item_id = self.lower_node_id(item_id);
self.stmt(s.span, hir::StmtKind::Item(item_id))
})
.collect();
Expand Down
15 changes: 3 additions & 12 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1799,10 +1799,7 @@ pub struct FnSig<'hir> {
// The bodies for items are stored "out of line", in a separate
// hashmap in the `Crate`. Here we just record the node-id of the item
// so it can fetched later.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Debug)]
pub struct TraitItemId {
pub hir_id: HirId,
}
pub type TraitItemId = HirId;

/// Represents an item declaration within a trait declaration,
/// possibly including a default implementation. A trait item is
Expand Down Expand Up @@ -1843,10 +1840,7 @@ pub enum TraitItemKind<'hir> {
// The bodies for items are stored "out of line", in a separate
// hashmap in the `Crate`. Here we just record the node-id of the item
// so it can fetched later.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Debug)]
pub struct ImplItemId {
pub hir_id: HirId,
}
pub type ImplItemId = HirId;

/// Represents anything within an `impl` block.
#[derive(RustcEncodable, RustcDecodable, Debug)]
Expand Down Expand Up @@ -2349,10 +2343,7 @@ impl VariantData<'hir> {
// The bodies for items are stored "out of line", in a separate
// hashmap in the `Crate`. Here we just record the node-id of the item
// so it can fetched later.
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)]
pub struct ItemId {
pub id: HirId,
}
pub type ItemId = HirId;

/// An item
///
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl PpAnn for hir::Crate<'a> {
}
fn nested(&self, state: &mut State<'_>, nested: Nested) {
match nested {
Nested::Item(id) => state.print_item(self.item(id.id)),
Nested::Item(id) => state.print_item(self.item(id)),
Nested::TraitItem(id) => state.print_trait_item(self.trait_item(id)),
Nested::ImplItem(id) => state.print_impl_item(self.impl_item(id)),
Nested::Body(id) => state.print_expr(&self.body(id).value),
Expand Down
23 changes: 1 addition & 22 deletions src/librustc_hir/stable_hash_impls.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};

use crate::def_id::DefId;
use crate::hir::{BodyId, Expr, ImplItemId, ItemId, Mod, TraitItemId, Ty, VisibilityKind};
use crate::hir::{BodyId, Expr, Mod, Ty, VisibilityKind};
use crate::hir_id::HirId;

/// Requirements for a `StableHashingContext` to be used in this crate.
Expand All @@ -11,9 +11,6 @@ pub trait HashStableContext: syntax::HashStableContext + rustc_target::HashStabl
fn hash_def_id(&mut self, _: DefId, hasher: &mut StableHasher);
fn hash_hir_id(&mut self, _: HirId, hasher: &mut StableHasher);
fn hash_body_id(&mut self, _: BodyId, hasher: &mut StableHasher);
fn hash_item_id(&mut self, _: ItemId, hasher: &mut StableHasher);
fn hash_impl_item_id(&mut self, _: ImplItemId, hasher: &mut StableHasher);
fn hash_trait_item_id(&mut self, _: TraitItemId, hasher: &mut StableHasher);
fn hash_hir_mod(&mut self, _: &Mod<'_>, hasher: &mut StableHasher);
fn hash_hir_expr(&mut self, _: &Expr<'_>, hasher: &mut StableHasher);
fn hash_hir_ty(&mut self, _: &Ty<'_>, hasher: &mut StableHasher);
Expand All @@ -38,24 +35,6 @@ impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for BodyId {
}
}

impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for ItemId {
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
hcx.hash_item_id(*self, hasher)
}
}

impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for ImplItemId {
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
hcx.hash_impl_item_id(*self, hasher)
}
}

impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for TraitItemId {
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
hcx.hash_trait_item_id(*self, hasher)
}
}

impl<HirCtx: crate::HashStableContext> HashStable<HirCtx> for Mod<'_> {
fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
hcx.hash_hir_mod(self, hasher)
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
if let hir::VisibilityKind::Inherited = it.vis.node {
self.private_traits.insert(it.hir_id);
for trait_item_ref in trait_item_refs {
self.private_traits.insert(trait_item_ref.id.hir_id);
self.private_traits.insert(trait_item_ref.id);
}
return;
}
Expand All @@ -443,7 +443,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc {
Some(Node::Item(item)) => {
if let hir::VisibilityKind::Inherited = item.vis.node {
for impl_item_ref in impl_item_refs {
self.private_traits.insert(impl_item_ref.id.hir_id);
self.private_traits.insert(impl_item_ref.id);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ impl EncodeContext<'tcx> {
record!(self.per_def.span[def_id] <- self.tcx.def_span(def_id));
record!(self.per_def.attributes[def_id] <- attrs);
record!(self.per_def.children[def_id] <- md.item_ids.iter().map(|item_id| {
tcx.hir().local_def_id(item_id.id).index
tcx.hir().local_def_id(*item_id).index
}));
self.encode_stability(def_id);
self.encode_deprecation(def_id);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_passes/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> {
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) {
match ty.kind {
TyKind::Def(item_id, _) => {
let item = self.tcx.hir().expect_item(item_id.id);
let item = self.tcx.hir().expect_item(item_id);
intravisit::walk_item(self, item);
}
_ => (),
Expand Down Expand Up @@ -412,7 +412,7 @@ impl<'v, 'k, 'tcx> ItemLikeVisitor<'v> for LifeSeeder<'k, 'tcx> {
&impl_item.attrs,
)
{
self.worklist.push(impl_item_ref.id.hir_id);
self.worklist.push(impl_item_ref.id);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_passes/hir_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> {
}

fn visit_nested_item(&mut self, id: hir::ItemId) {
let nested_item = self.krate.unwrap().item(id.id);
let nested_item = self.krate.unwrap().item(id);
self.visit_item(nested_item)
}

Expand Down
Loading