Skip to content

Commit

Permalink
Do not use DUMMY_HIR_ID as placeholder value in node_id_to_hir_id t…
Browse files Browse the repository at this point in the history
…able

Some helpers functions have been introduced to deal with (buggy) cases
where either a `NodeId` or a `DefId` do not have a corresponding `HirId`.
Those cases are tracked in issue rust-lang#71104.
  • Loading branch information
marmeladema committed Apr 14, 2020
1 parent 513a647 commit b9161ab
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 30 deletions.
23 changes: 10 additions & 13 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ struct LoweringContext<'a, 'hir: 'a> {

current_hir_id_owner: Vec<(LocalDefId, u32)>,
item_local_id_counters: NodeMap<u32>,
node_id_to_hir_id: IndexVec<NodeId, hir::HirId>,
node_id_to_hir_id: IndexVec<NodeId, Option<hir::HirId>>,

allow_try_trait: Option<Lrc<[Symbol]>>,
allow_gen_future: Option<Lrc<[Symbol]>>,
Expand Down Expand Up @@ -522,15 +522,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}

self.lower_node_id(CRATE_NODE_ID);
debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == hir::CRATE_HIR_ID);
debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == Some(hir::CRATE_HIR_ID));

visit::walk_crate(&mut MiscCollector { lctx: &mut self, hir_id_owner: None }, c);
visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c);

let module = self.lower_mod(&c.module);
let attrs = self.lower_attrs(&c.attrs);
let body_ids = body_ids(&self.bodies);
let proc_macros = c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id]).collect();
let proc_macros =
c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id].unwrap()).collect();

self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id);

