refactor Visitor into ItemLikeVisitor and intravisit::Visitor

There are now three patterns (shallow, deep, and nested visit).  These
are described in detail on the docs in `itemlikevisit::ItemLikeVisitor`.

src/librustc/dep_graph/mod.rs

@@ -25,5 +25,5 @@ pub use self::dep_node::WorkProductId;
 pub use self::graph::DepGraph;
 pub use self::graph::WorkProduct;
 pub use self::query::DepGraphQuery;
-pub use self::visit::visit_all_items_in_krate;
+pub use self::visit::visit_all_item_likes_in_krate;
 pub use self::raii::DepTask;

src/librustc/dep_graph/visit.rs

@@ -10,22 +10,21 @@
 
 use hir;
 use hir::def_id::DefId;
-use hir::intravisit::Visitor;
+use hir::itemlikevisit::ItemLikeVisitor;
 use ty::TyCtxt;
 
 use super::dep_node::DepNode;
 
-
 /// Visit all the items in the krate in some order. When visiting a
 /// particular item, first create a dep-node by calling `dep_node_fn`
 /// and push that onto the dep-graph stack of tasks, and also create a
 /// read edge from the corresponding AST node. This is used in
 /// compiler passes to automatically record the item that they are
 /// working on.
-pub fn visit_all_items_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                                                mut dep_node_fn: F,
-                                                visitor: &mut V)
-    where F: FnMut(DefId) -> DepNode<DefId>, V: Visitor<'tcx>
+pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                                     mut dep_node_fn: F,
+                                                     visitor: &mut V)
+    where F: FnMut(DefId) -> DepNode<DefId>, V: ItemLikeVisitor<'tcx>
 {
     struct TrackingVisitor<'visit, 'tcx: 'visit, F: 'visit, V: 'visit> {
         tcx: TyCtxt<'visit, 'tcx, 'tcx>,
@@ -33,8 +32,8 @@ pub fn visit_all_items_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         visitor: &'visit mut V
     }
 
-    impl<'visit, 'tcx, F, V> Visitor<'tcx> for TrackingVisitor<'visit, 'tcx, F, V>
-        where F: FnMut(DefId) -> DepNode<DefId>, V: Visitor<'tcx>
+    impl<'visit, 'tcx, F, V> ItemLikeVisitor<'tcx> for TrackingVisitor<'visit, 'tcx, F, V>
+        where F: FnMut(DefId) -> DepNode<DefId>, V: ItemLikeVisitor<'tcx>
     {
         fn visit_item(&mut self, i: &'tcx hir::Item) {
             let item_def_id = self.tcx.map.local_def_id(i.id);
@@ -54,5 +53,5 @@ pub fn visit_all_items_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         dep_node_fn: &mut dep_node_fn,
         visitor: visitor
     };
-    krate.visit_all_items(&mut tracking_visitor)
+    krate.visit_all_item_likes(&mut tracking_visitor)
 }

src/librustc/hir/intravisit.rs

@@ -8,7 +8,15 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! HIR walker. Each overridden visit method has full control over what
+//! HIR walker for walking the contents of nodes.
+//!
+//! **For an overview of the visitor strategy, see the docs on the
+//! `super::itemlikevisit::ItemLikeVisitor` trait.**
+//!
+//! If you have decided to use this visitor, here are some general
+//! notes on how to do it:
+//!
+//! Each overridden visit method has full control over what
 //! happens with its node, it can do its own traversal of the node's children,
 //! call `intravisit::walk_*` to apply the default traversal algorithm, or prevent
 //! deeper traversal by doing nothing.
@@ -30,6 +38,7 @@ use syntax::ast::{NodeId, CRATE_NODE_ID, Name, Attribute};
 use syntax::codemap::Spanned;
 use syntax_pos::Span;
 use hir::*;
+use super::itemlikevisit::DeepVisitor;
 
 use std::cmp;
 use std::u32;
@@ -78,10 +87,9 @@ pub trait Visitor<'v> : Sized {
 
     /// Invoked when a nested item is encountered. By default, does
     /// nothing. If you want a deep walk, you need to override to
