revamp Visitor with a single method for controlling nested visits

Author: nikomatsakis

src/librustc/hir/intravisit.rs

@@ -70,9 +70,42 @@ impl<'a> FnKind<'a> {
 /// Specifies what nested things a visitor wants to visit. Currently there are
 /// two modes: `OnlyBodies` descends into item bodies, but not into nested
 /// items; `All` descends into item bodies and nested items.
-pub enum NestedVisitMode {
-    OnlyBodies,
-    All
+pub enum NestedVisitorMap<'this, 'tcx: 'this> {
+    /// 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.
+    None,
+
+    /// Do not visit nested item-like things, but visit nested things
+    /// that are inside of an item-like.
+    ///
+    /// **This is the default mode.**
+    OnlyBodies(&'this Map<'tcx>),
+
+    /// Visit all nested things, including item-likes.
+    All(&'this Map<'tcx>),
+}
+
+impl<'this, 'tcx> NestedVisitorMap<'this, 'tcx> {
+    /// Returns the map to use for an "intra item-like" thing (if any).
+    /// e.g., function body.
+    pub fn intra(self) -> Option<&'this Map<'tcx>> {
+        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.
+    pub fn inter(self) -> Option<&'this Map<'tcx>> {
+        match self {
+            NestedVisitorMap::None => None,
+            NestedVisitorMap::OnlyBodies(_) => None,
+            NestedVisitorMap::All(map) => Some(map),
+        }
+    }
 }
 
 /// Each method of the Visitor trait is a hook to be potentially
@@ -109,13 +142,7 @@ 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) -> Option<&Map<'v>>;
-
-    /// Specifies what things nested things this visitor wants to visit. By
-    /// default, bodies will be visited, but not nested items.
-    fn nested_visit_mode(&mut self) -> NestedVisitMode {
-        NestedVisitMode::OnlyBodies
-    }
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'v>;
 
     /// Invoked when a nested item is encountered. By default does
     /// nothing unless you override `nested_visit_map` to return
@@ -127,8 +154,7 @@ pub trait Visitor<'v> : Sized {
     /// but cannot supply a `Map`; see `nested_visit_map` for advice.
     #[allow(unused_variables)]
     fn visit_nested_item(&mut self, id: ItemId) {
-        let opt_item = map_for_item(self)
-                           .map(|map| map.expect_item(id.id));
+        let opt_item = self.nested_visit_map().inter().map(|map| map.expect_item(id.id));
         if let Some(item) = opt_item {
             self.visit_item(item);
         }
@@ -139,8 +165,7 @@ pub trait Visitor<'v> : Sized {
     /// method.
     #[allow(unused_variables)]
     fn visit_nested_impl_item(&mut self, id: ImplItemId) {
-        let opt_item = map_for_item(self)
-                           .map(|map| map.impl_item(id));
+        let opt_item = self.nested_visit_map().inter().map(|map| map.impl_item(id));
         if let Some(item) = opt_item {
             self.visit_impl_item(item);
         }
@@ -151,8 +176,7 @@ pub trait Visitor<'v> : Sized {
     /// `nested_visit_map` to return `Some(_)`, in which case it will walk the
     /// body.
     fn visit_body(&mut self, id: ExprId) {
-        let opt_expr = map_for_body(self)
-                           .map(|map| map.expr(id));
+        let opt_expr = self.nested_visit_map().intra().map(|map| map.expr(id));
         if let Some(expr) = opt_expr {
             self.visit_expr(expr);
         }
@@ -302,18 +326,6 @@ pub trait Visitor<'v> : Sized {
     }
 }
 
-fn map_for_body<'v, V: Visitor<'v>>(visitor: &mut V) -> Option<&Map<'v>> {
-    visitor.nested_visit_map()
-}
-
-fn map_for_item<'v, V: Visitor<'v>>(visitor: &mut V) -> Option<&Map<'v>> {
-    match visitor.nested_visit_mode() {
-        NestedVisitMode::OnlyBodies => None,
-        NestedVisitMode::All => Some(visitor.nested_visit_map()
-                                     .expect("NestedVisitMode::All without nested_visit_map"))
-    }
-}
-
 pub fn walk_opt_name<'v, V: Visitor<'v>>(visitor: &mut V, span: Span, opt_name: Option<Name>) {
     if let Some(name) = opt_name {
         visitor.visit_name(span, name);
@@ -1061,8 +1073,8 @@ impl<'a, 'ast> IdRangeComputingVisitor<'a, 'ast> {
 }
 
 impl<'a, 'ast> Visitor<'ast> for IdRangeComputingVisitor<'a, 'ast> {
-    fn nested_visit_map(&mut self) -> Option<&Map<'ast>> {
-        Some(&self.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'ast> {
+        NestedVisitorMap::OnlyBodies(&self.map)
     }
 
     fn visit_id(&mut self, id: NodeId) {

src/librustc/hir/map/collector.rs

@@ -10,7 +10,7 @@
 
 use super::*;
 
-use hir::intravisit::Visitor;
+use hir::intravisit::{Visitor, NestedVisitorMap};
 use hir::def_id::DefId;
 use middle::cstore::InlinedItem;
 use std::iter::repeat;
@@ -91,7 +91,7 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
     /// deep walking so that we walk nested items in the context of
     /// their outer items.
 
-    fn nested_visit_map(&mut self) -> Option<&map::Map<'ast>> {
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'ast> {
         panic!("visit_nested_xxx must be manually implemented in this visitor")
     }
 

src/librustc/hir/map/def_collector.rs

@@ -11,7 +11,7 @@
 use hir::map::definitions::*;
 
 use hir;
-use hir::intravisit;
+use hir::intravisit::{self, Visitor, NestedVisitorMap};
 use hir::def_id::{CRATE_DEF_INDEX, DefId, DefIndex};
 
 use middle::cstore::InlinedItem;
@@ -326,9 +326,10 @@ impl<'a> visit::Visitor for DefCollector<'a> {
 }
 
 // We walk the HIR rather than the AST when reading items from metadata.
-impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'ast>> {
-        None
+impl<'ast> Visitor<'ast> for DefCollector<'ast> {
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'ast> {
+        // note however that we override `visit_body` below
+        NestedVisitorMap::None
     }
 
     fn visit_body(&mut self, id: hir::ExprId) {

src/librustc/lint/context.rs

@@ -791,12 +791,8 @@ impl<'a, 'tcx> hir_visit::Visitor<'tcx> for LateContext<'a, 'tcx> {
     /// Because lints are scoped lexically, we want to walk nested
     /// items in the context of the outer item, so enable
     /// deep-walking.
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
-    }
-
-    fn nested_visit_mode(&mut self) -> hir_visit::NestedVisitMode {
-        hir_visit::NestedVisitMode::All
+    fn nested_visit_map<'this>(&'this mut self) -> hir_visit::NestedVisitorMap<'this, 'tcx> {
+        hir_visit::NestedVisitorMap::All(&self.tcx.map)
     }
 
     fn visit_item(&mut self, it: &'tcx hir::Item) {
@@ -1113,8 +1109,8 @@ struct IdVisitor<'a, 'b: 'a, 'tcx: 'a+'b> {
 
 // Output any lints that were previously added to the session.
 impl<'a, 'b, 'tcx> hir_visit::Visitor<'tcx> for IdVisitor<'a, 'b, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.cx.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> hir_visit::NestedVisitorMap<'this, 'tcx> {
+        hir_visit::NestedVisitorMap::OnlyBodies(&self.cx.tcx.map)
     }
 
     fn visit_id(&mut self, id: ast::NodeId) {

src/librustc/middle/dataflow.rs

@@ -193,7 +193,9 @@ fn build_nodeid_to_index(decl: Option<&hir::FnDecl>,
         let mut formals = Formals { entry: entry, index: index };
         intravisit::walk_fn_decl(&mut formals, decl);
         impl<'a, 'v> intravisit::Visitor<'v> for Formals<'a> {
-            fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'v>> { None }
+            fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'v> {
+                panic!("should not encounter fn bodies or items")
+            }
 
             fn visit_pat(&mut self, p: &hir::Pat) {
                 self.index.entry(p.id).or_insert(vec![]).push(self.entry);

src/librustc/middle/dead.rs

@@ -15,7 +15,7 @@
 use dep_graph::DepNode;
 use hir::map as ast_map;
 use hir::{self, PatKind};
-use hir::intravisit::{self, Visitor, NestedVisitMode};
+use hir::intravisit::{self, Visitor, NestedVisitorMap};
 use hir::itemlikevisit::ItemLikeVisitor;
 
 use middle::privacy;
@@ -221,8 +221,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.tcx.map)
     }
 
     fn visit_variant_data(&mut self, def: &'tcx hir::VariantData, _: ast::Name,
@@ -510,12 +510,10 @@ impl<'a, 'tcx> Visitor<'tcx> for DeadVisitor<'a, 'tcx> {
     /// on inner functions when the outer function is already getting
     /// an error. We could do this also by checking the parents, but
     /// this is how the code is setup and it seems harmless enough.
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::All(&self.tcx.map)
     }
 
-    fn nested_visit_mode(&mut self) -> NestedVisitMode { NestedVisitMode::All }
-
     fn visit_item(&mut self, item: &'tcx hir::Item) {
         if self.should_warn_about_item(item) {
             self.warn_dead_code(

src/librustc/middle/effect.rs

@@ -21,7 +21,7 @@ use syntax::ast;
 use syntax_pos::Span;
 use hir::{self, PatKind};
 use hir::def::Def;
-use hir::intravisit::{self, FnKind, Visitor};
+use hir::intravisit::{self, FnKind, Visitor, NestedVisitorMap};
 
 #[derive(Copy, Clone)]
 struct UnsafeContext {
@@ -93,8 +93,8 @@ impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.tcx.map)
     }
 
     fn visit_fn(&mut self, fn_kind: FnKind<'tcx>, fn_decl: &'tcx hir::FnDecl,

src/librustc/middle/intrinsicck.rs

@@ -19,7 +19,7 @@ use ty::layout::{LayoutError, Pointer, SizeSkeleton};
 use syntax::abi::Abi::RustIntrinsic;
 use syntax::ast;
 use syntax_pos::Span;
-use hir::intravisit::{self, Visitor, FnKind};
+use hir::intravisit::{self, Visitor, FnKind, NestedVisitorMap};
 use hir;
 
 pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
@@ -117,8 +117,8 @@ impl<'a, 'gcx, 'tcx> ExprVisitor<'a, 'gcx, 'tcx> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for ItemVisitor<'a, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.tcx.map)
     }
 
     // const, static and N in [T; N].
@@ -163,8 +163,8 @@ impl<'a, 'tcx> Visitor<'tcx> for ItemVisitor<'a, 'tcx> {
 }
 
 impl<'a, 'gcx, 'tcx> Visitor<'gcx> for ExprVisitor<'a, 'gcx, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'gcx>> {
-        Some(&self.infcx.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'gcx> {
+        NestedVisitorMap::OnlyBodies(&self.infcx.tcx.map)
     }
 
     fn visit_expr(&mut self, expr: &'gcx hir::Expr) {

src/librustc/middle/liveness.rs

@@ -128,7 +128,7 @@ use syntax_pos::Span;
 use hir::Expr;
 use hir;
 use hir::print::{expr_to_string, block_to_string};
-use hir::intravisit::{self, Visitor, FnKind};
+use hir::intravisit::{self, Visitor, FnKind, NestedVisitorMap};
 
 /// For use with `propagate_through_loop`.
 enum LoopKind<'a> {
@@ -183,9 +183,10 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt) -> String {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for IrMaps<'a, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.tcx.map)
     }
+
     fn visit_fn(&mut self, fk: FnKind<'tcx>, fd: &'tcx hir::FnDecl,
                 b: hir::ExprId, s: Span, id: NodeId) {
         visit_fn(self, fk, fd, b, s, id);
@@ -352,9 +353,10 @@ impl<'a, 'tcx> IrMaps<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.ir.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.ir.tcx.map)
     }
+
     fn visit_fn(&mut self, _: FnKind<'tcx>, _: &'tcx hir::FnDecl,
                 _: hir::ExprId, _: Span, _: NodeId) {
         // do not check contents of nested fns

src/librustc/middle/reachable.rs

@@ -28,7 +28,7 @@ use syntax::abi::Abi;
 use syntax::ast;
 use syntax::attr;
 use hir;
-use hir::intravisit::{Visitor};
+use hir::intravisit::{Visitor, NestedVisitorMap};
 use hir::itemlikevisit::ItemLikeVisitor;
 use hir::intravisit;
 
@@ -89,8 +89,8 @@ struct ReachableContext<'a, 'tcx: 'a> {
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for ReachableContext<'a, 'tcx> {
-    fn nested_visit_map(&mut self) -> Option<&hir::map::Map<'tcx>> {
-        Some(&self.tcx.map)
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.tcx.map)
     }
 
     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {