Auto merge of #96825 - kckeiks:remove-item-like-visitor-trait, r=cjgillot

 Retire `ItemLikeVisitor` trait

Issue #95004
cc `@cjgillot`

Author: bors

compiler/rustc_hir/src/intravisit.rs

@@ -1,7 +1,40 @@
 //! HIR walker for walking the contents of nodes.
 //!
-//! **For an overview of the visitor strategy, see the docs on the
-//! `super::itemlikevisit::ItemLikeVisitor` trait.**
+//! Here are the three available patterns for the visitor strategy,
+//! in roughly the order of desirability:
+//!
+//! 1. **Shallow visit**: Get a simple callback for every item (or item-like thing) in the HIR.
+//!    - Example: find all items with a `#[foo]` attribute on them.
+//!    - How: Use the `hir_crate_items` or `hir_module_items` query to traverse over item-like ids
+//!       (ItemId, TraitItemId, etc.) and use tcx.def_kind and `tcx.hir().item*(id)` to filter and
+//!       access actual item-like thing, respectively.
+//!    - Pro: Efficient; just walks the lists of item ids and gives users control whether to access
+//!       the hir_owners themselves or not.
+//!    - Con: Don't get information about nesting
+//!    - Con: Don't have methods for specific bits of HIR, like "on
+//!      every expr, do this".
+//! 2. **Deep visit**: Want to scan for specific kinds of HIR nodes within
+//!    an item, but don't care about how item-like things are nested
+//!    within one another.
+//!    - Example: Examine each expression to look for its type and do some check or other.
+//!    - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to
+//!      `nested_filter::OnlyBodies` (and implement `nested_visit_map`), and use
+//!      `tcx.hir().deep_visit_all_item_likes(&mut visitor)`. Within your
+//!      `intravisit::Visitor` impl, implement methods like `visit_expr()` (don't forget to invoke
+//!      `intravisit::walk_expr()` to keep walking the subparts).
+//!    - Pro: Visitor methods for any kind of HIR node, not just item-like things.
+//!    - Pro: Integrates well into dependency tracking.
+//!    - Con: Don't get information about nesting between items
+//! 3. **Nested visit**: Want to visit the whole HIR and you care about the nesting between
+//!    item-like things.
+//!    - Example: Lifetime resolution, which wants to bring lifetimes declared on the
+//!      impl into scope while visiting the impl-items, and then back out again.
+//!    - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to
+//!      `nested_filter::All` (and implement `nested_visit_map`). Walk your crate with
+//!      `tcx.hir().walk_toplevel_module(visitor)` invoked on `tcx.hir().krate()`.
+//!    - Pro: Visitor methods for any kind of HIR node, not just item-like things.
+//!    - Pro: Preserves nesting information
+//!    - Con: Does not integrate well into dependency tracking.
 //!
 //! If you have decided to use this visitor, here are some general
 //! notes on how to do so:
@@ -32,43 +65,12 @@
 //! example generator inference, and possibly also HIR borrowck.
 
 use crate::hir::*;
-use crate::itemlikevisit::{ItemLikeVisitor, ParItemLikeVisitor};
+use crate::itemlikevisit::ParItemLikeVisitor;
 use rustc_ast::walk_list;
 use rustc_ast::{Attribute, Label};
 use rustc_span::symbol::{Ident, Symbol};
 use rustc_span::Span;
 
-pub struct DeepVisitor<'v, V> {
-    visitor: &'v mut V,
-}
-
-impl<'v, V> DeepVisitor<'v, V> {
-    pub fn new(base: &'v mut V) -> Self {
-        DeepVisitor { visitor: base }
-    }
-}
-
-impl<'v, 'hir, V> ItemLikeVisitor<'hir> for DeepVisitor<'v, V>
-where
-    V: Visitor<'hir>,
-{
-    fn visit_item(&mut self, item: &'hir Item<'hir>) {
-        self.visitor.visit_item(item);
-    }
-
-    fn visit_trait_item(&mut self, trait_item: &'hir TraitItem<'hir>) {
-        self.visitor.visit_trait_item(trait_item);
-    }
-
-    fn visit_impl_item(&mut self, impl_item: &'hir ImplItem<'hir>) {
-        self.visitor.visit_impl_item(impl_item);
-    }
-
-    fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem<'hir>) {
-        self.visitor.visit_foreign_item(foreign_item);
-    }
-}
-
 pub trait IntoVisitor<'hir> {
     type Visitor: Visitor<'hir>;
     fn into_visitor(&self) -> Self::Visitor;
