Auto merge of rust-lang#90986 - camsteffen:nested-filter, r=cjgillot

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`.

Author: bors

compiler/rustc_ast_lowering/src/index.rs

@@ -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;
@@ -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);

compiler/rustc_ast_lowering/src/lib.rs

@@ -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 {

compiler/rustc_hir/src/intravisit.rs

@@ -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;
@@ -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.
@@ -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
@@ -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>) {

compiler/rustc_hir/src/lib.rs

@@ -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)]

compiler/rustc_incremental/src/assert_dep_graph.rs

@@ -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;
@@ -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>) {

compiler/rustc_incremental/src/persist/dirty_clean.rs

@@ -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;
@@ -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) {

compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs

@@ -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};
@@ -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>) {

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/find_anon_type.rs

@@ -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};
 
@@ -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>) {
@@ -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) {

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs

@@ -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,
@@ -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,