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
+59 −56
Closed

HirIdify hir::ItemId #59092

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
Original file line number Diff line number Diff line change
@@ -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);
}
55 changes: 29 additions & 26 deletions src/librustc/hir/lowering.rs
View file
Original file line number Diff line number Diff line change
@@ -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>,
@@ -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) {
@@ -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);
@@ -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()
@@ -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);
}
@@ -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)
})
}

@@ -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 };
@@ -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,
@@ -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,
@@ -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 => {}
@@ -3496,7 +3498,7 @@ impl<'a> LoweringContext<'a> {
.skip(1)
.zip([id1, id2].iter())
{
vec.push(hir::ItemId { id });
vec.push(id);
}
},
}
@@ -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 {
4 changes: 2 additions & 2 deletions src/librustc/hir/map/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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 {
@@ -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),
8 changes: 4 additions & 4 deletions src/librustc/hir/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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>,
}
@@ -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>,
@@ -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]
}

@@ -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
4 changes: 2 additions & 2 deletions src/librustc/hir/print.rs
View file
Original file line number Diff line number Diff line change
@@ -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
}
}
@@ -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<()> {
7 changes: 4 additions & 3 deletions src/librustc/middle/resolve_lifetime.rs
View file
Original file line number Diff line number Diff line change
@@ -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 {
@@ -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))
{
2 changes: 1 addition & 1 deletion src/librustc_metadata/encoder.rs
View file
Original file line number Diff line number Diff line change
@@ -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),
7 changes: 3 additions & 4 deletions src/librustc_privacy/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -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));
@@ -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) {
2 changes: 1 addition & 1 deletion src/librustc_typeck/astconv.rs
View file
Original file line number Diff line number Diff line change
@@ -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)) => {
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/method/suggest.rs
View file
Original file line number Diff line number Diff line change
@@ -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
18 changes: 9 additions & 9 deletions src/librustdoc/clean/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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
@@ -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
@@ -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 {
@@ -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 {
Loading