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

HirIdify hir::ItemId #59092

Closed
wants to merge 1 commit into from
Closed

HirIdify hir::ItemId #59092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,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_by_hir_id(id.id));
if let Some(item) = opt_item {
self.visit_item(item);
}
Expand Down
55 changes: 29 additions & 26 deletions src/librustc/hir/lowering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub struct LoweringContext<'a> {
resolver: &'a mut dyn Resolver,

/// The items being lowered are collected here.
items: BTreeMap<NodeId, hir::Item>,
items: BTreeMap<hir::HirId, hir::Item>,

trait_items: BTreeMap<hir::TraitItemId, hir::TraitItem>,
impl_items: BTreeMap<hir::ImplItemId, hir::ImplItem>,
Expand Down Expand Up @@ -323,7 +323,7 @@ enum AnonymousLifetimeMode {
PassThrough,
}

struct ImplTraitTypeIdVisitor<'a> { ids: &'a mut SmallVec<[hir::ItemId; 1]> }
struct ImplTraitTypeIdVisitor<'a> { ids: &'a mut SmallVec<[NodeId; 1]> }

impl<'a, 'b> Visitor<'a> for ImplTraitTypeIdVisitor<'b> {
fn visit_ty(&mut self, ty: &'a Ty) {
Expand All @@ -332,7 +332,7 @@ impl<'a, 'b> Visitor<'a> for ImplTraitTypeIdVisitor<'b> {
| TyKind::BareFn(_)
=> return,

TyKind::ImplTrait(id, _) => self.ids.push(hir::ItemId { id }),
TyKind::ImplTrait(id, _) => self.ids.push(id),
_ => {},
}
visit::walk_ty(self, ty);
Expand Down Expand Up @@ -436,17 +436,16 @@ impl<'a> LoweringContext<'a> {
}

fn visit_item(&mut self, item: &'lcx Item) {
let mut item_lowered = true;
let mut item_hir_id = None;
self.lctx.with_hir_id_owner(item.id, |lctx| {
if let Some(hir_item) = lctx.lower_item(item) {
lctx.insert_item(item.id, hir_item);
} else {
item_lowered = false;
item_hir_id = Some(hir_item.hir_id);
lctx.insert_item(hir_item);
}
});

if item_lowered {
let item_generics = match self.lctx.items.get(&item.id).unwrap().node {
if let Some(hir_id) = item_hir_id {
let item_generics = match self.lctx.items.get(&hir_id).unwrap().node {
hir::ItemKind::Impl(_, _, _, ref generics, ..)
| hir::ItemKind::Trait(_, _, ref generics, ..) => {
generics.params.clone()
Expand Down Expand Up @@ -519,7 +518,8 @@ impl<'a> LoweringContext<'a> {
}
}

fn insert_item(&mut self, id: NodeId, item: hir::Item) {
fn insert_item(&mut self, item: hir::Item) {
let id = item.hir_id;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could try asserting that the local id of the item is 0 here?

You could also try printing out information about the item which isn't 0.

Copy link
Contributor Author

@ljedrz ljedrz Mar 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Got one:

non-zero local id in Item {
    ident: TryFrom#0, 
    hir_id: HirId { owner: DefIndex(0:147), local_id: 1 }, 
    attrs: [], 
    node: Use(path(convert::TryFrom), Single), 
    vis: Spanned { node: Inherited, span: Span { lo: BytePos(73297), hi: BytePos(73297), ctxt: #0 } }, 
    span: Span { lo: BytePos(73311), hi: BytePos(73318), ctxt: #0 }
}

self.items.insert(id, item);
self.modules.get_mut(&self.current_module).unwrap().items.insert(id);
}
Expand Down Expand Up @@ -1425,10 +1425,10 @@ impl<'a> LoweringContext<'a> {
// Insert the item into the global list. This usually happens
// automatically for all AST items. But this existential type item
// does not actually exist in the AST.
lctx.insert_item(exist_ty_id.node_id, exist_ty_item);
lctx.insert_item(exist_ty_item);

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

Expand Down Expand Up @@ -2003,9 +2003,9 @@ impl<'a> LoweringContext<'a> {
)
}

fn lower_local(&mut self, l: &Local) -> (hir::Local, SmallVec<[hir::ItemId; 1]>) {
fn lower_local(&mut self, l: &Local) -> (hir::Local, SmallVec<[NodeId; 1]>) {
let LoweredNodeId { node_id: _, hir_id } = self.lower_node_id(l.id);
let mut ids = SmallVec::<[hir::ItemId; 1]>::new();
let mut ids = SmallVec::<[NodeId; 1]>::new();
if self.sess.features_untracked().impl_trait_in_bindings {
if let Some(ref ty) = l.ty {
let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
Expand Down Expand Up @@ -3122,7 +3122,6 @@ impl<'a> LoweringContext<'a> {
let vis = respan(vis.span, vis_kind);

this.insert_item(
new_id.node_id,
hir::Item {
hir_id: new_id.hir_id,
ident,
Expand Down Expand Up @@ -3227,7 +3226,6 @@ impl<'a> LoweringContext<'a> {
let vis = respan(vis.span, vis_kind);

this.insert_item(
new_id,
hir::Item {
hir_id: new_hir_id,
ident,
Expand Down Expand Up @@ -3451,43 +3449,47 @@ impl<'a> LoweringContext<'a> {
}

fn lower_item_id(&mut self, i: &Item) -> SmallVec<[hir::ItemId; 1]> {
match i.node {
let node_ids = match i.node {
ItemKind::Use(ref use_tree) => {
let mut vec = smallvec![hir::ItemId { id: i.id }];
let mut vec = smallvec![i.id];
self.lower_item_id_use_tree(use_tree, i.id, &mut vec);
vec
}
ItemKind::MacroDef(..) => SmallVec::new(),
ItemKind::Fn(..) |
ItemKind::Impl(.., None, _, _) => smallvec![hir::ItemId { id: i.id }],
ItemKind::Impl(.., None, _, _) => smallvec![i.id],
ItemKind::Static(ref ty, ..) => {
let mut ids = smallvec![hir::ItemId { id: i.id }];
let mut ids = smallvec![i.id];
if self.sess.features_untracked().impl_trait_in_bindings {
let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
visitor.visit_ty(ty);
}
ids
},
ItemKind::Const(ref ty, ..) => {
let mut ids = smallvec![hir::ItemId { id: i.id }];
let mut ids = smallvec![i.id];
if self.sess.features_untracked().impl_trait_in_bindings {
let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
visitor.visit_ty(ty);
}
ids
},
_ => smallvec![hir::ItemId { id: i.id }],
}
_ => smallvec![i.id],
};

node_ids.into_iter()
.map(|node_id| hir::ItemId { id: self.lower_node_id(node_id).hir_id })
.collect()
Copy link
Contributor

Choose a reason for hiding this comment

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

I replaced this with:

        node_ids.into_iter().map(|node_id| hir::ItemId {
            id: self.lower_node_id_generic(node_id, |_| {
                panic!("expected node_id to be lowered already {:#?}", i)
            }).hir_id
        }).collect()

That panics and shows the the following item wasn't assigned a HirId:

expected node_id to be lowered already Item {
    ident: #0,
    attrs: [],
    id: NodeId(2249),
    node: Use(
        UseTree {
            prefix: path(convert),
            kind: Nested(
                [
                    (
                        UseTree {
                            prefix: path(TryFrom),
                            kind: Simple(
                                None,
                                NodeId(2252),
                                NodeId(2253)
                            ),
                        },
                        NodeId(2254)
                    ),
                    (
                        UseTree {
                            prefix: path(Infallible),
                            kind: Simple(
                                None,
                                NodeId(2256),
                                NodeId(2257)
                            ),
                        },
                        NodeId(2258)
                    )
                ]
            ),
        }
    ),
    vis: Spanned {
        node: Inherited,
    },
}

I guess this means that either too many NodeIds are in this list or some id is not being correctly lowered to a HirId.

@oli-obk Do you have any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

These node ids haven't been lowered yet at all. lower_item_id is called before the item is lowered in order to obtain any sub-item's ids. If you want to error if a second lowering fails, you'll need to do that where the actual items are lowered (so after lower_item_id is called).

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that only AST items had their ids lowered in MiscCollector. I changed that to also include the use trees in #59413.

}

fn lower_item_id_use_tree(&mut self,
tree: &UseTree,
base_id: NodeId,
vec: &mut SmallVec<[hir::ItemId; 1]>)
vec: &mut SmallVec<[NodeId; 1]>)
{
match tree.kind {
UseTreeKind::Nested(ref nested_vec) => for &(ref nested, id) in nested_vec {
vec.push(hir::ItemId { id });
vec.push(id);
self.lower_item_id_use_tree(nested, id, vec);
},
UseTreeKind::Glob => {}
Expand All @@ -3496,7 +3498,7 @@ impl<'a> LoweringContext<'a> {
.skip(1)
.zip([id1, id2].iter())
{
vec.push(hir::ItemId { id });
vec.push(id);
}
},
}
Expand Down Expand Up @@ -4611,6 +4613,7 @@ impl<'a> LoweringContext<'a> {
let mut ids: SmallVec<[hir::Stmt; 1]> = item_ids
.into_iter()
.map(|item_id| {
let item_id = hir::ItemId { id: self.lower_node_id(item_id).hir_id };
let LoweredNodeId { node_id: _, hir_id } = self.next_id();

hir::Stmt {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/hir/map/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ impl<'hir> Map<'hir> {
let module = &self.forest.krate.modules[&node_id];

for id in &module.items {
visitor.visit_item(self.expect_item(*id));
visitor.visit_item(self.expect_item_by_hir_id(*id));
}

for id in &module.trait_items {
Expand Down Expand Up @@ -1292,7 +1292,7 @@ pub fn map_crate<'hir>(sess: &crate::session::Session,
impl<'hir> print::PpAnn for Map<'hir> {
fn nested(&self, state: &mut print::State<'_>, nested: print::Nested) -> io::Result<()> {
match nested {
Nested::Item(id) => state.print_item(self.expect_item(id.id)),
Nested::Item(id) => state.print_item(self.expect_item_by_hir_id(id.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
8 changes: 4 additions & 4 deletions src/librustc/hir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ pub struct WhereEqPredicate {
pub struct ModuleItems {
// Use BTreeSets here so items are in the same order as in the
// list of all items in Crate
pub items: BTreeSet<NodeId>,
pub items: BTreeSet<HirId>,
pub trait_items: BTreeSet<TraitItemId>,
pub impl_items: BTreeSet<ImplItemId>,
}
Expand All @@ -718,7 +718,7 @@ pub struct Crate {
// does, because it can affect the order in which errors are
// detected, which in turn can make compile-fail tests yield
// slightly different results.
pub items: BTreeMap<NodeId, Item>,
pub items: BTreeMap<HirId, Item>,

pub trait_items: BTreeMap<TraitItemId, TraitItem>,
pub impl_items: BTreeMap<ImplItemId, ImplItem>,
Expand All @@ -738,7 +738,7 @@ pub struct Crate {
}

impl Crate {
pub fn item(&self, id: NodeId) -> &Item {
pub fn item(&self, id: HirId) -> &Item {
&self.items[&id]
}

Expand Down Expand Up @@ -2200,7 +2200,7 @@ impl VariantData {
// so it can fetched later.
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)]
pub struct ItemId {
pub id: NodeId,
pub id: HirId,
}

/// An item
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/hir/print.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub trait PpAnn {
fn post(&self, _state: &mut State<'_>, _node: AnnNode<'_>) -> io::Result<()> {
Ok(())
}
fn try_fetch_item(&self, _: ast::NodeId) -> Option<&hir::Item> {
fn try_fetch_item(&self, _: hir::HirId) -> Option<&hir::Item> {
None
}
}
Expand All @@ -58,7 +58,7 @@ impl PpAnn for NoAnn {}
pub const NO_ANN: &dyn PpAnn = &NoAnn;

impl PpAnn for hir::Crate {
fn try_fetch_item(&self, item: ast::NodeId) -> Option<&hir::Item> {
fn try_fetch_item(&self, item: hir::HirId) -> Option<&hir::Item> {
Some(self.item(item))
}
fn nested(&self, state: &mut State<'_>, nested: Nested) -> io::Result<()> {
Expand Down
7 changes: 4 additions & 3 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,8 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
// `abstract type MyAnonTy<'b>: MyTrait<'b>;`
// ^ ^ this gets resolved in the scope of
// the exist_ty generics
let (generics, bounds) = match self.tcx.hir().expect_item(item_id.id).node {
let (generics, bounds) = match self.tcx.hir().expect_item_by_hir_id(item_id.id).node
{
// named existential types are reached via TyKind::Path
// this arm is for `impl Trait` in the types of statics, constants and locals
hir::ItemKind::Existential(hir::ExistTy {
Expand Down Expand Up @@ -677,8 +678,8 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
let parent_impl_id = hir::ImplItemId { hir_id: parent_id };
let parent_trait_id = hir::TraitItemId { hir_id: parent_id };
let krate = self.tcx.hir().forest.krate();
let parent_node_id = self.tcx.hir().hir_to_node_id(parent_id);
if !(krate.items.contains_key(&parent_node_id)

if !(krate.items.contains_key(&parent_id)
|| krate.impl_items.contains_key(&parent_impl_id)
|| krate.trait_items.contains_key(&parent_trait_id))
{
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_metadata/encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
span: self.lazy(&tcx.def_span(def_id)),
attributes: self.encode_attributes(attrs),
children: self.lazy_seq(md.item_ids.iter().map(|item_id| {
tcx.hir().local_def_id(item_id.id).index
tcx.hir().local_def_id_from_hir_id(item_id.id).index
})),
stability: self.encode_stability(def_id),
deprecation: self.encode_deprecation(def_id),
Expand Down
7 changes: 3 additions & 4 deletions src/librustc_privacy/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> {
{
if let hir::ItemKind::Mod(m) = &item.node {
for item_id in m.item_ids.as_ref() {
let item = self.tcx.hir().expect_item(item_id.id);
let def_id = self.tcx.hir().local_def_id(item_id.id);
let item = self.tcx.hir().expect_item_by_hir_id(item_id.id);
let def_id = self.tcx.hir().local_def_id_from_hir_id(item_id.id);
if !self.tcx.hygienic_eq(segment.ident, item.ident, def_id) { continue; }
if let hir::ItemKind::Use(..) = item.node {
self.update(item.hir_id, Some(AccessLevel::Exported));
Expand Down Expand Up @@ -724,8 +724,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
unreachable!()
};
for id in &module.item_ids {
let hir_id = self.tcx.hir().node_to_hir_id(id.id);
self.update(hir_id, level);
self.update(id.id, level);
}
let def_id = self.tcx.hir().local_def_id_from_hir_id(module_id);
if let Some(exports) = self.tcx.module_exports(def_id) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/astconv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1812,7 +1812,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
self.def_to_ty(opt_self_ty, path, false)
}
hir::TyKind::Def(item_id, ref lifetimes) => {
let did = tcx.hir().local_def_id(item_id.id);
let did = tcx.hir().local_def_id_from_hir_id(item_id.id);
self.impl_trait_ty_to_ty(did, lifetimes)
},
hir::TyKind::Path(hir::QPath::TypeRelative(ref qself, ref segment)) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ impl<'a, 'tcx, 'gcx> hir::intravisit::Visitor<'tcx> for UsePlacementFinder<'a, '
}
// Find a `use` statement.
for item_id in &module.item_ids {
let item = self.tcx.hir().expect_item(item_id.id);
let item = self.tcx.hir().expect_item_by_hir_id(item_id.id);
match item.node {
hir::ItemKind::Use(..) => {
// Don't suggest placing a `use` before the prelude
Expand Down
18 changes: 9 additions & 9 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,16 +277,16 @@ impl Clean<ExternalCrate> for CrateNum {
};
let primitives = if root.is_local() {
cx.tcx.hir().krate().module.item_ids.iter().filter_map(|&id| {
let item = cx.tcx.hir().expect_item(id.id);
let item = cx.tcx.hir().expect_item_by_hir_id(id.id);
match item.node {
hir::ItemKind::Mod(_) => {
as_primitive(Def::Mod(cx.tcx.hir().local_def_id(id.id)))
as_primitive(Def::Mod(cx.tcx.hir().local_def_id_from_hir_id(id.id)))
}
hir::ItemKind::Use(ref path, hir::UseKind::Single)
if item.vis.node.is_pub() => {
as_primitive(path.def).map(|(_, prim, attrs)| {
// Pretend the primitive is local.
(cx.tcx.hir().local_def_id(id.id), prim, attrs)
(cx.tcx.hir().local_def_id_from_hir_id(id.id), prim, attrs)
})
}
_ => None
Expand Down Expand Up @@ -319,15 +319,15 @@ impl Clean<ExternalCrate> for CrateNum {
};
let keywords = if root.is_local() {
cx.tcx.hir().krate().module.item_ids.iter().filter_map(|&id| {
let item = cx.tcx.hir().expect_item(id.id);
let item = cx.tcx.hir().expect_item_by_hir_id(id.id);
match item.node {
hir::ItemKind::Mod(_) => {
as_keyword(Def::Mod(cx.tcx.hir().local_def_id(id.id)))
as_keyword(Def::Mod(cx.tcx.hir().local_def_id_from_hir_id(id.id)))
}
hir::ItemKind::Use(ref path, hir::UseKind::Single)
if item.vis.node.is_pub() => {
as_keyword(path.def).map(|(_, prim, attrs)| {
(cx.tcx.hir().local_def_id(id.id), prim, attrs)
(cx.tcx.hir().local_def_id_from_hir_id(id.id), prim, attrs)
})
}
_ => None
Expand Down Expand Up @@ -2557,7 +2557,7 @@ impl Clean<Type> for hir::Ty {
},
TyKind::Tup(ref tys) => Tuple(tys.clean(cx)),
TyKind::Def(item_id, _) => {
let item = cx.tcx.hir().expect_item(item_id.id);
let item = cx.tcx.hir().expect_item_by_hir_id(item_id.id);
if let hir::ItemKind::Existential(ref ty) = item.node {
ImplTrait(ty.bounds.clean(cx))
} else {
Expand Down Expand Up @@ -4170,10 +4170,10 @@ pub fn path_to_def_local(tcx: &TyCtxt<'_, '_, '_>, path: &[&str]) -> Option<DefI
let segment = path_it.next()?;

for item_id in mem::replace(&mut items, HirVec::new()).iter() {
let item = tcx.hir().expect_item(item_id.id);
let item = tcx.hir().expect_item_by_hir_id(item_id.id);
if item.ident.name == *segment {
if path_it.peek().is_none() {
return Some(tcx.hir().local_def_id(item_id.id))
return Some(tcx.hir().local_def_id_from_hir_id(item_id.id))
}

items = match &item.node {
Expand Down
Loading