Skip to content
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

Retire ItemLikeVisitor trait #96825

Merged
merged 30 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bd2b210
Remove CheckConstTraitVisitor
kckeiks May 3, 2022
0026034
replace usage of visit_item_likes_in_modules with hir_module_items query
kckeiks May 3, 2022
52f833a
remove LifeSeeder
kckeiks May 3, 2022
dab0e75
remove DiagnosticItemCollector
kckeiks May 3, 2022
fb73ae2
remove ItemLikeVisitor impl for EntryContext
kckeiks May 3, 2022
b1f0209
optimize find_item to fetch Item only when needed
kckeiks May 3, 2022
0ef16fe
remove OuterVisitor
kckeiks May 4, 2022
45c37da
remove LayoutTest
kckeiks May 4, 2022
0a029e2
remove CollectPrivateImplItemsVisitor
kckeiks May 4, 2022
90685c6
check def_kind before fetching item
kckeiks May 4, 2022
eea16de
replace hir().def_kind for def_kind query in rustc_passes
kckeiks May 4, 2022
e8ef5bf
remove TraitVisitor
kckeiks May 4, 2022
e166409
remove Visitor impl for PrivateItemsInPublicInterfacesChecker
kckeiks May 5, 2022
cdba1dc
add module_items
kckeiks May 7, 2022
52d721c
remove TestItemNamesVisitor
kckeiks May 7, 2022
885b90b
remove DebuggerVisualizerCollector
kckeiks May 7, 2022
0b7dd95
remove HirVisitor
kckeiks May 7, 2022
93616dd
remove ItemLikeVisitor and DeepVisitor
kckeiks May 7, 2022
7e44078
update comments about visitor strategy
kckeiks May 7, 2022
f975d05
rename visit item-like methods
kckeiks May 7, 2022
2e98879
change back to using tcx.hir() visit-item method
kckeiks May 7, 2022
cad1fd2
update rustdoc code to use new method name
kckeiks May 7, 2022
df11942
change for_each_module's parameter to FnMut
kckeiks May 9, 2022
91223ac
use for_each_module instead of iterating over Item's
kckeiks May 9, 2022
433a5f1
avoid converting to DefId from LocalDefId when using query
kckeiks May 9, 2022
17e86d9
remove unnecessary methods from HirIdValidator
kckeiks May 9, 2022
f1c256d
remove redundant branch
kckeiks May 9, 2022
959636d
avoid fetching HIR when handling Impl assoc items
kckeiks May 12, 2022
f77658b
use opt_item_name to pattern match items with names
kckeiks May 12, 2022
48fd666
allocate string only when error will be emitted
kckeiks May 14, 2022
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
80 changes: 36 additions & 44 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
50 changes: 0 additions & 50 deletions compiler/rustc_hir/src/itemlikevisit.rs
Original file line number Diff line number Diff line change
@@ -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>);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/assert_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};

Expand Down
25 changes: 7 additions & 18 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
30 changes: 18 additions & 12 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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);

Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/nested_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2384,7 +2384,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());
Expand Down
Loading