@@ -315,16 +317,6 @@ pub trait Visitor<'v>: Sized {
         walk_body(self, b);
     }
 
-    /// When invoking `visit_all_item_likes()`, you need to supply an
-    /// item-like visitor. This method converts an "intra-visit"
-    /// visitor into an item-like visitor that walks the entire tree.
-    /// If you use this, you probably don't want to process the
-    /// contents of nested item-like things, since the outer loop will
-    /// visit them as well.
-    fn as_deep_visitor(&mut self) -> DeepVisitor<'_, Self> {
-        DeepVisitor::new(self)
-    }
-
     ///////////////////////////////////////////////////////////////////////////
 
     fn visit_id(&mut self, _hir_id: HirId) {

compiler/rustc_hir/src/itemlikevisit.rs

@@ -1,55 +1,5 @@
 use super::{ForeignItem, ImplItem, Item, TraitItem};
 
-/// The "item-like visitor" defines only the top-level methods
-/// that can be invoked by `Crate::visit_all_item_likes()`. Whether
-/// this trait is the right one to implement will depend on the
-/// overall pattern you need. Here are the three available patterns,
-/// in roughly the order of desirability:
-///
-/// 1. **Shallow visit**: Get a simple callback for every item (or item-like thing) in the HIR.
-///    - Example: find all items with a `#[foo]` attribute on them.
-///    - How: Implement `ItemLikeVisitor` and call `tcx.hir().visit_all_item_likes()`.
-///    - Pro: Efficient; just walks the lists of item-like things, not the nodes themselves.
-///    - Con: Don't get information about nesting
-///    - Con: Don't have methods for specific bits of HIR, like "on
-///      every expr, do this".
-/// 2. **Deep visit**: Want to scan for specific kinds of HIR nodes within
-///    an item, but don't care about how item-like things are nested
-///    within one another.
-///    - Example: Examine each expression to look for its type and do some check or other.
-///    - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to
-///      `nested_filter::OnlyBodies` (and implement `nested_visit_map`), and use
-///      `tcx.hir().visit_all_item_likes(&mut visitor.as_deep_visitor())`. Within your
-///      `intravisit::Visitor` impl, implement methods like `visit_expr()` (don't forget to invoke
-///      `intravisit::walk_expr()` to keep walking the subparts).
-///    - Pro: Visitor methods for any kind of HIR node, not just item-like things.
-///    - Pro: Integrates well into dependency tracking.
-///    - Con: Don't get information about nesting between items
-/// 3. **Nested visit**: Want to visit the whole HIR and you care about the nesting between
-///    item-like things.
-///    - Example: Lifetime resolution, which wants to bring lifetimes declared on the
-///      impl into scope while visiting the impl-items, and then back out again.
-///    - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to
-///      `nested_filter::All` (and implement `nested_visit_map`). Walk your crate with
-///      `tcx.hir().walk_toplevel_module(visitor)` invoked on `tcx.hir().krate()`.
-///    - Pro: Visitor methods for any kind of HIR node, not just item-like things.
-///    - Pro: Preserves nesting information
-///    - Con: Does not integrate well into dependency tracking.
-///
-/// Note: the methods of `ItemLikeVisitor` intentionally have no
-/// defaults, so that as we expand the list of item-like things, we
-/// revisit the various visitors to see if they need to change. This
-/// is harder to do with `intravisit::Visitor`, so when you add a new
-/// `visit_nested_foo()` method, it is recommended that you search for
-/// existing `fn visit_nested` methods to see where changes are
-/// needed.
-pub trait ItemLikeVisitor<'hir> {
-    fn visit_item(&mut self, item: &'hir Item<'hir>);
-    fn visit_trait_item(&mut self, trait_item: &'hir TraitItem<'hir>);
-    fn visit_impl_item(&mut self, impl_item: &'hir ImplItem<'hir>);
-    fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem<'hir>);
-}
-
 /// A parallel variant of `ItemLikeVisitor`.
 pub trait ParItemLikeVisitor<'hir> {
     fn visit_item(&self, item: &'hir Item<'hir>);