Expand Down Expand Up @@ -571,26 +572,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
ast_node_id: NodeId,
alloc_hir_id: impl FnOnce(&mut Self) -> hir::HirId,
) -> hir::HirId {
if ast_node_id == DUMMY_NODE_ID {
return hir::DUMMY_HIR_ID;
}
assert_ne!(ast_node_id, DUMMY_NODE_ID);

let min_size = ast_node_id.as_usize() + 1;

if min_size > self.node_id_to_hir_id.len() {
self.node_id_to_hir_id.resize(min_size, hir::DUMMY_HIR_ID);
self.node_id_to_hir_id.resize(min_size, None);
}

let existing_hir_id = self.node_id_to_hir_id[ast_node_id];

if existing_hir_id == hir::DUMMY_HIR_ID {
if let Some(existing_hir_id) = self.node_id_to_hir_id[ast_node_id] {
existing_hir_id
} else {
// Generate a new `HirId`.
let hir_id = alloc_hir_id(self);
self.node_id_to_hir_id[ast_node_id] = hir_id;
self.node_id_to_hir_id[ast_node_id] = Some(hir_id);

hir_id
} else {
existing_hir_id
}
}

Expand Down
20 changes: 17 additions & 3 deletions src/librustc_hir/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub struct Definitions {
node_id_to_def_id: FxHashMap<ast::NodeId, LocalDefId>,
def_id_to_node_id: IndexVec<LocalDefId, ast::NodeId>,

pub(super) node_id_to_hir_id: IndexVec<ast::NodeId, hir::HirId>,
pub(super) node_id_to_hir_id: IndexVec<ast::NodeId, Option<hir::HirId>>,
/// The reverse mapping of `node_id_to_hir_id`.
pub(super) hir_id_to_node_id: FxHashMap<hir::HirId, ast::NodeId>,

Expand Down Expand Up @@ -359,11 +359,22 @@ impl Definitions {

#[inline]
pub fn node_id_to_hir_id(&self, node_id: ast::NodeId) -> hir::HirId {
self.node_id_to_hir_id[node_id].unwrap()
}

#[inline]
pub fn opt_node_id_to_hir_id(&self, node_id: ast::NodeId) -> Option<hir::HirId> {
self.node_id_to_hir_id[node_id]
}

#[inline]
pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId {
let node_id = self.def_id_to_node_id[id];
self.node_id_to_hir_id[node_id].unwrap()
}

#[inline]
pub fn opt_local_def_id_to_hir_id(&self, id: LocalDefId) -> Option<hir::HirId> {
let node_id = self.def_id_to_node_id[id];
self.node_id_to_hir_id[node_id]
}
Expand Down Expand Up @@ -470,7 +481,10 @@ impl Definitions {

/// Initializes the `ast::NodeId` to `HirId` mapping once it has been generated during
/// AST to HIR lowering.
pub fn init_node_id_to_hir_id_mapping(&mut self, mapping: IndexVec<ast::NodeId, hir::HirId>) {
pub fn init_node_id_to_hir_id_mapping(
&mut self,
mapping: IndexVec<ast::NodeId, Option<hir::HirId>>,
) {
assert!(
self.node_id_to_hir_id.is_empty(),
"trying to initialize `NodeId` -> `HirId` mapping twice"
Expand All @@ -481,7 +495,7 @@ impl Definitions {
self.hir_id_to_node_id = self
.node_id_to_hir_id
.iter_enumerated()
.map(|(node_id, &hir_id)| (hir_id, node_id))
.filter_map(|(node_id, &hir_id)| hir_id.map(|hir_id| (hir_id, node_id)))
.collect();
}

Expand Down
10 changes: 10 additions & 0 deletions src/librustc_middle/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,21 @@ impl<'hir> Map<'hir> {
self.tcx.definitions.node_id_to_hir_id(node_id)
}

#[inline]
pub fn opt_node_id_to_hir_id(&self, node_id: NodeId) -> Option<HirId> {
self.tcx.definitions.opt_node_id_to_hir_id(node_id)
}

#[inline]
pub fn local_def_id_to_hir_id(&self, def_id: LocalDefId) -> HirId {
self.tcx.definitions.local_def_id_to_hir_id(def_id)
}

#[inline]
pub fn opt_local_def_id_to_hir_id(&self, def_id: LocalDefId) -> Option<HirId> {
self.tcx.definitions.opt_local_def_id_to_hir_id(def_id)
}

pub fn def_kind(&self, hir_id: HirId) -> Option<DefKind> {
let node = self.find(hir_id)?;

Expand Down
17 changes: 10 additions & 7 deletions src/librustc_middle/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1126,13 +1126,16 @@ impl<'tcx> TyCtxt<'tcx> {

let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default();
for (k, v) in resolutions.trait_map {
let hir_id = definitions.node_id_to_hir_id(k);
let map = trait_map.entry(hir_id.owner).or_default();
let v = v
.into_iter()
.map(|tc| tc.map_import_ids(|id| definitions.node_id_to_hir_id(id)))
.collect();
map.insert(hir_id.local_id, StableVec::new(v));
// FIXME(#71104) Should really be using just `node_id_to_hir_id` but
// some `NodeId` do not seem to have a corresponding HirId.
if let Some(hir_id) = definitions.opt_node_id_to_hir_id(k) {
let map = trait_map.entry(hir_id.owner).or_default();
let v = v
.into_iter()
.map(|tc| tc.map_import_ids(|id| definitions.node_id_to_hir_id(id)))
.collect();
map.insert(hir_id.local_id, StableVec::new(v));
}
}

GlobalCtxt {
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,12 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {

let macro_module_def_id =
ty::DefIdTree::parent(self.tcx, self.tcx.hir().local_def_id(md.hir_id)).unwrap();
let mut module_id = match self.tcx.hir().as_local_hir_id(macro_module_def_id) {
// FIXME(#71104) Should really be using just `as_local_hir_id` but
// some `DefId` do not seem to have a corresponding HirId.
let hir_id = macro_module_def_id
.as_local()
.and_then(|def_id| self.tcx.hir().opt_local_def_id_to_hir_id(def_id));
let mut module_id = match hir_id {
Some(module_id) if self.tcx.hir().is_hir_id_module(module_id) => module_id,
// `module_id` doesn't correspond to a `mod`, return early (#63164, #65252).
_ => return,
Expand Down
13 changes: 8 additions & 5 deletions src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,14 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> {
collector.visit_pat(&arg.pat);

for (id, ident, ..) in collector.collected_idents {
let hir_id = self.tcx.hir().node_id_to_hir_id(id);
let typ = match self.save_ctxt.tables.node_type_opt(hir_id) {
Some(s) => s.to_string(),
None => continue,
};
// FIXME(#71104) Should really be using just `node_id_to_hir_id` but
// some `NodeId` do not seem to have a corresponding HirId.
let hir_id = self.tcx.hir().opt_node_id_to_hir_id(id);
let typ =
match hir_id.and_then(|hir_id| self.save_ctxt.tables.node_type_opt(hir_id)) {
Some(s) => s.to_string(),
None => continue,
};
if !self.span.filter_generated(ident.span) {
let id = id_from_node_id(id, &self.save_ctxt);
let span = self.span_from_span(ident.span);
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,11 @@ fn has_typeck_tables(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
return tcx.has_typeck_tables(outer_def_id);
}

if let Some(id) = tcx.hir().as_local_hir_id(def_id) {
// FIXME(#71104) Should really be using just `as_local_hir_id` but
// some `LocalDefId` do not seem to have a corresponding HirId.
if let Some(id) =
def_id.as_local().and_then(|def_id| tcx.hir().opt_local_def_id_to_hir_id(def_id))
{
primary_body_of(tcx, id).is_some()
} else {
false
Expand Down

0 comments on commit b9161ab

Please sign in to comment.