Skip to content

Commit 7341cad

Browse files
CentrilCentril
authored
Rollup merge of rust-lang#71116 - marmeladema:dummy-hir-id-removal, r=eddyb
Entirely remove `DUMMY_HIR_ID` 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.
2 parents 8da5952 + c15e13a commit 7341cad

File tree

15 files changed

+79
-94
lines changed

15 files changed

+79
-94
lines changed

‎src/librustc_ast_lowering/lib.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ struct LoweringContext<'a, 'hir: 'a> {
168168

169169
current_hir_id_owner: Vec<(LocalDefId, u32)>,
170170
item_local_id_counters: NodeMap<u32>,
171-
node_id_to_hir_id: IndexVec<NodeId, hir::HirId>,
171+
node_id_to_hir_id: IndexVec<NodeId, Option<hir::HirId>>,
172172

173173
allow_try_trait: Option<Lrc<[Symbol]>>,
174174
allow_gen_future: Option<Lrc<[Symbol]>>,
@@ -522,15 +522,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
522522
}
523523

524524
self.lower_node_id(CRATE_NODE_ID);
525-
debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == hir::CRATE_HIR_ID);
525+
debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == Some(hir::CRATE_HIR_ID));
526526

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

530530
let module = self.lower_mod(&c.module);
531531
let attrs = self.lower_attrs(&c.attrs);
532532
let body_ids = body_ids(&self.bodies);
533-
let proc_macros = c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id]).collect();
533+
let proc_macros =
534+
c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id].unwrap()).collect();
534535

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

