auto merge of #8623 : pnkfelix/rust/fsk-visitor-vpar-defaults-step4, r=nmatsakis

Follow up to #8619 (step 3 of 5).

(See #8527, which was step 1 of 5, for the full outline.)

Part of #7081.

8 files changed, 314 insertions, 217 deletions

diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs
index 620a1e9efe3..0fa7750afa7 100644
--- a/src/librustc/middle/borrowck/check_loans.rs
+++ b/src/librustc/middle/borrowck/check_loans.rs
@@ -27,7 +27,8 @@ use syntax::ast::{m_mutbl, m_imm, m_const};
 use syntax::ast;
 use syntax::ast_util;
 use syntax::codemap::span;
-use syntax::oldvisit;
+use syntax::visit;
+use syntax::visit::Visitor;
 use util::ppaux::Repr;
 
 #[deriving(Clone)]
@@ -39,6 +40,27 @@ struct CheckLoanCtxt<'self> {
     reported: @mut HashSet<ast::NodeId>,
 }
 
+struct CheckLoanVisitor;
+
+impl<'self> Visitor<CheckLoanCtxt<'self>> for CheckLoanVisitor {
+    fn visit_expr<'a>(&mut self, ex:@ast::expr, e:CheckLoanCtxt<'a>) {
+        check_loans_in_expr(self, ex, e);
+    }
+    fn visit_local(&mut self, l:@ast::Local, e:CheckLoanCtxt) {
+        check_loans_in_local(self, l, e);
+    }
+    fn visit_block(&mut self, b:&ast::Block, e:CheckLoanCtxt) {
+        check_loans_in_block(self, b, e);
+    }
+    fn visit_pat(&mut self, p:@ast::pat, e:CheckLoanCtxt) {
+        check_loans_in_pat(self, p, e);
+    }
+    fn visit_fn(&mut self, fk:&visit::fn_kind, fd:&ast::fn_decl,
+                b:&ast::Block, s:span, n:ast::NodeId, e:CheckLoanCtxt) {
+        check_loans_in_fn(self, fk, fd, b, s, n, e);
+    }
+}
+
 pub fn check_loans(bccx: @BorrowckCtxt,
                    dfcx_loans: &LoanDataFlow,
                    move_data: move_data::FlowedMoveData,
@@ -54,15 +76,8 @@ pub fn check_loans(bccx: @BorrowckCtxt,
         reported: @mut HashSet::new(),
     };
 
-    let vt = oldvisit::mk_vt(@oldvisit::Visitor {
-        visit_expr: check_loans_in_expr,
-        visit_local: check_loans_in_local,
-        visit_block: check_loans_in_block,
-        visit_pat: check_loans_in_pat,
-        visit_fn: check_loans_in_fn,
-        .. *oldvisit::default_visitor()
-    });
-    (vt.visit_block)(body, (clcx, vt));
+    let mut vt = CheckLoanVisitor;
+    vt.visit_block(body, clcx);
 }
 
 enum MoveError {
@@ -626,27 +641,27 @@ impl<'self> CheckLoanCtxt<'self> {
     }
 }
 
-fn check_loans_in_fn<'a>(fk: &oldvisit::fn_kind,
+fn check_loans_in_fn<'a>(visitor: &mut CheckLoanVisitor,
+                         fk: &visit::fn_kind,
                          decl: &ast::fn_decl,
                          body: &ast::Block,
                          sp: span,
                          id: ast::NodeId,
