Skip to content

Entirely remove DUMMY_HIR_ID #71116

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

Merged
merged 2 commits into from
Apr 15, 2020
Merged
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
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
24 changes: 18 additions & 6 deletions src/librustc_hir/definitions.rs
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, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use crate::hir;
use crate::hir_id::DUMMY_HIR_ID;

use rustc_ast::ast;
use rustc_ast::crate_disambiguator::CrateDisambiguator;
Expand Down Expand Up @@ -87,7 +86,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 @@ -345,8 +344,7 @@ impl Definitions {
#[inline]
pub fn as_local_hir_id(&self, def_id: DefId) -> Option<hir::HirId> {
if let Some(def_id) = def_id.as_local() {
let hir_id = self.local_def_id_to_hir_id(def_id);
if hir_id != DUMMY_HIR_ID { Some(hir_id) } else { None }
Some(self.local_def_id_to_hir_id(def_id))
} else {
None
}
Expand All @@ -359,11 +357,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 +479,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 +493,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
3 changes: 0 additions & 3 deletions src/librustc_hir/hir_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,4 @@ pub const CRATE_HIR_ID: HirId = HirId {
local_id: ItemLocalId::from_u32(0),
};

pub const DUMMY_HIR_ID: HirId =
HirId { owner: LocalDefId { local_def_index: CRATE_DEF_INDEX }, local_id: DUMMY_ITEM_LOCAL_ID };

pub const DUMMY_ITEM_LOCAL_ID: ItemLocalId = ItemLocalId::MAX;
9 changes: 1 addition & 8 deletions src/librustc_middle/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,23 +250,16 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
None => format!("{:?}", node),
};

let forgot_str = if hir_id == hir::DUMMY_HIR_ID {
format!("\nMaybe you forgot to lower the node id {:?}?", node_id)
} else {
String::new()
};

span_bug!(
span,
"inconsistent DepNode at `{:?}` for `{}`: \
current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?}){}",
current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?})",
self.source_map.span_to_string(span),
node_str,
self.definitions.def_path(self.current_dep_node_owner).to_string_no_crate(),
self.current_dep_node_owner,
self.definitions.def_path(hir_id.owner).to_string_no_crate(),
hir_id.owner,
forgot_str,
)
}
}
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
16 changes: 2 additions & 14 deletions src/librustc_middle/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ fn late_report_deprecation(
suggestion: Option<Symbol>,
lint: &'static Lint,
span: Span,
def_id: DefId,
hir_id: HirId,
) {
if span.in_derive_expansion() {
Expand All @@ -229,9 +228,6 @@ fn late_report_deprecation(
}
diag.emit()
});
if hir_id == hir::DUMMY_HIR_ID {
span_bug!(span, "emitted a {} lint with dummy HIR id: {:?}", lint.name, def_id);
}
}

/// Result of `TyCtxt::eval_stability`.
Expand Down Expand Up @@ -296,7 +292,7 @@ impl<'tcx> TyCtxt<'tcx> {
if !skip {
let (message, lint) =
deprecation_message(&depr_entry.attr, &self.def_path_str(def_id));
late_report_deprecation(self, &message, None, lint, span, def_id, id);
late_report_deprecation(self, &message, None, lint, span, id);
}
};
}
Expand All @@ -319,15 +315,7 @@ impl<'tcx> TyCtxt<'tcx> {
if let Some(depr) = &stability.rustc_depr {
let (message, lint) =
rustc_deprecation_message(depr, &self.def_path_str(def_id));
late_report_deprecation(
self,
&message,
depr.suggestion,
lint,
span,
def_id,
id,
);
late_report_deprecation(self, &message, depr.suggestion, lint, span, 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
10 changes: 0 additions & 10 deletions src/librustc_passes/hir_id_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,6 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
fn visit_id(&mut self, hir_id: HirId) {
let owner = self.owner.expect("no owner");

if hir_id == hir::DUMMY_HIR_ID {
self.error(|| {
format!(
"HirIdValidator: HirId {:?} is invalid",
self.hir_map.node_to_string(hir_id)
)
});
return;
}

if owner != hir_id.owner {
self.error(|| {
format!(
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
8 changes: 0 additions & 8 deletions src/librustc_resolve/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2704,14 +2704,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

fn insert_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime, def: Region) {
if lifetime_ref.hir_id == hir::DUMMY_HIR_ID {
span_bug!(
lifetime_ref.span,
"lifetime reference not renumbered, \
probably a bug in rustc_ast::fold"
);
}

debug!(
"insert_lifetime: {} resolved to {:?} span={:?}",
self.tcx.hir().node_to_string(lifetime_ref.hir_id),
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
3 changes: 0 additions & 3 deletions src/librustc_typeck/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,9 +860,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
&mut self,
expr_hir_id: hir::HirId,
) -> Result<(), MethodError<'tcx>> {
if expr_hir_id == hir::DUMMY_HIR_ID {
return Ok(());
}
let mut duplicates = FxHashSet::default();
let opt_applicable_traits = self.tcx.in_scope_traits(expr_hir_id);
if let Some(applicable_traits) = opt_applicable_traits {
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
18 changes: 8 additions & 10 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,18 +375,16 @@ impl<'tcx> Clean<Option<Vec<GenericBound>>> for InternalSubsts<'tcx> {

impl Clean<Lifetime> for hir::Lifetime {
fn clean(&self, cx: &DocContext<'_>) -> Lifetime {
if self.hir_id != hir::DUMMY_HIR_ID {
let def = cx.tcx.named_region(self.hir_id);
match def {
Some(rl::Region::EarlyBound(_, node_id, _))
| Some(rl::Region::LateBound(_, node_id, _))
| Some(rl::Region::Free(_, node_id)) => {
if let Some(lt) = cx.lt_substs.borrow().get(&node_id).cloned() {
return lt;
}
let def = cx.tcx.named_region(self.hir_id);
match def {
Some(rl::Region::EarlyBound(_, node_id, _))
| Some(rl::Region::LateBound(_, node_id, _))
| Some(rl::Region::Free(_, node_id)) => {
if let Some(lt) = cx.lt_substs.borrow().get(&node_id).cloned() {
return lt;
}
_ => {}
}
_ => {}
}
Lifetime(self.name.ident().to_string())
}
Expand Down
6 changes: 1 addition & 5 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,7 @@ pub fn print_const_expr(cx: &DocContext<'_>, body: hir::BodyId) -> String {

/// Given a type Path, resolve it to a Type using the TyCtxt
pub fn resolve_type(cx: &DocContext<'_>, path: Path, id: hir::HirId) -> Type {
if id == hir::DUMMY_HIR_ID {
debug!("resolve_type({:?})", path);
} else {
debug!("resolve_type({:?},{:?})", path, id);
}
debug!("resolve_type({:?},{:?})", path, id);

let is_generic = match path.res {
Res::PrimTy(p) => return Primitive(PrimitiveType::from(p)),
Expand Down