From 872ac73f59465ccaf63f74fd12785c583d3aa0a6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Mar 2025 17:02:33 +1100 Subject: [PATCH 1/2] Move `visit_id` calls. In `walk_item`, we call `visit_id` on every item kind. For most of them we do it directly in `walk_item`. But for `ItemKind::Mod`, `ItemKind::Enum`, and `ItemKind::Use` we instead do it in the `walk_*` function called (via the `visit_*` function) from `walk_item`. I can see no reason for this inconsistency, so this commit makes those three cases like all the other cases, moving the `visit_id` calls into `walk_item`. This also avoids the need for a few `HirId` arguments. --- compiler/rustc_hir/src/intravisit.rs | 25 +++++++++--------------- compiler/rustc_lint/src/late.rs | 2 +- compiler/rustc_middle/src/hir/map.rs | 2 +- compiler/rustc_passes/src/input_stats.rs | 4 ++-- src/librustdoc/html/render/span_map.rs | 2 +- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 41eb5b45bd1e0..0aa07d96b3de7 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -316,8 +316,8 @@ pub trait Visitor<'v>: Sized { fn visit_ident(&mut self, ident: Ident) -> Self::Result { walk_ident(self, ident) } - fn visit_mod(&mut self, m: &'v Mod<'v>, _s: Span, n: HirId) -> Self::Result { - walk_mod(self, m, n) + fn visit_mod(&mut self, m: &'v Mod<'v>, _s: Span, _n: HirId) -> Self::Result { + walk_mod(self, m) } fn visit_foreign_item(&mut self, i: &'v ForeignItem<'v>) -> Self::Result { walk_foreign_item(self, i) @@ -464,8 +464,8 @@ pub trait Visitor<'v>: Sized { fn visit_field_def(&mut self, s: &'v FieldDef<'v>) -> Self::Result { walk_field_def(self, s) } - fn visit_enum_def(&mut self, enum_definition: &'v EnumDef<'v>, item_id: HirId) -> Self::Result { - walk_enum_def(self, enum_definition, item_id) + fn visit_enum_def(&mut self, enum_definition: &'v EnumDef<'v>) -> Self::Result { + walk_enum_def(self, enum_definition) } fn visit_variant(&mut self, v: &'v Variant<'v>) -> Self::Result { walk_variant(self, v) @@ -539,6 +539,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: visit_opt!(visitor, visit_name, orig_name); } ItemKind::Use(ref path, _) => { + try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_use(path, item.hir_id())); } ItemKind::Static(ref typ, _, body) => { @@ -566,7 +567,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: try_visit!(visitor.visit_id(item.hir_id())); } ItemKind::Mod(ref module) => { - // `visit_mod()` takes care of visiting the `Item`'s `HirId`. + try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_mod(module, item.span, item.hir_id())); } ItemKind::ForeignMod { abi: _, items } => { @@ -587,9 +588,9 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: try_visit!(visitor.visit_generics(generics)); } ItemKind::Enum(ref enum_definition, ref generics) => { + try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_generics(generics)); - // `visit_enum_def()` takes care of visiting the `Item`'s `HirId`. - try_visit!(visitor.visit_enum_def(enum_definition, item.hir_id())); + try_visit!(visitor.visit_enum_def(enum_definition)); } ItemKind::Impl(Impl { constness: _, @@ -638,12 +639,7 @@ pub fn walk_ident<'v, V: Visitor<'v>>(visitor: &mut V, ident: Ident) -> V::Resul visitor.visit_name(ident.name) } -pub fn walk_mod<'v, V: Visitor<'v>>( - visitor: &mut V, - module: &'v Mod<'v>, - mod_hir_id: HirId, -) -> V::Result { - try_visit!(visitor.visit_id(mod_hir_id)); +pub fn walk_mod<'v, V: Visitor<'v>>(visitor: &mut V, module: &'v Mod<'v>) -> V::Result { walk_list!(visitor, visit_nested_item, module.item_ids.iter().copied()); V::Result::output() } @@ -1145,7 +1141,6 @@ pub fn walk_use<'v, V: Visitor<'v>>( path: &'v UsePath<'v>, hir_id: HirId, ) -> V::Result { - try_visit!(visitor.visit_id(hir_id)); let UsePath { segments, ref res, span } = *path; for &res in res { try_visit!(visitor.visit_path(&Path { segments, res, span }, hir_id)); @@ -1326,9 +1321,7 @@ pub fn walk_field_def<'v, V: Visitor<'v>>( pub fn walk_enum_def<'v, V: Visitor<'v>>( visitor: &mut V, enum_definition: &'v EnumDef<'v>, - item_id: HirId, ) -> V::Result { - try_visit!(visitor.visit_id(item_id)); walk_list!(visitor, visit_variant, enum_definition.variants); V::Result::output() } diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index d22515d62d60e..23d6efa050836 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -74,7 +74,7 @@ impl<'tcx, T: LateLintPass<'tcx>> LateContextAndPass<'tcx, T> { fn process_mod(&mut self, m: &'tcx hir::Mod<'tcx>, n: HirId) { lint_callback!(self, check_mod, m, n); - hir_visit::walk_mod(self, m, n); + hir_visit::walk_mod(self, m); } } diff --git a/compiler/rustc_middle/src/hir/map.rs b/compiler/rustc_middle/src/hir/map.rs index fad8c7dcbcbcb..c85af81ee25bd 100644 --- a/compiler/rustc_middle/src/hir/map.rs +++ b/compiler/rustc_middle/src/hir/map.rs @@ -1356,7 +1356,7 @@ impl<'hir> Visitor<'hir> for ItemCollector<'hir> { self.submodules.push(item.owner_id); // A module collector does not recurse inside nested modules. if self.crate_collector { - intravisit::walk_mod(self, module, item.hir_id()); + intravisit::walk_mod(self, module); } } else { intravisit::walk_item(self, item) diff --git a/compiler/rustc_passes/src/input_stats.rs b/compiler/rustc_passes/src/input_stats.rs index 92ea49f18e5d1..8ff11197e09de 100644 --- a/compiler/rustc_passes/src/input_stats.rs +++ b/compiler/rustc_passes/src/input_stats.rs @@ -255,9 +255,9 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> { hir_visit::walk_body(self, b); } - fn visit_mod(&mut self, m: &'v hir::Mod<'v>, _s: Span, n: HirId) { + fn visit_mod(&mut self, m: &'v hir::Mod<'v>, _s: Span, _n: HirId) { self.record("Mod", None, m); - hir_visit::walk_mod(self, m, n) + hir_visit::walk_mod(self, m) } fn visit_foreign_item(&mut self, i: &'v hir::ForeignItem<'v>) { diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index ce9c42c01cc73..bd84272618003 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -253,7 +253,7 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { // If it's a "mod foo {}", we want to look to its documentation page. self.extract_info_from_hir_id(id); } - intravisit::walk_mod(self, m, id); + intravisit::walk_mod(self, m); } fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { From 8a981241fe86dfac2e8212638c87bbaf1ae53264 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 7 Mar 2025 17:37:23 +1100 Subject: [PATCH 2/2] Factor out repeated `visit_id` calls. Every `ItemKind` now has one. --- compiler/rustc_hir/src/intravisit.rs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 0aa07d96b3de7..be9f490edcceb 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -532,29 +532,25 @@ pub fn walk_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v Param<'v>) -> } pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V::Result { + try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_ident(item.ident)); match item.kind { ItemKind::ExternCrate(orig_name) => { - try_visit!(visitor.visit_id(item.hir_id())); visit_opt!(visitor, visit_name, orig_name); } ItemKind::Use(ref path, _) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_use(path, item.hir_id())); } ItemKind::Static(ref typ, _, body) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_ty_unambig(typ)); try_visit!(visitor.visit_nested_body(body)); } ItemKind::Const(ref typ, ref generics, body) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_ty_unambig(typ)); try_visit!(visitor.visit_generics(generics)); try_visit!(visitor.visit_nested_body(body)); } ItemKind::Fn { sig, generics, body: body_id, .. } => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_fn( FnKind::ItemFn(item.ident, generics, sig.header), sig.decl, @@ -563,19 +559,14 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: item.owner_id.def_id, )); } - ItemKind::Macro(..) => { - try_visit!(visitor.visit_id(item.hir_id())); - } + ItemKind::Macro(..) => {} ItemKind::Mod(ref module) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_mod(module, item.span, item.hir_id())); } ItemKind::ForeignMod { abi: _, items } => { - try_visit!(visitor.visit_id(item.hir_id())); walk_list!(visitor, visit_foreign_item_ref, items); } ItemKind::GlobalAsm { asm: _, fake_body } => { - try_visit!(visitor.visit_id(item.hir_id())); // Visit the fake body, which contains the asm statement. // Therefore we should not visit the asm statement again // outside of the body, or some visitors won't have their @@ -583,12 +574,10 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: try_visit!(visitor.visit_nested_body(fake_body)); } ItemKind::TyAlias(ref ty, ref generics) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_ty_unambig(ty)); try_visit!(visitor.visit_generics(generics)); } ItemKind::Enum(ref enum_definition, ref generics) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_generics(generics)); try_visit!(visitor.visit_enum_def(enum_definition)); } @@ -603,7 +592,6 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: self_ty, items, }) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_generics(generics)); visit_opt!(visitor, visit_trait_ref, of_trait); try_visit!(visitor.visit_ty_unambig(self_ty)); @@ -612,17 +600,14 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: ItemKind::Struct(ref struct_definition, ref generics) | ItemKind::Union(ref struct_definition, ref generics) => { try_visit!(visitor.visit_generics(generics)); - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_variant_data(struct_definition)); } ItemKind::Trait(.., ref generics, bounds, trait_item_refs) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_generics(generics)); walk_list!(visitor, visit_param_bound, bounds); walk_list!(visitor, visit_trait_item_ref, trait_item_refs); } ItemKind::TraitAlias(ref generics, bounds) => { - try_visit!(visitor.visit_id(item.hir_id())); try_visit!(visitor.visit_generics(generics)); walk_list!(visitor, visit_param_bound, bounds); }