-                         (this, visitor): (CheckLoanCtxt<'a>,
-                                           oldvisit::vt<CheckLoanCtxt<'a>>)) {
+                         this: CheckLoanCtxt<'a>) {
     match *fk {
-        oldvisit::fk_item_fn(*) |
-        oldvisit::fk_method(*) => {
+        visit::fk_item_fn(*) |
+        visit::fk_method(*) => {
             // Don't process nested items.
             return;
         }
 
-        oldvisit::fk_anon(*) |
-        oldvisit::fk_fn_block(*) => {
+        visit::fk_anon(*) |
+        visit::fk_fn_block(*) => {
             check_captured_variables(this, id, sp);
         }
     }
 
-    oldvisit::visit_fn(fk, decl, body, sp, id, (this, visitor));
+    visit::walk_fn(visitor, fk, decl, body, sp, id, this);
 
     fn check_captured_variables(this: CheckLoanCtxt,
                                 closure_id: ast::NodeId,
@@ -689,16 +704,16 @@ fn check_loans_in_fn<'a>(fk: &oldvisit::fn_kind,
     }
 }
 
-fn check_loans_in_local<'a>(local: @ast::Local,
-                            (this, vt): (CheckLoanCtxt<'a>,
-                                         oldvisit::vt<CheckLoanCtxt<'a>>)) {
-    oldvisit::visit_local(local, (this, vt));
+fn check_loans_in_local<'a>(vt: &mut CheckLoanVisitor,
+                            local: @ast::Local,
+                            this: CheckLoanCtxt<'a>) {
+    visit::walk_local(vt, local, this);
 }
 
-fn check_loans_in_expr<'a>(expr: @ast::expr,
-                           (this, vt): (CheckLoanCtxt<'a>,
-                                        oldvisit::vt<CheckLoanCtxt<'a>>)) {
-    oldvisit::visit_expr(expr, (this, vt));
+fn check_loans_in_expr<'a>(vt: &mut CheckLoanVisitor,
+                           expr: @ast::expr,
+                           this: CheckLoanCtxt<'a>) {
+    visit::walk_expr(vt, expr, this);
 
     debug!("check_loans_in_expr(expr=%s)",
            expr.repr(this.tcx()));
@@ -749,19 +764,19 @@ fn check_loans_in_expr<'a>(expr: @ast::expr,
     }
 }
 
-fn check_loans_in_pat<'a>(pat: @ast::pat,
-                          (this, vt): (CheckLoanCtxt<'a>,
-                                       oldvisit::vt<CheckLoanCtxt<'a>>))
+fn check_loans_in_pat<'a>(vt: &mut CheckLoanVisitor,
+                          pat: @ast::pat,
+                          this: CheckLoanCtxt<'a>)
 {
     this.check_for_conflicting_loans(pat.id);
     this.check_move_out_from_id(pat.id, pat.span);
-    oldvisit::visit_pat(pat, (this, vt));
+    visit::walk_pat(vt, pat, this);
 }
 
-fn check_loans_in_block<'a>(blk: &ast::Block,
-                            (this, vt): (CheckLoanCtxt<'a>,
-                                         oldvisit::vt<CheckLoanCtxt<'a>>))
+fn check_loans_in_block<'a>(vt: &mut CheckLoanVisitor,
+                            blk: &ast::Block,
+                            this: CheckLoanCtxt<'a>)
 {
-    oldvisit::visit_block(blk, (this, vt));
+    visit::walk_block(vt, blk, this);
     this.check_for_conflicting_loans(blk.id);
 }
diff --git a/src/librustc/middle/entry.rs b/src/librustc/middle/entry.rs
index 34aeaf8a6ce..e6830a1f64e 100644
--- a/src/librustc/middle/entry.rs
+++ b/src/librustc/middle/entry.rs
@@ -15,9 +15,9 @@ use syntax::ast::{Crate, NodeId, item, item_fn};
 use syntax::ast_map;
 use syntax::attr;
 use syntax::codemap::span;
-use syntax::oldvisit::{default_visitor, mk_vt, vt, Visitor, visit_crate};
-use syntax::oldvisit::{visit_item};
 use syntax::parse::token::special_idents;
+use syntax::visit;
+use syntax::visit::Visitor;
 use std::util;
 
 struct EntryContext {
@@ -39,7 +39,13 @@ struct EntryContext {
     non_main_fns: ~[(NodeId, span)],
 }
 
-type EntryVisitor = vt<@mut EntryContext>;
+struct EntryVisitor;
+
+impl Visitor<@mut EntryContext> for EntryVisitor {
+    fn visit_item(&mut self, item:@item, ctxt:@mut EntryContext) {
+        find_item(item, ctxt, self);
+    }
+}
 
 pub fn find_entry_point(session: Session, crate: &Crate, ast_map: ast_map::map) {
 
@@ -65,15 +71,14 @@ pub fn find_entry_point(session: Session, crate: &Crate, ast_map: ast_map::map)
         non_main_fns: ~[],
     };
 
-    visit_crate(crate, (ctxt, mk_vt(@Visitor {
-        visit_item: |item, (ctxt, visitor)| find_item(item, ctxt, visitor),
-        .. *default_visitor()
-    })));
+    let mut v = EntryVisitor;
+
+    visit::walk_crate(&mut v, crate, ctxt);
 
     configure_main(ctxt);
 }
 
-fn find_item(item: @item, ctxt: @mut EntryContext, visitor: EntryVisitor) {
+fn find_item(item: @item, ctxt: @mut EntryContext, visitor: &mut EntryVisitor) {
     match item.node {
         item_fn(*) => {
             if item.ident == special_idents::main {
@@ -120,7 +125,7 @@ fn find_item(item: @item, ctxt: @mut EntryContext, visitor: EntryVisitor) {
         _ => ()
     }
 
-    visit_item(item, (ctxt, visitor));
+    visit::walk_item(visitor, item, ctxt);
 }
 
 fn configure_main(ctxt: @mut EntryContext) {
diff --git a/src/librustc/middle/freevars.rs b/src/librustc/middle/freevars.rs
index 5f7ef8d31f7..c6446955cc8 100644
--- a/src/librustc/middle/freevars.rs
+++ b/src/librustc/middle/freevars.rs
@@ -17,7 +17,10 @@ use middle::ty;
 
 use std::hashmap::HashMap;
 use syntax::codemap::span;
-use syntax::{ast, ast_util, oldvisit};
+use syntax::{ast, ast_util};
+use syntax::visit;
+use syntax::visit::Visitor;
+use syntax::ast::{item};
 
 // A vector of defs representing the free variables referred to in a function.
 // (The def_upvar will already have been stripped).
@@ -29,27 +32,27 @@ pub struct freevar_entry {
 pub type freevar_info = @~[@freevar_entry];
 pub type freevar_map = @mut HashMap<ast::NodeId, freevar_info>;
 
-// Searches through part of the AST for all references to locals or
-// upvars in this frame and returns the list of definition IDs thus found.
-// Since we want to be able to collect upvars in some arbitrary piece
-// of the AST, we take a walker function that we invoke with a visitor
-// in order to start the search.
-fn collect_freevars(def_map: resolve::DefMap, blk: &ast::Block)
-    -> freevar_info {
-    let seen = @mut HashMap::new();
-    let refs = @mut ~[];
+struct CollectFreevarsVisitor {
+    seen: @mut HashMap<ast::NodeId, ()>,
+    refs: @mut ~[@freevar_entry],
+    def_map: resolve::DefMap,
+}
+
+impl Visitor<int> for CollectFreevarsVisitor {
 
-    fn ignore_item(_i: @ast::item, (_depth, _v): (int, oldvisit::vt<int>)) { }
+    fn visit_item(&mut self, _:@item, _:int) {
+        // ignore_item
+    }
+
+    fn visit_expr(&mut self, expr:@ast::expr, depth:int) {
 
-    let walk_expr: @fn(expr: @ast::expr, (int, oldvisit::vt<int>)) =
-        |expr, (depth, v)| {
             match expr.node {
               ast::expr_fn_block(*) => {
-                oldvisit::visit_expr(expr, (depth + 1, v))
+                visit::walk_expr(self, expr, depth + 1)
               }
               ast::expr_path(*) | ast::expr_self => {
                   let mut i = 0;
-                  match def_map.find(&expr.id) {
+                  match self.def_map.find(&expr.id) {
                     None => fail!("path not found"),
                     Some(&df) => {
                       let mut def = df;
@@ -62,28 +65,58 @@ fn collect_freevars(def_map: resolve::DefMap, blk: &ast::Block)
                       }
                       if i == depth { // Made it to end of loop
                         let dnum = ast_util::def_id_of_def(def).node;
-                        if !seen.contains_key(&dnum) {
-                            refs.push(@freevar_entry {
+                        if !self.seen.contains_key(&dnum) {
+                            self.refs.push(@freevar_entry {
                                 def: def,
                                 span: expr.span,
                             });
-                            seen.insert(dnum, ());
+                            self.seen.insert(dnum, ());
                         }
                       }
                     }
                   }
               }
-              _ => oldvisit::visit_expr(expr, (depth, v))
+              _ => visit::walk_expr(self, expr, depth)
             }
-        };
+    }
 
-    let v = oldvisit::mk_vt(@oldvisit::Visitor {visit_item: ignore_item,
-                                          visit_expr: walk_expr,
-                                          .. *oldvisit::default_visitor()});
-    (v.visit_block)(blk, (1, v));
+
+}
+
+// Searches through part of the AST for all references to locals or
+// upvars in this frame and returns the list of definition IDs thus found.
+// Since we want to be able to collect upvars in some arbitrary piece
+// of the AST, we take a walker function that we invoke with a visitor
+// in order to start the search.
+fn collect_freevars(def_map: resolve::DefMap, blk: &ast::Block)
+    -> freevar_info {
+    let seen = @mut HashMap::new();
+    let refs = @mut ~[];
+
+    let mut v = CollectFreevarsVisitor {
+        seen: seen,
+        refs: refs,
+        def_map: def_map,
+    };
+
+    v.visit_block(blk, 1);
     return @(*refs).clone();
 }
 
+struct AnnotateFreevarsVisitor {
+    def_map: resolve::DefMap,
+    freevars: freevar_map,
+}
+
+impl Visitor<()> for AnnotateFreevarsVisitor {
+    fn visit_fn(&mut self, fk:&visit::fn_kind, fd:&ast::fn_decl,
+                blk:&ast::Block, s:span, nid:ast::NodeId, _:()) {
+        let vars = collect_freevars(self.def_map, blk);
+        self.freevars.insert(nid, vars);
+        visit::walk_fn(self, fk, fd, blk, s, nid, ());
+    }
+}
+
 // Build a map from every function and for-each body to a set of the
 // freevars contained in it. The implementation is not particularly
 // efficient as it fully recomputes the free variables at every
@@ -93,20 +126,11 @@ pub fn annotate_freevars(def_map: resolve::DefMap, crate: &ast::Crate) ->
    freevar_map {
     let freevars = @mut HashMap::new();
 
-    let walk_fn: @fn(&oldvisit::fn_kind,
-                     &ast::fn_decl,
-                     &ast::Block,
-                     span,
-                     ast::NodeId) = |_, _, blk, _, nid| {
-        let vars = collect_freevars(def_map, blk);
-        freevars.insert(nid, vars);
+    let mut visitor = AnnotateFreevarsVisitor {
+        def_map: def_map,
+        freevars: freevars,
     };
-
-    let visitor =
-        oldvisit::mk_simple_visitor(@oldvisit::SimpleVisitor {
-            visit_fn: walk_fn,
-            .. *oldvisit::default_simple_visitor()});
-    oldvisit::visit_crate(crate, ((), visitor));
+    visit::walk_crate(&mut visitor, crate, ());
 
     return freevars;
 }
diff --git a/src/librustc/middle/kind.rs b/src/librustc/middle/kind.rs
index 84eb371d7b3..6d7d8112991 100644
--- a/src/librustc/middle/kind.rs
+++ b/src/librustc/middle/kind.rs
@@ -21,7 +21,8 @@ use syntax::attr;
 use syntax::codemap::span;
 use syntax::opt_vec;
 use syntax::print::pprust::expr_to_str;
-use syntax::{oldvisit, ast_util};
+use syntax::{visit,ast_util};
+use syntax::visit::Visitor;
 
 // Kind analysis pass.
 //
@@ -58,6 +59,29 @@ pub struct Context {
     current_item: NodeId
 }
 
+struct KindAnalysisVisitor;
+
+impl Visitor<Context> for KindAnalysisVisitor {
+
+    fn visit_expr(&mut self, ex:@expr, e:Context) {
+        check_expr(self, ex, e);
+    }
+
+    fn visit_fn(&mut self, fk:&visit::fn_kind, fd:&fn_decl, b:&Block, s:span, n:NodeId, e:Context) {
+        check_fn(self, fk, fd, b, s, n, e);
+    }
+
+    fn visit_ty(&mut self, t:&Ty, e:Context) {
+        check_ty(self, t, e);
+    }
+    fn visit_item(&mut self, i:@item, e:Context) {
+        check_item(self, i, e);
+    }
+    fn visit_block(&mut self, b:&Block, e:Context) {
+        check_block(self, b, e);
+    }
+}
+
 pub fn check_crate(tcx: ty::ctxt,
                    method_map: typeck::method_map,
                    crate: &Crate) {
@@ -66,15 +90,8 @@ pub fn check_crate(tcx: ty::ctxt,
         method_map: method_map,
         current_item: -1
     };
-    let visit = oldvisit::mk_vt(@oldvisit::Visitor {
-        visit_expr: check_expr,
-        visit_fn: check_fn,
-        visit_ty: check_ty,
-        visit_item: check_item,
-        visit_block: check_block,
-        .. *oldvisit::default_visitor()
-    });
-    oldvisit::visit_crate(crate, (ctx, visit));
+    let mut visit = KindAnalysisVisitor;
+    visit::walk_crate(&mut visit, crate, ctx);
     tcx.sess.abort_if_errors();
 }
 
@@ -108,12 +125,13 @@ fn check_struct_safe_for_destructor(cx: Context,
     }
 }
 
-fn check_block(block: &Block,
-               (cx, visitor): (Context, oldvisit::vt<Context>)) {
-    oldvisit::visit_block(block, (cx, visitor));
+fn check_block(visitor: &mut KindAnalysisVisitor,
+               block: &Block,
+               cx: Context) {
+    visit::walk_block(visitor, block, cx);
 }
 
-fn check_item(item: @item, (cx, visitor): (Context, oldvisit::vt<Context>)) {
+fn check_item(visitor: &mut KindAnalysisVisitor, item: @item, cx: Context) {
     // If this is a destructor, check kinds.
     if !attr::contains_name(item.attrs, "unsafe_destructor") {
         match item.node {
@@ -153,7 +171,7 @@ fn check_item(item: @item, (cx, visitor): (Context, oldvisit::vt<Context>)) {
     }
 
     let cx = Context { current_item: item.id, ..cx };
-    oldvisit::visit_item(item, (cx, visitor));
+    visit::walk_item(visitor, item, cx);
 }
 
 // Yields the appropriate function to check the kind of closed over
@@ -227,13 +245,13 @@ fn with_appropriate_checker(cx: Context, id: NodeId,
 // Check that the free variables used in a shared/sendable closure conform
 // to the copy/move kind bounds. Then recursively check the function body.
 fn check_fn(
-    fk: &oldvisit::fn_kind,
+    v: &mut KindAnalysisVisitor,
+    fk: &visit::fn_kind,
     decl: &fn_decl,
     body: &Block,
     sp: span,
     fn_id: NodeId,
-    (cx, v): (Context,
-              oldvisit::vt<Context>)) {
+    cx: Context) {
 
     // Check kinds on free variables:
     do with_appropriate_checker(cx, fn_id) |chk| {
@@ -243,10 +261,10 @@ fn check_fn(
         }
     }
 
-    oldvisit::visit_fn(fk, decl, body, sp, fn_id, (cx, v));
+    visit::walk_fn(v, fk, decl, body, sp, fn_id, cx);
 }
 
-pub fn check_expr(e: @expr, (cx, v): (Context, oldvisit::vt<Context>)) {
+pub fn check_expr(v: &mut KindAnalysisVisitor, e: @expr, cx: Context) {
     debug!("kind::check_expr(%s)", expr_to_str(e, cx.tcx.sess.intr()));
 
     // Handle any kind bounds on type parameters
@@ -311,10 +329,10 @@ pub fn check_expr(e: @expr, (cx, v): (Context, oldvisit::vt<Context>)) {
         }
         _ => {}
     }
-    oldvisit::visit_expr(e, (cx, v));
+    visit::walk_expr(v, e, cx);
 }
 
-fn check_ty(aty: &Ty, (cx, v): (Context, oldvisit::vt<Context>)) {
+fn check_ty(v: &mut KindAnalysisVisitor, aty: &Ty, cx: Context) {
     match aty.node {
       ty_path(_, _, id) => {
           let r = cx.tcx.node_type_substs.find(&id);
@@ -329,7 +347,7 @@ fn check_ty(aty: &Ty, (cx, v): (Context, oldvisit::vt<Context>)) {
       }
       _ => {}
     }
-    oldvisit::visit_ty(aty, (cx, v));
+    visit::walk_ty(v, aty, cx);
 }
 
 // Calls "any_missing" if any bounds were missing.
diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs
index ae9d3d5ec0e..9b11301a9c4 100644
--- a/src/librustc/middle/lang_items.rs
+++ b/src/librustc/middle/lang_items.rs
@@ -26,8 +26,9 @@ use metadata::cstore::iter_crate_data;
 use syntax::ast::{Crate, def_id, MetaItem};
 use syntax::ast_util::local_def;
 use syntax::attr::AttrMetaMethods;
-use syntax::oldvisit::{default_simple_visitor, mk_simple_visitor};
-use syntax::oldvisit::{SimpleVisitor, visit_crate};
+use syntax::ast::{item};
+use syntax::visit;
+use syntax::visit::Visitor;
 
 use std::hashmap::HashMap;
 
@@ -292,6 +293,27 @@ struct LanguageItemCollector<'self> {
     item_refs: HashMap<@str, uint>,
 }
 
+struct LanguageItemVisitor<'self> {
+    this: *mut LanguageItemCollector<'self>,
+}
+
+impl<'self> Visitor<()> for LanguageItemVisitor<'self> {
+
+    fn visit_item(&mut self, item:@item, _:()) {
+
+                for attribute in item.attrs.iter() {
+                    unsafe {
+                        (*self.this).match_and_collect_meta_item(
+                            local_def(item.id),
+                            attribute.node.value
+                        );
+                    }
+                }
+
+        visit::walk_item(self, item, ());
+    }
+}
+
 impl<'self> LanguageItemCollector<'self> {
     pub fn new<'a>(crate: &'a Crate, session: Session)
                    -> LanguageItemCollector<'a> {
@@ -404,19 +426,8 @@ impl<'self> LanguageItemCollector<'self> {
 
     pub fn collect_local_language_items(&mut self) {
         let this: *mut LanguageItemCollector = &mut *self;
-        visit_crate(self.crate, ((), mk_simple_visitor(@SimpleVisitor {
-            visit_item: |item| {
-                for attribute in item.attrs.iter() {
-                    unsafe {
-                        (*this).match_and_collect_meta_item(
-                            local_def(item.id),
-                            attribute.node.value
-                        );
-                    }
-                }
-            },
-            .. *default_simple_visitor()
-        })));
+        let mut v = LanguageItemVisitor { this: this };
+        visit::walk_crate(&mut v, self.crate, ());
     }
 
     pub fn collect_external_language_items(&mut self) {
diff --git a/src/librustc/middle/typeck/coherence.rs b/src/librustc/middle/typeck/coherence.rs
index b93f979894d..b659f6081b1 100644
--- a/src/librustc/middle/typeck/coherence.rs
+++ b/src/librustc/middle/typeck/coherence.rs
@@ -45,10 +45,7 @@ use syntax::ast_map;
 use syntax::ast_util::{def_id_of_def, local_def};
 use syntax::codemap::{span, dummy_sp};
 use syntax::opt_vec;
-use syntax::oldvisit::{default_simple_visitor, default_visitor};
-use syntax::oldvisit::{mk_simple_visitor, mk_vt, visit_crate, visit_item};
-use syntax::oldvisit::{Visitor, SimpleVisitor};
-use syntax::oldvisit::{visit_mod};
+use syntax::visit;
 use syntax::parse;
 use util::ppaux::ty_to_str;
 
@@ -168,15 +165,13 @@ pub struct CoherenceChecker {
     base_type_def_ids: @mut HashMap<def_id,def_id>,
 }
 
-impl CoherenceChecker {
-    pub fn check_coherence(self, crate: &Crate) {
-        // Check implementations and traits. This populates the tables
-        // containing the inherent methods and extension methods. It also
-        // builds up the trait inheritance table.
-        visit_crate(crate, ((), mk_simple_visitor(@SimpleVisitor {
-            visit_item: |item| {
+struct CoherenceCheckVisitor { cc: CoherenceChecker }
+
+impl visit::Visitor<()> for CoherenceCheckVisitor {
+    fn visit_item(&mut self, item:@item, _:()) {
+
 //                debug!("(checking coherence) item '%s'",
-//                       self.crate_context.tcx.sess.str_of(item.ident));
+//                       self.cc.crate_context.tcx.sess.str_of(item.ident));
 
                 match item.node {
                     item_impl(_, ref opt_trait, _, _) => {
@@ -184,15 +179,75 @@ impl CoherenceChecker {
                             opt_trait.iter()
                                      .map(|x| (*x).clone())
                                      .collect();
-                        self.check_implementation(item, opt_trait);
+                        self.cc.check_implementation(item, opt_trait);
                     }
                     _ => {
                         // Nothing to do.
                     }
                 };
-            },
-            .. *default_simple_visitor()
-        })));
+
+        visit::walk_item(self, item, ());
+    }
+}
+
+struct PrivilegedScopeVisitor { cc: CoherenceChecker }
+
+impl visit::Visitor<()> for PrivilegedScopeVisitor {
+    fn visit_item(&mut self, item:@item, _:()) {
+
+                match item.node {
+                    item_mod(ref module_) => {
+                        // Then visit the module items.
+                        visit::walk_mod(self, module_, ());
+                    }
+                    item_impl(_, None, ref ast_ty, _) => {
+                        if !self.cc.ast_type_is_defined_in_local_crate(ast_ty) {
+                            // This is an error.
+                            let session = self.cc.crate_context.tcx.sess;
+                            session.span_err(item.span,
+                                             "cannot associate methods with a type outside the \
+                                              crate the type is defined in; define and implement \
+                                              a trait or new type instead");
+                        }
+                    }
+                    item_impl(_, Some(ref trait_ref), _, _) => {
+                        // `for_ty` is `Type` in `impl Trait for Type`
+                        let for_ty =
+                            ty::node_id_to_type(self.cc.crate_context.tcx,
+                                                item.id);
+                        if !type_is_defined_in_local_crate(for_ty) {
+                            // This implementation is not in scope of its base
+                            // type. This still might be OK if the trait is
+                            // defined in the same crate.
+
+                            let trait_def_id =
+                                self.cc.trait_ref_to_trait_def_id(trait_ref);
+
+                            if trait_def_id.crate != LOCAL_CRATE {
+                                let session = self.cc.crate_context.tcx.sess;
+                                session.span_err(item.span,
+                                                 "cannot provide an extension implementation \
+                                                  for a trait not defined in this crate");
+                            }
+                        }
+
+                        visit::walk_item(self, item, ());
+                    }
+                    _ => {
+                        visit::walk_item(self, item, ());
+                    }
+                }
+    }
+}
+
+impl CoherenceChecker {
+    pub fn check_coherence(self, crate: &Crate) {
+        // Check implementations and traits. This populates the tables
+        // containing the inherent methods and extension methods. It also
+        // builds up the trait inheritance table.
+
+        let mut visitor = CoherenceCheckVisitor { cc: self };
+        visit::walk_crate(&mut visitor, crate, ());
 
         // Check that there are no overlapping trait instances
         self.check_implementation_coherence();
@@ -486,53 +541,8 @@ impl CoherenceChecker {
 
     // Privileged scope checking
     pub fn check_privileged_scopes(self, crate: &Crate) {
-        visit_crate(crate, ((), mk_vt(@Visitor {
-            visit_item: |item, (_context, visitor)| {
-                match item.node {
-                    item_mod(ref module_) => {
-                        // Then visit the module items.
-                        visit_mod(module_, item.span, item.id, ((), visitor));
-                    }
-                    item_impl(_, None, ref ast_ty, _) => {
-                        if !self.ast_type_is_defined_in_local_crate(ast_ty) {
-                            // This is an error.
-                            let session = self.crate_context.tcx.sess;
-                            session.span_err(item.span,
-                                             "cannot associate methods with a type outside the \
-                                              crate the type is defined in; define and implement \
-                                              a trait or new type instead");
-                        }
-                    }
-                    item_impl(_, Some(ref trait_ref), _, _) => {
-                        // `for_ty` is `Type` in `impl Trait for Type`
-                        let for_ty =
-                            ty::node_id_to_type(self.crate_context.tcx,
-                                                item.id);
-                        if !type_is_defined_in_local_crate(for_ty) {
-                            // This implementation is not in scope of its base
-                            // type. This still might be OK if the trait is
-                            // defined in the same crate.
-
-                            let trait_def_id =
-                                self.trait_ref_to_trait_def_id(trait_ref);
-
-                            if trait_def_id.crate != LOCAL_CRATE {
-                                let session = self.crate_context.tcx.sess;
-                                session.span_err(item.span,
-                                                 "cannot provide an extension implementation \
-                                                  for a trait not defined in this crate");
-                            }
-                        }
-
-                        visit_item(item, ((), visitor));
-                    }
-                    _ => {
-                        visit_item(item, ((), visitor));
-                    }
-                }
-            },
-            .. *default_visitor()
-        })));
+        let mut visitor = PrivilegedScopeVisitor{ cc: self };
+        visit::walk_crate(&mut visitor, crate, ());
     }
 
     pub fn trait_ref_to_trait_def_id(&self, trait_ref: &trait_ref) -> def_id {
diff --git a/src/librustc/util/common.rs b/src/librustc/util/common.rs
index 26c22eed08c..22d7a4a5f58 100644
--- a/src/librustc/util/common.rs
+++ b/src/librustc/util/common.rs
@@ -11,7 +11,8 @@
 
 use syntax::ast;
 use syntax::codemap::{span};
-use syntax::oldvisit;
+use syntax::visit;
+use syntax::visit::Visitor;
 
 use std::hashmap::HashSet;
 use extra;
@@ -59,42 +60,48 @@ pub fn field_exprs(fields: ~[ast::Field]) -> ~[@ast::expr] {
     fields.map(|f| f.expr)
 }
 
-// Takes a predicate p, returns true iff p is true for any subexpressions
-// of b -- skipping any inner loops (loop, while, loop_body)
-pub fn loop_query(b: &ast::Block, p: @fn(&ast::expr_) -> bool) -> bool {
-    let rs = @mut false;
-    let visit_expr: @fn(@ast::expr,
-                        (@mut bool,
-                         oldvisit::vt<@mut bool>)) = |e, (flag, v)| {
-        *flag |= p(&e.node);
+struct LoopQueryVisitor {
+    p: @fn(&ast::expr_) -> bool
+}
+
+impl Visitor<@mut bool> for LoopQueryVisitor {
+    fn visit_expr(&mut self, e:@ast::expr, flag:@mut bool) {
+        *flag |= (self.p)(&e.node);
         match e.node {
           // Skip inner loops, since a break in the inner loop isn't a
           // break inside the outer loop
           ast::expr_loop(*) | ast::expr_while(*) => {}
-          _ => oldvisit::visit_expr(e, (flag, v))
+          _ => visit::walk_expr(self, e, flag)
         }
-    };
-    let v = oldvisit::mk_vt(@oldvisit::Visitor {
-        visit_expr: visit_expr,
-        .. *oldvisit::default_visitor()});
-    oldvisit::visit_block(b, (rs, v));
+    }
+}
+
+// Takes a predicate p, returns true iff p is true for any subexpressions
+// of b -- skipping any inner loops (loop, while, loop_body)
+pub fn loop_query(b: &ast::Block, p: @fn(&ast::expr_) -> bool) -> bool {
+    let rs = @mut false;
+    let mut v = LoopQueryVisitor { p: p };
+    visit::walk_block(&mut v, b, rs);
     return *rs;
 }
 
+struct BlockQueryVisitor {
+    p: @fn(@ast::expr) -> bool
+}
+
+impl Visitor<@mut bool> for BlockQueryVisitor {
+    fn visit_expr(&mut self, e:@ast::expr, flag:@mut bool) {
+        *flag |= (self.p)(e);
+        visit::walk_expr(self, e, flag)
+    }
+}
+
 // Takes a predicate p, returns true iff p is true for any subexpressions
 // of b -- skipping any inner loops (loop, while, loop_body)
 pub fn block_query(b: &ast::Block, p: @fn(@ast::expr) -> bool) -> bool {
     let rs = @mut false;
-    let visit_expr: @fn(@ast::expr,
-                        (@mut bool,
-                         oldvisit::vt<@mut bool>)) = |e, (flag, v)| {
-        *flag |= p(e);
-        oldvisit::visit_expr(e, (flag, v))
-    };
-    let v = oldvisit::mk_vt(@oldvisit::Visitor{
-        visit_expr: visit_expr,
-        .. *oldvisit::default_visitor()});
-    oldvisit::visit_block(b, (rs, v));
+    let mut v = BlockQueryVisitor { p: p };
+    visit::walk_block(&mut v, b, rs);
     return *rs;
 }
 
diff --git a/src/librusti/utils.rs b/src/librusti/utils.rs
index 94c0e4fe01e..467a3fdc278 100644
--- a/src/librusti/utils.rs
+++ b/src/librusti/utils.rs
@@ -13,24 +13,31 @@ use syntax::ast;
 use syntax::print::pp;
 use syntax::print::pprust;
 use syntax::parse::token;
+use syntax::visit;
 
-pub fn each_binding(l: @ast::Local, f: @fn(&ast::Path, ast::NodeId)) {
-    use syntax::oldvisit;
+struct EachBindingVisitor {
+    f: @fn(&ast::Path, ast::NodeId)
+}
 
-    let vt = oldvisit::mk_simple_visitor(
-        @oldvisit::SimpleVisitor {
-            visit_pat: |pat| {
+impl visit::Visitor<()> for EachBindingVisitor {
+    fn visit_pat(&mut self, pat:@ast::pat, _:()) {
                 match pat.node {
                     ast::pat_ident(_, ref path, _) => {
-                        f(path, pat.id);
+                        (self.f)(path, pat.id);
                     }
                     _ => {}
                 }
-            },
-            .. *oldvisit::default_simple_visitor()
-        }
-    );
-    (vt.visit_pat)(l.pat, ((), vt));
+
+                visit::walk_pat(self, pat, ());
+    }
+}
+
+pub fn each_binding(l: @ast::Local, f: @fn(&ast::Path, ast::NodeId)) {
+    use syntax::visit::Visitor;
+
+    let mut vt = EachBindingVisitor{ f: f };
+
+    vt.visit_pat(l.pat, ());
 }
 
 /// A utility function that hands off a pretty printer to a callback.