-    /// fetch the item contents. But most of the time, it is easier
-    /// (and better) to invoke `Crate::visit_all_items`, which visits
-    /// all items in the crate in some order (but doesn't respect
-    /// nesting).
+    /// fetch the item contents. But most of the time, it is easier to
+    /// use either the "shallow" or "deep" visit patterns described on
+    /// `itemlikevisit::ItemLikeVisitor`.
     #[allow(unused_variables)]
     fn visit_nested_item(&mut self, id: ItemId) {
     }
@@ -92,6 +100,16 @@ pub trait Visitor<'v> : Sized {
         walk_item(self, i)
     }
 
+    /// When invoking `visit_all_item_likes()`, you need to supply an
+    /// item-like visitor.  This method converts a "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<'s>(&'s mut self) -> DeepVisitor<'s, Self> {
+        DeepVisitor::new(self)
+    }
+
     ///////////////////////////////////////////////////////////////////////////
 
     fn visit_id(&mut self, _node_id: NodeId) {

src/librustc/hir/itemlikevisit.rs

@@ -0,0 +1,79 @@
+// Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use super::Item;
+use super::intravisit::Visitor;
+
+/// The "item-like visitor" 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.visit_all_item_likes_in_krate()`.
+///    - Pro: Efficient; just walks the lists of item-like things, not the nodes themselves.
+///    - Pro: Integrates well into dependency tracking.
+///    - 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 use
+///      `tcx.visit_all_item_likes_in_krate(visitor.as_deep_visitor())`. Within
+///      your `intravisit::Visitor` impl, implement methods like
+///      `visit_expr()`; don't forget to invoke
+///      `intravisit::walk_visit_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 `visit_nested_foo()` foo methods
+///      as needed. Walk your crate with `intravisit::walk_crate()` invoked on `tcx.map.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);
+}
+
+pub struct DeepVisitor<'v, V: 'v> {
+    visitor: &'v mut V,
+}
+
+impl<'v, 'hir, V> DeepVisitor<'v, V>
+    where V: Visitor<'hir> + '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) {
+        self.visitor.visit_item(item);
+    }
+}

src/librustc/hir/mod.rs

@@ -68,6 +68,7 @@ pub mod check_attr;
 pub mod def;
 pub mod def_id;
 pub mod intravisit;
+pub mod itemlikevisit;
 pub mod lowering;
 pub mod map;
 pub mod pat_util;