compiler/rustc_incremental/src/assert_dep_graph.rs

@@ -75,7 +75,7 @@ pub fn assert_dep_graph(tcx: TyCtxt<'_>) {
             let mut visitor =
                 IfThisChanged { tcx, if_this_changed: vec![], then_this_would_need: vec![] };
             visitor.process_attrs(hir::CRATE_HIR_ID);
-            tcx.hir().visit_all_item_likes(&mut visitor.as_deep_visitor());
+            tcx.hir().deep_visit_all_item_likes(&mut visitor);
             (visitor.if_this_changed, visitor.then_this_would_need)
         };
 

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -14,7 +14,6 @@ use rustc_hir::def_id::{
 };
 use rustc_hir::definitions::DefPathData;
 use rustc_hir::intravisit::{self, Visitor};
-use rustc_hir::itemlikevisit::ItemLikeVisitor;
 use rustc_hir::lang_items;
 use rustc_hir::{AnonConst, GenericParamKind};
 use rustc_index::bit_set::GrowableBitSet;
@@ -453,7 +452,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             return;
         }
 
-        self.tcx.hir().visit_all_item_likes(&mut self.as_deep_visitor());
+        self.tcx.hir().deep_visit_all_item_likes(self);
     }
 
     fn encode_def_path_table(&mut self) {
@@ -2243,26 +2242,16 @@ pub fn provide(providers: &mut Providers) {
         traits_in_crate: |tcx, cnum| {
             assert_eq!(cnum, LOCAL_CRATE);
 
-            #[derive(Default)]
-            struct TraitsVisitor {
-                traits: Vec<DefId>,
-            }
-            impl ItemLikeVisitor<'_> for TraitsVisitor {
-                fn visit_item(&mut self, item: &hir::Item<'_>) {
-                    if let hir::ItemKind::Trait(..) | hir::ItemKind::TraitAlias(..) = item.kind {
-                        self.traits.push(item.def_id.to_def_id());
-                    }
+            let mut traits = Vec::new();
+            for id in tcx.hir().items() {
+                if matches!(tcx.def_kind(id.def_id), DefKind::Trait | DefKind::TraitAlias) {
+                    traits.push(id.def_id.to_def_id())
                 }
-                fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem<'_>) {}
-                fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem<'_>) {}
-                fn visit_foreign_item(&mut self, _foreign_item: &hir::ForeignItem<'_>) {}
             }
 
-            let mut visitor = TraitsVisitor::default();
-            tcx.hir().visit_all_item_likes(&mut visitor);
             // Bring everything into deterministic order.
-            visitor.traits.sort_by_cached_key(|&def_id| tcx.def_path_hash(def_id));
-            tcx.arena.alloc_slice(&visitor.traits)
+            traits.sort_by_cached_key(|&def_id| tcx.def_path_hash(def_id));
+            tcx.arena.alloc_slice(&traits)
         },
 
         ..*providers

compiler/rustc_middle/src/hir/map/mod.rs

@@ -9,7 +9,6 @@ use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
 use rustc_hir::intravisit::{self, Visitor};
-use rustc_hir::itemlikevisit::ItemLikeVisitor;
 use rustc_hir::*;
 use rustc_index::vec::Idx;
 use rustc_middle::hir::nested_filter;
@@ -161,6 +160,10 @@ impl<'hir> Map<'hir> {
         self.tcx.hir_crate_items(()).items.iter().copied()
     }
 
+    pub fn module_items(self, module: LocalDefId) -> impl Iterator<Item = ItemId> + 'hir {
+        self.tcx.hir_module_items(module).items()
+    }
+
     pub fn par_for_each_item(self, f: impl Fn(ItemId) + Sync + Send) {
         par_for_each_in(&self.tcx.hir_crate_items(()).items[..], |id| f(*id));
     }
@@ -603,16 +606,16 @@ impl<'hir> Map<'hir> {
     }
 
     /// Visits all items in the crate in some deterministic (but
-    /// unspecified) order. If you just need to process every item,
-    /// but don't care about nesting, this method is the best choice.
+    /// unspecified) order. If you need to process every item,
+    /// and care about nesting -- usually because your algorithm
+    /// follows lexical scoping rules -- then this method is the best choice.
+    /// If you don't care about nesting, you should use the `tcx.hir_crate_items()` query
+    /// or `items()` instead.
     ///
-    /// If you do care about nesting -- usually because your algorithm
-    /// follows lexical scoping rules -- then you want a different
-    /// approach. You should override `visit_nested_item` in your
-    /// visitor and then call `intravisit::walk_crate` instead.
-    pub fn visit_all_item_likes<V>(self, visitor: &mut V)
+    /// Please see the notes in `intravisit.rs` for more information.
+    pub fn deep_visit_all_item_likes<V>(self, visitor: &mut V)
     where
-        V: itemlikevisit::ItemLikeVisitor<'hir>,
+        V: Visitor<'hir>,
     {
         let krate = self.krate();
         for owner in krate.owners.iter().filter_map(|i| i.as_owner()) {
@@ -643,9 +646,12 @@ impl<'hir> Map<'hir> {
         })
     }
 
-    pub fn visit_item_likes_in_module<V>(self, module: LocalDefId, visitor: &mut V)
+    /// If you don't care about nesting, you should use the
+    /// `tcx.hir_module_items()` query or `module_items()` instead.
+    /// Please see notes in `deep_visit_all_item_likes`.
+    pub fn deep_visit_item_likes_in_module<V>(self, module: LocalDefId, visitor: &mut V)
     where
-        V: ItemLikeVisitor<'hir>,
+        V: Visitor<'hir>,
     {
         let module = self.tcx.hir_module_items(module);
 
@@ -666,7 +672,7 @@ impl<'hir> Map<'hir> {
         }
     }
 
-    pub fn for_each_module(self, f: impl Fn(LocalDefId)) {
+    pub fn for_each_module(self, mut f: impl FnMut(LocalDefId)) {
         let crate_items = self.tcx.hir_crate_items(());
         for module in crate_items.submodules.iter() {
             f(*module)

compiler/rustc_middle/src/hir/nested_filter.rs

@@ -8,7 +8,7 @@ use rustc_hir::intravisit::nested_filter::NestedFilter;
 /// constant arguments of types, e.g. in `let _: [(); /* HERE */];`.
 ///
 /// **This is the most common choice.** A very common pattern is
-/// to use `visit_all_item_likes()` as an outer loop,
+/// to use `deep_visit_all_item_likes()` as an outer loop,
 /// and to have the visitor that visits the contents of each item
 /// using this setting.
 pub struct OnlyBodies(());

compiler/rustc_middle/src/ty/mod.rs

@@ -1995,7 +1995,7 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 
     /// Look up the name of a definition across crates. This does not look at HIR.
-    fn opt_item_name(self, def_id: DefId) -> Option<Symbol> {
+    pub fn opt_item_name(self, def_id: DefId) -> Option<Symbol> {
         if let Some(cnum) = def_id.as_crate_root() {
             Some(self.crate_name(cnum))
         } else {

compiler/rustc_mir_transform/src/lib.rs

@@ -170,7 +170,7 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> {
             intravisit::walk_struct_def(self, v)
         }
     }
-    tcx.hir().visit_all_item_likes(&mut GatherCtors { tcx, set: &mut set }.as_deep_visitor());
+    tcx.hir().deep_visit_all_item_likes(&mut GatherCtors { tcx, set: &mut set });
 
     set
 }

compiler/rustc_passes/src/check_attr.rs

@@ -2390,7 +2390,7 @@ fn check_non_exported_macro_for_invalid_attrs(tcx: TyCtxt<'_>, item: &Item<'_>)
 
 fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
     let check_attr_visitor = &mut CheckAttrVisitor { tcx };
-    tcx.hir().visit_item_likes_in_module(module_def_id, &mut check_attr_visitor.as_deep_visitor());
+    tcx.hir().deep_visit_item_likes_in_module(module_def_id, check_attr_visitor);
     if module_def_id.is_top_level_module() {
         check_attr_visitor.check_attributes(CRATE_HIR_ID, DUMMY_SP, Target::Mod, None);
         check_invalid_crate_level_attr(tcx, tcx.hir().krate_attrs());