@@ -571,26 +572,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
571572
ast_node_id: NodeId,
572573
alloc_hir_id: impl FnOnce(&mut Self) -> hir::HirId,
573574
) -> hir::HirId {
574-
if ast_node_id == DUMMY_NODE_ID {
575-
return hir::DUMMY_HIR_ID;
576-
}
575+
assert_ne!(ast_node_id, DUMMY_NODE_ID);
577576

578577
let min_size = ast_node_id.as_usize() + 1;
579578

580579
if min_size > self.node_id_to_hir_id.len() {
581-
self.node_id_to_hir_id.resize(min_size, hir::DUMMY_HIR_ID);
580+
self.node_id_to_hir_id.resize(min_size, None);
582581
}
583582

584-
let existing_hir_id = self.node_id_to_hir_id[ast_node_id];
585-
586-
if existing_hir_id == hir::DUMMY_HIR_ID {
583+
if let Some(existing_hir_id) = self.node_id_to_hir_id[ast_node_id] {
584+
existing_hir_id
585+
} else {
587586
// Generate a new `HirId`.
588587
let hir_id = alloc_hir_id(self);
589-
self.node_id_to_hir_id[ast_node_id] = hir_id;
588+
self.node_id_to_hir_id[ast_node_id] = Some(hir_id);
590589

591590
hir_id
592-
} else {
593-
existing_hir_id
594591
}
595592
}
596593

‎src/librustc_hir/definitions.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
pub use crate::def_id::DefPathHash;
88
use crate::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
99
use crate::hir;
10-
use crate::hir_id::DUMMY_HIR_ID;
1110

1211
use rustc_ast::ast;
1312
use rustc_ast::crate_disambiguator::CrateDisambiguator;
@@ -87,7 +86,7 @@ pub struct Definitions {
8786
node_id_to_def_id: FxHashMap<ast::NodeId, LocalDefId>,
8887
def_id_to_node_id: IndexVec<LocalDefId, ast::NodeId>,
8988

90-
pub(super) node_id_to_hir_id: IndexVec<ast::NodeId, hir::HirId>,
89+
pub(super) node_id_to_hir_id: IndexVec<ast::NodeId, Option<hir::HirId>>,
9190
/// The reverse mapping of `node_id_to_hir_id`.
9291
pub(super) hir_id_to_node_id: FxHashMap<hir::HirId, ast::NodeId>,
9392

@@ -345,8 +344,7 @@ impl Definitions {
345344
#[inline]
346345
pub fn as_local_hir_id(&self, def_id: DefId) -> Option<hir::HirId> {
347346
if let Some(def_id) = def_id.as_local() {
348-
let hir_id = self.local_def_id_to_hir_id(def_id);
349-
if hir_id != DUMMY_HIR_ID { Some(hir_id) } else { None }
347+
Some(self.local_def_id_to_hir_id(def_id))
350348
} else {
351349
None
352350
}
@@ -359,11 +357,22 @@ impl Definitions {
359357

360358
#[inline]
361359
pub fn node_id_to_hir_id(&self, node_id: ast::NodeId) -> hir::HirId {
360+
self.node_id_to_hir_id[node_id].unwrap()
361+
}
362+
363+
#[inline]
364+
pub fn opt_node_id_to_hir_id(&self, node_id: ast::NodeId) -> Option<hir::HirId> {
362365
self.node_id_to_hir_id[node_id]
363366
}
364367

365368
#[inline]
366369
pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId {
370+
let node_id = self.def_id_to_node_id[id];
371+
self.node_id_to_hir_id[node_id].unwrap()
372+
}
373+
374+
#[inline]
375+
pub fn opt_local_def_id_to_hir_id(&self, id: LocalDefId) -> Option<hir::HirId> {
367376
let node_id = self.def_id_to_node_id[id];
368377
self.node_id_to_hir_id[node_id]
369378
}
@@ -470,7 +479,10 @@ impl Definitions {
470479

471480
/// Initializes the `ast::NodeId` to `HirId` mapping once it has been generated during
472481
/// AST to HIR lowering.
473-
pub fn init_node_id_to_hir_id_mapping(&mut self, mapping: IndexVec<ast::NodeId, hir::HirId>) {
482+
pub fn init_node_id_to_hir_id_mapping(
483+
&mut self,
484+
mapping: IndexVec<ast::NodeId, Option<hir::HirId>>,
485+
) {
474486
assert!(
475487
self.node_id_to_hir_id.is_empty(),
476488
"trying to initialize `NodeId` -> `HirId` mapping twice"
@@ -481,7 +493,7 @@ impl Definitions {
481493
self.hir_id_to_node_id = self
482494
.node_id_to_hir_id
483495
.iter_enumerated()
484-
.map(|(node_id, &hir_id)| (hir_id, node_id))
496+
.filter_map(|(node_id, &hir_id)| hir_id.map(|hir_id| (hir_id, node_id)))
485497
.collect();
486498
}
487499

‎src/librustc_hir/hir_id.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,4 @@ pub const CRATE_HIR_ID: HirId = HirId {
4545
local_id: ItemLocalId::from_u32(0),
4646
};
4747

48-
pub const DUMMY_HIR_ID: HirId =
49-
HirId { owner: LocalDefId { local_def_index: CRATE_DEF_INDEX }, local_id: DUMMY_ITEM_LOCAL_ID };
50-
5148
pub const DUMMY_ITEM_LOCAL_ID: ItemLocalId = ItemLocalId::MAX;

‎src/librustc_middle/hir/map/collector.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -250,23 +250,16 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
250250
None => format!("{:?}", node),
251251
};
252252

253-
let forgot_str = if hir_id == hir::DUMMY_HIR_ID {
254-
format!("\nMaybe you forgot to lower the node id {:?}?", node_id)
255-
} else {
256-
String::new()
257-
};
258-
259253
span_bug!(
260254
span,
261255
"inconsistent DepNode at `{:?}` for `{}`: \
262-
current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?}){}",
256+
current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?})",
263257
self.source_map.span_to_string(span),
264258
node_str,
265259
self.definitions.def_path(self.current_dep_node_owner).to_string_no_crate(),
266260
self.current_dep_node_owner,
267261
self.definitions.def_path(hir_id.owner).to_string_no_crate(),
268262
hir_id.owner,
269-
forgot_str,
270263
)
271264
}
272265
}

‎src/librustc_middle/hir/map/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,21 @@ impl<'hir> Map<'hir> {
214214
self.tcx.definitions.node_id_to_hir_id(node_id)
215215
}
216216

217+
#[inline]
218+
pub fn opt_node_id_to_hir_id(&self, node_id: NodeId) -> Option<HirId> {
219+
self.tcx.definitions.opt_node_id_to_hir_id(node_id)
220+
}
221+
217222
#[inline]
218223
pub fn local_def_id_to_hir_id(&self, def_id: LocalDefId) -> HirId {
219224
self.tcx.definitions.local_def_id_to_hir_id(def_id)
220225
}
221226

227+
#[inline]
228+
pub fn opt_local_def_id_to_hir_id(&self, def_id: LocalDefId) -> Option<HirId> {
229+
self.tcx.definitions.opt_local_def_id_to_hir_id(def_id)
230+
}
231+
222232
pub fn def_kind(&self, hir_id: HirId) -> Option<DefKind> {
223233
let node = self.find(hir_id)?;
224234

‎src/librustc_middle/middle/stability.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ fn late_report_deprecation(
215215
suggestion: Option<Symbol>,
216216
lint: &'static Lint,
217217
span: Span,
218-
def_id: DefId,
219218
hir_id: HirId,
220219
) {
221220
if span.in_derive_expansion() {
@@ -229,9 +228,6 @@ fn late_report_deprecation(
229228
}
230229
diag.emit()
231230
});
232-
if hir_id == hir::DUMMY_HIR_ID {
233-
span_bug!(span, "emitted a {} lint with dummy HIR id: {:?}", lint.name, def_id);
234-
}
235231
}
236232

237233
/// Result of `TyCtxt::eval_stability`.
@@ -296,7 +292,7 @@ impl<'tcx> TyCtxt<'tcx> {
296292
if !skip {
297293
let (message, lint) =
298294
deprecation_message(&depr_entry.attr, &self.def_path_str(def_id));
299-
late_report_deprecation(self, &message, None, lint, span, def_id, id);
295+
late_report_deprecation(self, &message, None, lint, span, id);
300296
}
301297
};
302298
}
@@ -319,15 +315,7 @@ impl<'tcx> TyCtxt<'tcx> {
319315
if let Some(depr) = &stability.rustc_depr {
320316
let (message, lint) =
321317
rustc_deprecation_message(depr, &self.def_path_str(def_id));
322-
late_report_deprecation(
323-
self,
324-
&message,
325-
depr.suggestion,
326-
lint,
327-
span,
328-
def_id,
329-
id,
330-
);
318+
late_report_deprecation(self, &message, depr.suggestion, lint, span, id);
331319
}
332320
}
333321
}