@@ -438,8 +439,8 @@ impl Crate {
     /// 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_items<'hir, V>(&'hir self, visitor: &mut V)
-        where V: intravisit::Visitor<'hir>
+    pub fn visit_all_item_likes<'hir, V>(&'hir self, visitor: &mut V)
+        where V: itemlikevisit::ItemLikeVisitor<'hir>
     {
         for (_, item) in &self.items {
             visitor.visit_item(item);

src/librustc/middle/dead.rs

@@ -16,6 +16,7 @@ use dep_graph::DepNode;
 use hir::map as ast_map;
 use hir::{self, pat_util, PatKind};
 use hir::intravisit::{self, Visitor};
+use hir::itemlikevisit::ItemLikeVisitor;
 
 use middle::privacy;
 use ty::{self, TyCtxt};
@@ -333,7 +334,7 @@ struct LifeSeeder {
     worklist: Vec<ast::NodeId>
 }
 
-impl<'v> Visitor<'v> for LifeSeeder {
+impl<'v> ItemLikeVisitor<'v> for LifeSeeder {
     fn visit_item(&mut self, item: &hir::Item) {
         let allow_dead_code = has_allow_dead_code_or_lang_attr(&item.attrs);
         if allow_dead_code {
@@ -388,7 +389,7 @@ fn create_and_seed_worklist<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     let mut life_seeder = LifeSeeder {
         worklist: worklist
     };
-    krate.visit_all_items(&mut life_seeder);
+    krate.visit_all_item_likes(&mut life_seeder);
 
     return life_seeder.worklist;
 }

src/librustc/middle/effect.rs

@@ -235,5 +235,5 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
         unsafe_context: UnsafeContext::new(SafeContext),
     };
 
-    tcx.map.krate().visit_all_items(&mut visitor);
+    tcx.map.krate().visit_all_item_likes(&mut visitor.as_deep_visitor());
 }

src/librustc/middle/entry.rs

@@ -18,7 +18,7 @@ use syntax::attr;
 use syntax::entry::EntryPointType;
 use syntax_pos::Span;
 use hir::{Item, ItemFn};
-use hir::intravisit::Visitor;
+use hir::itemlikevisit::ItemLikeVisitor;
 
 struct EntryContext<'a, 'tcx: 'a> {
     session: &'a Session,
@@ -39,7 +39,7 @@ struct EntryContext<'a, 'tcx: 'a> {
     non_main_fns: Vec<(NodeId, Span)> ,
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for EntryContext<'a, 'tcx> {
+impl<'a, 'tcx> ItemLikeVisitor<'tcx> for EntryContext<'a, 'tcx> {
     fn visit_item(&mut self, item: &'tcx Item) {
         let def_id = self.map.local_def_id(item.id);
         let def_key = self.map.def_key(def_id);
@@ -74,7 +74,7 @@ pub fn find_entry_point(session: &Session, ast_map: &ast_map::Map) {
         non_main_fns: Vec::new(),
     };
 
-    ast_map.krate().visit_all_items(&mut ctxt);
+    ast_map.krate().visit_all_item_likes(&mut ctxt);
 
     configure_main(&mut ctxt);
 }

src/librustc/middle/intrinsicck.rs

@@ -26,7 +26,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
     let mut visitor = ItemVisitor {
         tcx: tcx
     };
-    tcx.visit_all_items_in_krate(DepNode::IntrinsicCheck, &mut visitor);
+    tcx.visit_all_item_likes_in_krate(DepNode::IntrinsicCheck, &mut visitor.as_deep_visitor());
 }
 
 struct ItemVisitor<'a, 'tcx: 'a> {

src/librustc/middle/lang_items.rs

@@ -31,7 +31,7 @@ use util::nodemap::FxHashMap;
 
 use syntax::ast;
 use syntax::parse::token::InternedString;
-use hir::intravisit::Visitor;
+use hir::itemlikevisit::ItemLikeVisitor;
 use hir;
 
 // The actual lang items defined come at the end of this file in one handy table.
@@ -149,7 +149,7 @@ struct LanguageItemCollector<'a, 'tcx: 'a> {
     item_refs: FxHashMap<&'static str, usize>,
 }
 
-impl<'a, 'v, 'tcx> Visitor<'v> for LanguageItemCollector<'a, 'tcx> {
+impl<'a, 'v, 'tcx> ItemLikeVisitor<'v> for LanguageItemCollector<'a, 'tcx> {
     fn visit_item(&mut self, item: &hir::Item) {
         if let Some(value) = extract(&item.attrs) {
             let item_index = self.item_refs.get(&value[..]).cloned();
@@ -219,7 +219,7 @@ impl<'a, 'tcx> LanguageItemCollector<'a, 'tcx> {
     }
 
     pub fn collect_local_language_items(&mut self, krate: &hir::Crate) {
-        krate.visit_all_items(self);
+        krate.visit_all_item_likes(self);
     }
 
     pub fn collect_external_language_items(&mut self) {

src/librustc/middle/liveness.rs

@@ -196,7 +196,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for IrMaps<'a, 'tcx> {
 
 pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
     let _task = tcx.dep_graph.in_task(DepNode::Liveness);
-    tcx.map.krate().visit_all_items(&mut IrMaps::new(tcx));
+    tcx.map.krate().visit_all_item_likes(&mut IrMaps::new(tcx).as_deep_visitor());
     tcx.sess.abort_if_errors();
 }
 

src/librustc/middle/reachable.rs

@@ -29,6 +29,7 @@ use syntax::ast;
 use syntax::attr;
 use hir;
 use hir::intravisit::Visitor;
+use hir::itemlikevisit::ItemLikeVisitor;
 use hir::intravisit;
 
 // Returns true if the given set of generics implies that the item it's
@@ -324,7 +325,7 @@ struct CollectPrivateImplItemsVisitor<'a> {
     worklist: &'a mut Vec<ast::NodeId>,
 }
 
-impl<'a, 'v> Visitor<'v> for CollectPrivateImplItemsVisitor<'a> {
+impl<'a, 'v> ItemLikeVisitor<'v> for CollectPrivateImplItemsVisitor<'a> {
     fn visit_item(&mut self, item: &hir::Item) {
         // We need only trait impls here, not inherent impls, and only non-exported ones
         if let hir::ItemImpl(.., Some(_), _, ref impl_items) = item.node {
@@ -364,7 +365,7 @@ pub fn find_reachable<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             access_levels: access_levels,
             worklist: &mut reachable_context.worklist,
         };
-        tcx.map.krate().visit_all_items(&mut collect_private_impl_items);
+        tcx.map.krate().visit_all_item_likes(&mut collect_private_impl_items);
     }
 
     // Step 2: Mark all symbols that the symbols on the worklist touch.

src/librustc/middle/region.rs

@@ -1235,7 +1235,7 @@ pub fn resolve_crate(sess: &Session, map: &ast_map::Map) -> RegionMaps {
             },
             terminating_scopes: NodeSet()
         };
-        krate.visit_all_items(&mut visitor);
+        krate.visit_all_item_likes(&mut visitor.as_deep_visitor());
     }
     return maps;
 }

src/librustc/middle/resolve_lifetime.rs

@@ -119,7 +119,7 @@ pub fn krate(sess: &Session,
         late_bound: NodeMap(),
     };
     sess.track_errors(|| {
-        krate.visit_all_items(&mut LifetimeContext {
+        intravisit::walk_crate(&mut LifetimeContext {
             sess: sess,
             hir_map: hir_map,
             map: &mut map,
@@ -127,14 +127,23 @@ pub fn krate(sess: &Session,
             def_map: def_map,
             trait_ref_hack: false,
             labels_in_fn: vec![],
-        });
+        }, krate);
     })?;
     Ok(map)
 }
 
 impl<'a, 'tcx, 'v> Visitor<'v> for LifetimeContext<'a, 'tcx> {
+    // Override the nested functions -- lifetimes follow lexical scope,
+    // so it's convenient to walk the tree in lexical order.
+
+    fn visit_nested_item(&mut self, id: hir::ItemId) {
+        let item = self.hir_map.expect_item(id.id);
+        self.visit_item(item)
+    }
+
     fn visit_item(&mut self, item: &hir::Item) {
-        assert!(self.labels_in_fn.is_empty());
+        // Save labels for nested items.
+        let saved_labels_in_fn = replace(&mut self.labels_in_fn, vec![]);
 
         // Items always introduce a new root scope
         self.with(RootScope, |_, this| {
@@ -175,7 +184,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for LifetimeContext<'a, 'tcx> {
         });
 
         // Done traversing the item; remove any labels it created
-        self.labels_in_fn.truncate(0);
+        self.labels_in_fn = saved_labels_in_fn;
     }
 
     fn visit_foreign_item(&mut self, item: &hir::ForeignItem) {

src/librustc/middle/weak_lang_items.rs

@@ -50,7 +50,7 @@ pub fn check_crate(krate: &hir::Crate,
 
     {
         let mut cx = Context { sess: sess, items: items };
-        krate.visit_all_items(&mut cx);
+        krate.visit_all_item_likes(&mut cx.as_deep_visitor());
     }
     verify(sess, items);
 }

src/librustc/ty/mod.rs

@@ -50,7 +50,7 @@ use syntax_pos::{DUMMY_SP, Span};
 use rustc_const_math::ConstInt;
 
 use hir;
-use hir::intravisit::Visitor;
+use hir::itemlikevisit::ItemLikeVisitor;
 
 pub use self::sty::{Binder, DebruijnIndex};
 pub use self::sty::{BuiltinBound, BuiltinBounds};
@@ -2695,12 +2695,12 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         self.mk_region(ty::ReScope(self.region_maps.node_extent(id)))
     }
 
-    pub fn visit_all_items_in_krate<V,F>(self,
-                                         dep_node_fn: F,
-                                         visitor: &mut V)
-        where F: FnMut(DefId) -> DepNode<DefId>, V: Visitor<'gcx>
+    pub fn visit_all_item_likes_in_krate<V,F>(self,
+                                              dep_node_fn: F,
+                                              visitor: &mut V)
+        where F: FnMut(DefId) -> DepNode<DefId>, V: ItemLikeVisitor<'gcx>
     {
-        dep_graph::visit_all_items_in_krate(self.global_tcx(), dep_node_fn, visitor);
+        dep_graph::visit_all_item_likes_in_krate(self.global_tcx(), dep_node_fn, visitor);
     }
 
     /// Looks up the span of `impl_did` if the impl is local; otherwise returns `Err`