Skip to content

Commit

Permalink
Auto merge of rust-lang#90986 - camsteffen:nested-filter, r=cjgillot
Browse files Browse the repository at this point in the history
Replace `NestedVisitorMap` with generic `NestedFilter`

This is an attempt to make the `intravisit::Visitor` API simpler and "more const" with regard to nested visiting.

With this change, `intravisit::Visitor` does not visit nested things by default, unless you specify `type NestedFilter = nested_filter::OnlyBodies` (or `All`). `nested_visit_map` returns `Self::Map` instead of `NestedVisitorMap<Self::Map>`. It panics by default (unreachable if `type NestedFilter` is omitted).

One somewhat trixty thing here is that `nested_filter::{OnlyBodies, All}` live in `rustc_middle` so that they may have `type Map = map::Map` and so that `impl Visitor`s never need to specify `type Map` - it has a default of `Self::NestedFilter::Map`.
  • Loading branch information
bors committed Jan 17, 2022
2 parents a34c079 + be6d693 commit ee5d8d3
Show file tree
Hide file tree
Showing 110 changed files with 387 additions and 946 deletions.
8 changes: 1 addition & 7 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_data_structures::sorted_map::SortedMap;
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::definitions;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::*;
use rustc_index::vec::{Idx, IndexVec};
use rustc_session::Session;
Expand Down Expand Up @@ -101,16 +101,10 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
}

impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
type Map = !;

/// Because we want to track parent items and so forth, enable
/// deep walking so that we walk nested items in the context of
/// their outer items.
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
panic!("`visit_nested_xxx` must be manually implemented in this visitor");
}