‎src/librustc_middle/ty/context.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,13 +1126,16 @@ impl<'tcx> TyCtxt<'tcx> {
11261126

11271127
let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default();
11281128
for (k, v) in resolutions.trait_map {
1129-
let hir_id = definitions.node_id_to_hir_id(k);
1130-
let map = trait_map.entry(hir_id.owner).or_default();
1131-
let v = v
1132-
.into_iter()
1133-
.map(|tc| tc.map_import_ids(|id| definitions.node_id_to_hir_id(id)))
1134-
.collect();
1135-
map.insert(hir_id.local_id, StableVec::new(v));
1129+
// FIXME(#71104) Should really be using just `node_id_to_hir_id` but
1130+
// some `NodeId` do not seem to have a corresponding HirId.
1131+
if let Some(hir_id) = definitions.opt_node_id_to_hir_id(k) {
1132+
let map = trait_map.entry(hir_id.owner).or_default();
1133+
let v = v
1134+
.into_iter()
1135+
.map(|tc| tc.map_import_ids(|id| definitions.node_id_to_hir_id(id)))
1136+
.collect();
1137+
map.insert(hir_id.local_id, StableVec::new(v));
1138+
}
11361139
}
11371140

11381141
GlobalCtxt {

‎src/librustc_passes/hir_id_validator.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -143,16 +143,6 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
143143
fn visit_id(&mut self, hir_id: HirId) {
144144
let owner = self.owner.expect("no owner");
145145

146-
if hir_id == hir::DUMMY_HIR_ID {
147-
self.error(|| {
148-
format!(
149-
"HirIdValidator: HirId {:?} is invalid",
150-
self.hir_map.node_to_string(hir_id)
151-
)
152-
});
153-
return;
154-
}
155-
156146
if owner != hir_id.owner {
157147
self.error(|| {
158148
format!(

‎src/librustc_privacy/lib.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,12 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
928928

929929
let macro_module_def_id =
930930
ty::DefIdTree::parent(self.tcx, self.tcx.hir().local_def_id(md.hir_id)).unwrap();
931-
let mut module_id = match self.tcx.hir().as_local_hir_id(macro_module_def_id) {
931+
// FIXME(#71104) Should really be using just `as_local_hir_id` but
932+
// some `DefId` do not seem to have a corresponding HirId.
933+
let hir_id = macro_module_def_id
934+
.as_local()
935+
.and_then(|def_id| self.tcx.hir().opt_local_def_id_to_hir_id(def_id));
936+
let mut module_id = match hir_id {
932937
Some(module_id) if self.tcx.hir().is_hir_id_module(module_id) => module_id,
933938
// `module_id` doesn't correspond to a `mod`, return early (#63164, #65252).
934939
_ => return,

‎src/librustc_resolve/late/lifetimes.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,14 +2704,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
27042704
}
27052705

27062706
fn insert_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime, def: Region) {
2707-
if lifetime_ref.hir_id == hir::DUMMY_HIR_ID {
2708-
span_bug!(
2709-
lifetime_ref.span,
2710-
"lifetime reference not renumbered, \
2711-
probably a bug in rustc_ast::fold"
2712-
);
2713-
}
2714-
27152707
debug!(
27162708
"insert_lifetime: {} resolved to {:?} span={:?}",
27172709
self.tcx.hir().node_to_string(lifetime_ref.hir_id),

‎src/librustc_save_analysis/dump_visitor.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,14 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> {
225225
collector.visit_pat(&arg.pat);
226226

227227
for (id, ident, ..) in collector.collected_idents {
228-
let hir_id = self.tcx.hir().node_id_to_hir_id(id);
229-
let typ = match self.save_ctxt.tables.node_type_opt(hir_id) {
230-
Some(s) => s.to_string(),
231-
None => continue,
232-
};
228+
// FIXME(#71104) Should really be using just `node_id_to_hir_id` but
229+
// some `NodeId` do not seem to have a corresponding HirId.
230+
let hir_id = self.tcx.hir().opt_node_id_to_hir_id(id);
231+
let typ =
232+
match hir_id.and_then(|hir_id| self.save_ctxt.tables.node_type_opt(hir_id)) {
233+
Some(s) => s.to_string(),
234+
None => continue,
235+
};
233236
if !self.span.filter_generated(ident.span) {
234237
let id = id_from_node_id(id, &self.save_ctxt);
235238
let span = self.span_from_span(ident.span);

‎src/librustc_typeck/check/method/probe.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -865,9 +865,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
865865
&mut self,
866866
expr_hir_id: hir::HirId,
867867
) -> Result<(), MethodError<'tcx>> {
868-
if expr_hir_id == hir::DUMMY_HIR_ID {
869-
return Ok(());
870-
}
871868
let mut duplicates = FxHashSet::default();
872869
let opt_applicable_traits = self.tcx.in_scope_traits(expr_hir_id);
873870
if let Some(applicable_traits) = opt_applicable_traits {

‎src/librustc_typeck/check/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,11 @@ fn has_typeck_tables(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
838838
return tcx.has_typeck_tables(outer_def_id);
839839
}
840840

841-
if let Some(id) = tcx.hir().as_local_hir_id(def_id) {
841+
// FIXME(#71104) Should really be using just `as_local_hir_id` but
842+
// some `LocalDefId` do not seem to have a corresponding HirId.
843+
if let Some(id) =
844+
def_id.as_local().and_then(|def_id| tcx.hir().opt_local_def_id_to_hir_id(def_id))
845+
{
842846
primary_body_of(tcx, id).is_some()
843847
} else {
844848
false

‎src/librustdoc/clean/mod.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -375,18 +375,16 @@ impl<'tcx> Clean<Option<Vec<GenericBound>>> for InternalSubsts<'tcx> {
375375

376376
impl Clean<Lifetime> for hir::Lifetime {
377377
fn clean(&self, cx: &DocContext<'_>) -> Lifetime {
378-
if self.hir_id != hir::DUMMY_HIR_ID {
379-
let def = cx.tcx.named_region(self.hir_id);
380-
match def {
381-
Some(rl::Region::EarlyBound(_, node_id, _))
382-
| Some(rl::Region::LateBound(_, node_id, _))
383-
| Some(rl::Region::Free(_, node_id)) => {
384-
if let Some(lt) = cx.lt_substs.borrow().get(&node_id).cloned() {
385-
return lt;
386-
}
378+
let def = cx.tcx.named_region(self.hir_id);
379+
match def {
380+
Some(rl::Region::EarlyBound(_, node_id, _))
381+
| Some(rl::Region::LateBound(_, node_id, _))
382+
| Some(rl::Region::Free(_, node_id)) => {
383+
if let Some(lt) = cx.lt_substs.borrow().get(&node_id).cloned() {
384+
return lt;
387385
}
388-
_ => {}
389386
}
387+
_ => {}
390388
}
391389
Lifetime(self.name.ident().to_string())
392390
}

‎src/librustdoc/clean/utils.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -580,11 +580,7 @@ pub fn print_const_expr(cx: &DocContext<'_>, body: hir::BodyId) -> String {
580580

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

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

0 commit comments

Comments
 (0)