fn visit_nested_item(&mut self, item: ItemId) {
debug!("visit_nested_item: {:?}", item);
self.insert_nested(item.def_id);
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2436,12 +2436,6 @@ fn lifetimes_from_impl_trait_bounds(
}

impl<'r, 'v> intravisit::Visitor<'v> for ImplTraitLifetimeCollector<'r> {
type Map = intravisit::ErasedMap<'v>;

fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::None
}

fn visit_generic_args(&mut self, span: Span, parameters: &'v hir::GenericArgs<'v>) {
// Don't collect elided lifetimes used inside of `Fn()` syntax.
if parameters.parenthesized {
Expand Down
134 changes: 54 additions & 80 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,87 +160,44 @@ impl<'hir> Map<'hir> for ! {
}
}

/// An erased version of `Map<'hir>`, using dynamic dispatch.
/// NOTE: This type is effectively only usable with `NestedVisitorMap::None`.
pub struct ErasedMap<'hir>(&'hir dyn Map<'hir>);
pub mod nested_filter {
use super::Map;

impl<'hir> Map<'hir> for ErasedMap<'hir> {
fn find(&self, _: HirId) -> Option<Node<'hir>> {
None
}
fn body(&self, id: BodyId) -> &'hir Body<'hir> {
self.0.body(id)
}
fn item(&self, id: ItemId) -> &'hir Item<'hir> {
self.0.item(id)
}
fn trait_item(&self, id: TraitItemId) -> &'hir TraitItem<'hir> {
self.0.trait_item(id)
}
fn impl_item(&self, id: ImplItemId) -> &'hir ImplItem<'hir> {
self.0.impl_item(id)
}
fn foreign_item(&self, id: ForeignItemId) -> &'hir ForeignItem<'hir> {
self.0.foreign_item(id)
/// Specifies what nested things a visitor wants to visit. The most
/// common choice is `OnlyBodies`, which will cause the visitor to
/// visit fn bodies for fns that it encounters, but skip over nested
/// item-like things.
///
/// See the comments on `ItemLikeVisitor` for more details on the overall
/// visit strategy.
pub trait NestedFilter<'hir> {
type Map: Map<'hir>;

/// Whether the visitor visits nested "item-like" things.
/// E.g., item, impl-item.
const INTER: bool;
/// Whether the visitor visits "intra item-like" things.
/// E.g., function body, closure, `AnonConst`
const INTRA: bool;
}
}

/// Specifies what nested things a visitor wants to visit. The most
/// common choice is `OnlyBodies`, which will cause the visitor to
/// visit fn bodies for fns that it encounters, but skip over nested
/// item-like things.
///
/// See the comments on `ItemLikeVisitor` for more details on the overall
/// visit strategy.
pub enum NestedVisitorMap<M> {
/// Do not visit any nested things. When you add a new
/// "non-nested" thing, you will want to audit such uses to see if
/// they remain valid.
///
/// Use this if you are only walking some particular kind of tree
/// (i.e., a type, or fn signature) and you don't want to thread a
/// HIR map around.
None,

/// Do not visit nested item-like things, but visit nested things
/// that are inside of an item-like.
///
/// **This is the most common choice.** A very common pattern is
/// to use `visit_all_item_likes()` as an outer loop,
/// and to have the visitor that visits the contents of each item
/// using this setting.
OnlyBodies(M),

/// Visits all nested things, including item-likes.
///
/// **This is an unusual choice.** It is used when you want to
/// process everything within their lexical context. Typically you
/// kick off the visit by doing `walk_krate()`.
All(M),
}

impl<M> NestedVisitorMap<M> {
/// Returns the map to use for an "intra item-like" thing (if any).
/// E.g., function body.
fn intra(self) -> Option<M> {
match self {
NestedVisitorMap::None => None,
NestedVisitorMap::OnlyBodies(map) => Some(map),
NestedVisitorMap::All(map) => Some(map),
}
}

/// Returns the map to use for an "item-like" thing (if any).
/// E.g., item, impl-item.
fn inter(self) -> Option<M> {
match self {
NestedVisitorMap::None => None,
NestedVisitorMap::OnlyBodies(_) => None,
NestedVisitorMap::All(map) => Some(map),
}
pub struct None(());
impl NestedFilter<'_> for None {
type Map = !;
const INTER: bool = false;
const INTRA: bool = false;
}
}

use nested_filter::NestedFilter;

/// Each method of the Visitor trait is a hook to be potentially
/// overridden. Each method's default implementation recursively visits
/// the substructure of the input via the corresponding `walk` method;
Expand All @@ -258,7 +215,9 @@ impl<M> NestedVisitorMap<M> {
/// to monitor future changes to `Visitor` in case a new method with a
/// new default implementation gets introduced.)
pub trait Visitor<'v>: Sized {
type Map: Map<'v>;
// this type should not be overridden, it exists for convenient usage as `Self::Map`
type Map: Map<'v> = <Self::NestedFilter as NestedFilter<'v>>::Map;
type NestedFilter: NestedFilter<'v> = nested_filter::None;

///////////////////////////////////////////////////////////////////////////
// Nested items.
Expand All @@ -279,7 +238,12 @@ pub trait Visitor<'v>: Sized {
/// `panic!()`. This way, if a new `visit_nested_XXX` variant is
/// added in the future, we will see the panic in your code and
/// fix it appropriately.
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map>;
fn nested_visit_map(&mut self) -> Self::Map {
panic!(
"nested_visit_map must be implemented or consider using \
`type NestedFilter = nested_filter::None` (the default)"
);
}

/// Invoked when a nested item is encountered. By default does
/// nothing unless you override `nested_visit_map` to return other than
Expand All @@ -290,41 +254,51 @@ pub trait Visitor<'v>: Sized {
/// reason to override this method is if you want a nested pattern
/// but cannot supply a `Map`; see `nested_visit_map` for advice.
fn visit_nested_item(&mut self, id: ItemId) {
let opt_item = self.nested_visit_map().inter().map(|map| map.item(id));
walk_list!(self, visit_item, opt_item);
if Self::NestedFilter::INTER {
let item = self.nested_visit_map().item(id);
self.visit_item(item);
}
}

/// Like `visit_nested_item()`, but for trait items. See
/// `visit_nested_item()` for advice on when to override this
/// method.
fn visit_nested_trait_item(&mut self, id: TraitItemId) {
let opt_item = self.nested_visit_map().inter().map(|map| map.trait_item(id));
walk_list!(self, visit_trait_item, opt_item);
if Self::NestedFilter::INTER {
let item = self.nested_visit_map().trait_item(id);
self.visit_trait_item(item);
}
}

/// Like `visit_nested_item()`, but for impl items. See
/// `visit_nested_item()` for advice on when to override this
/// method.
fn visit_nested_impl_item(&mut self, id: ImplItemId) {
let opt_item = self.nested_visit_map().inter().map(|map| map.impl_item(id));
walk_list!(self, visit_impl_item, opt_item);
if Self::NestedFilter::INTER {
let item = self.nested_visit_map().impl_item(id);
self.visit_impl_item(item);
}
}

/// Like `visit_nested_item()`, but for foreign items. See
/// `visit_nested_item()` for advice on when to override this
/// method.
fn visit_nested_foreign_item(&mut self, id: ForeignItemId) {
let opt_item = self.nested_visit_map().inter().map(|map| map.foreign_item(id));
walk_list!(self, visit_foreign_item, opt_item);
if Self::NestedFilter::INTER {
let item = self.nested_visit_map().foreign_item(id);
self.visit_foreign_item(item);
}
}

/// Invoked to visit the body of a function, method or closure. Like
/// visit_nested_item, does nothing by default unless you override
/// `nested_visit_map` to return other than `None`, in which case it will walk
/// the body.
fn visit_nested_body(&mut self, id: BodyId) {
let opt_body = self.nested_visit_map().intra().map(|map| map.body(id));
walk_list!(self, visit_body, opt_body);
if Self::NestedFilter::INTRA {
let body = self.nested_visit_map().body(id);
self.visit_body(body);
}
}

fn visit_param(&mut self, param: &'v Param<'v>) {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//!
//! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/hir.html
#![feature(associated_type_defaults)]
#![feature(const_btree_new)]
#![feature(crate_visibility_modifier)]
#![feature(once_cell)]
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_incremental/src/assert_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ use rustc_data_structures::graph::implementation::{Direction, NodeIndex, INCOMIN
use rustc_graphviz as dot;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{self, Visitor};
use rustc_middle::dep_graph::{
DepGraphQuery, DepKind, DepNode, DepNodeExt, DepNodeFilter, EdgeFilter,
};
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::Span;
Expand Down Expand Up @@ -173,10 +173,10 @@ impl<'tcx> IfThisChanged<'tcx> {
}

impl<'tcx> Visitor<'tcx> for IfThisChanged<'tcx> {
type Map = Map<'tcx>;
type NestedFilter = nested_filter::OnlyBodies;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.tcx.hir())
fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}

fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_incremental/src/persist/dirty_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_hir::Node as HirNode;
use rustc_hir::{ImplItemKind, ItemKind as HirItem, TraitItemKind};
use rustc_middle::dep_graph::{label_strs, DepNode, DepNodeExt};
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::Span;
Expand Down Expand Up @@ -472,10 +472,10 @@ impl<'tcx> FindAllAttrs<'tcx> {
}

impl<'tcx> intravisit::Visitor<'tcx> for FindAllAttrs<'tcx> {
type Map = Map<'tcx>;
type NestedFilter = nested_filter::All;

fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::All(self.tcx.hir())
fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}

fn visit_attribute(&mut self, _: hir::HirId, attr: &'tcx Attribute) {
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Namespace};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, MatchSource, Pat};
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter;
use rustc_middle::infer::unify_key::ConstVariableOriginKind;
use rustc_middle::ty::print::Print;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
Expand Down Expand Up @@ -83,10 +83,10 @@ impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> {
}

impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
type NestedFilter = nested_filter::OnlyBodies;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.infcx.tcx.hir())
fn nested_visit_map(&mut self) -> Self::Map {
self.infcx.tcx.hir()
}

fn visit_local(&mut self, local: &'tcx Local<'tcx>) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use rustc_hir as hir;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::Node;
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_lifetime as rl;
use rustc_middle::ty::{self, Region, TyCtxt};

Expand Down Expand Up @@ -84,10 +85,10 @@ struct FindNestedTypeVisitor<'tcx> {
}

impl<'tcx> Visitor<'tcx> for FindNestedTypeVisitor<'tcx> {
type Map = Map<'tcx>;
type NestedFilter = nested_filter::OnlyBodies;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.tcx.hir())
fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}

fn visit_ty(&mut self, arg: &'tcx hir::Ty<'tcx>) {
Expand Down Expand Up @@ -208,10 +209,10 @@ struct TyPathVisitor<'tcx> {
}

impl<'tcx> Visitor<'tcx> for TyPathVisitor<'tcx> {
type Map = Map<'tcx>;
type NestedFilter = nested_filter::OnlyBodies;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Map<'tcx>> {
NestedVisitorMap::OnlyBodies(self.tcx.hir())
fn nested_visit_map(&mut self) -> Map<'tcx> {
self.tcx.hir()
}

fn visit_lifetime(&mut self, lifetime: &hir::Lifetime) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::traits::{ObligationCauseCode, UnifyReceiverContext};
use rustc_data_structures::stable_set::FxHashSet;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorReported};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{walk_ty, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{walk_ty, Visitor};
use rustc_hir::{self as hir, GenericBound, Item, ItemKind, Lifetime, LifetimeName, Node, TyKind};
use rustc_middle::ty::{
self, AssocItemContainer, RegionKind, StaticLifetimeVisitor, Ty, TyCtxt, TypeFoldable,
Expand Down Expand Up @@ -575,12 +575,6 @@ impl<'tcx> TypeVisitor<'tcx> for TraitObjectVisitor {
pub(super) struct HirTraitObjectVisitor<'a>(pub(super) &'a mut Vec<Span>, pub(super) DefId);

impl<'a, 'tcx> Visitor<'tcx> for HirTraitObjectVisitor<'a> {
type Map = ErasedMap<'tcx>;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}

fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) {
if let TyKind::TraitObject(
poly_trait_refs,
Expand Down
Loading

0 comments on commit ee5d8d3

Please sign in to comment.