auto merge of rust-lang#8527 : pnkfelix/rust/fsk-visitor-vpar-defaults-step1, r=nikomatsakis

Rewriting visit.rs to operate on a borrowed `&mut V` where `<V:Visitor>`

r? @nikomatsakis
r? @pcwalton

This is the first in a planned series of incremental pull requests.  (There will probably be five pull requests including this one, though they can be combined or split as necessary.)

Part of rust-lang#7081.  (But definitely does *not* complete it, not on its own, and not even after all five parts land; there are still a few loose ends to tie up or trim afterwards.)

The bulk of this change for this particular PR is 3d83010, which has the changes necessary to visit.rs to support everything else that comes later.  The other commits are illustrating the standard mechanical transformation that I am applying.

One important point for nearly *all* of these pull requests: I was deliberately *not* trying to be intelligent in the transformation. 

 * My goal was to minimize code churn, and make the transformation as mechanical as possible.  
 * For example, I kept the separation between the Visitor struct (corresponding to the earlier vtable of functions that were potentially closed over local state) and the explicitly passed (and clones) visitor Env.  I am certain that this is almost always unnecessary, and a later task will be to go through an meld the Env's into the Visitors as appropriate.  (My original goal had been to make such melding part of this task; that's why I turned them into a (Env, vtable) tuple way back when.  But I digress.)
 * Also, my main goal here was to get rid of the record of `@fn`'s as described by the oldvisit.rs API.  (This series gets rid of all but one such case; I'm still investigating that.)  There is *still* plenty of `@`-boxing left to be removed, I'm sure, and even still some `@fn`'s too; removing all of those is not the goal here; its just to get rid of the encoded protocol of `@fn`'s in the (old)visit API.

To see where things will be going in the future (i.e., to get a sneak-preview of future pull-requests in the series), see:

 * https://github.com/pnkfelix/rust/commits/fsk-visitor-vpar-defaults-step1 (that's this one)
 * https://github.com/pnkfelix/rust/commits/fsk-visitor-vpar-defaults-step2
 * https://github.com/pnkfelix/rust/commits/fsk-visitor-vpar-defaults-step3
 * https://github.com/pnkfelix/rust/commits/fsk-visitor-vpar-defaults-step4
 * https://github.com/pnkfelix/rust/commits/fsk-visitor-vpar-defaults-step5
    * Note that between step 4 and step 5 there is just a single commit, but its a doozy because its the only case where my mechanical transformation did not apply, and thus more serious rewriting was necessary.  See commit da902b2

Author: bors

src/librustc/metadata/encoder.rs

@@ -39,7 +39,8 @@ use syntax::attr;
 use syntax::attr::AttrMetaMethods;
 use syntax::diagnostic::span_handler;
 use syntax::parse::token::special_idents;
-use syntax::{ast_util, oldvisit};
+use syntax::ast_util;
+use syntax::visit;
 use syntax::parse::token;
 use syntax;
 use writer = extra::ebml::writer;
@@ -1184,6 +1185,74 @@ fn encode_info_for_foreign_item(ecx: &EncodeContext,
     ebml_w.end_tag();
 }
 
+fn my_visit_expr(_e:@expr) { }
+
+fn my_visit_item(i:@item, items: ast_map::map, ebml_w:&writer::Encoder,
+                 ecx_ptr:*int, index: @mut ~[entry<i64>]) {
+    match items.get_copy(&i.id) {
+        ast_map::node_item(_, pt) => {
+            let mut ebml_w = ebml_w.clone();
+            // See above
+            let ecx : &EncodeContext = unsafe { cast::transmute(ecx_ptr) };
+            encode_info_for_item(ecx, &mut ebml_w, i, index, *pt);
+        }
+        _ => fail!("bad item")
+    }
+}
+
+fn my_visit_foreign_item(ni:@foreign_item, items: ast_map::map, ebml_w:&writer::Encoder,
+                         ecx_ptr:*int, index: @mut ~[entry<i64>]) {
+    match items.get_copy(&ni.id) {
+        ast_map::node_foreign_item(_, abi, _, pt) => {
+            debug!("writing foreign item %s::%s",
+                   ast_map::path_to_str(
+                       *pt,
+                       token::get_ident_interner()),
+                   token::ident_to_str(&ni.ident));
+
+            let mut ebml_w = ebml_w.clone();
+            // See above
+            let ecx : &EncodeContext = unsafe { cast::transmute(ecx_ptr) };
+            encode_info_for_foreign_item(ecx,
+                                         &mut ebml_w,
+                                         ni,
+                                         index,
+                                         pt,
+                                         abi);
+        }
+        // case for separate item and foreign-item tables
+        _ => fail!("bad foreign item")
+    }
+}
+
+struct EncodeVisitor {
+    ebml_w_for_visit_item: writer::Encoder,
+    ebml_w_for_visit_foreign_item: writer::Encoder,
+    ecx_ptr:*int,
+    items: ast_map::map,
+    index: @mut ~[entry<i64>],
+}
+
+impl visit::Visitor<()> for EncodeVisitor {
+    fn visit_expr(&mut self, ex:@expr, _:()) { my_visit_expr(ex); }
+    fn visit_item(&mut self, i:@item, _:()) {
+        visit::walk_item(self, i, ());
+        my_visit_item(i,
+                      self.items,
+                      &self.ebml_w_for_visit_item,
+                      self.ecx_ptr,
+                      self.index);
+    }
+    fn visit_foreign_item(&mut self, ni:@foreign_item, _:()) {
+        visit::walk_foreign_item(self, ni, ());
+        my_visit_foreign_item(ni,
+                              self.items,
+                              &self.ebml_w_for_visit_foreign_item,
+                              self.ecx_ptr,
+                              self.index);
+    }
+}
+
 fn encode_info_for_items(ecx: &EncodeContext,
                          ebml_w: &mut writer::Encoder,
                          crate: &Crate)
@@ -1201,54 +1270,17 @@ fn encode_info_for_items(ecx: &EncodeContext,
     let items = ecx.tcx.items;
 
     // See comment in `encode_side_tables_for_ii` in astencode
-    let ecx_ptr : *() = unsafe { cast::transmute(ecx) };
-
-    oldvisit::visit_crate(crate, ((), oldvisit::mk_vt(@oldvisit::Visitor {
-        visit_expr: |_e, (_cx, _v)| { },
-        visit_item: {
-            let ebml_w = (*ebml_w).clone();
-            |i, (cx, v)| {
-                oldvisit::visit_item(i, (cx, v));
-                match items.get_copy(&i.id) {
-                    ast_map::node_item(_, pt) => {
-                        let mut ebml_w = ebml_w.clone();
-                        // See above
-                        let ecx : &EncodeContext = unsafe { cast::transmute(ecx_ptr) };
-                        encode_info_for_item(ecx, &mut ebml_w, i, index, *pt);
-                    }
-                    _ => fail!("bad item")
-                }
-            }
-        },
-        visit_foreign_item: {
-            let ebml_w = (*ebml_w).clone();
-            |ni, (cx, v)| {
-                oldvisit::visit_foreign_item(ni, (cx, v));
-                match items.get_copy(&ni.id) {
-                    ast_map::node_foreign_item(_, abi, _, pt) => {
-                        debug!("writing foreign item %s::%s",
-                               ast_map::path_to_str(
-                                *pt,
-                                token::get_ident_interner()),
-                                token::ident_to_str(&ni.ident));
-
-                        let mut ebml_w = ebml_w.clone();
-                        // See above
-                        let ecx : &EncodeContext = unsafe { cast::transmute(ecx_ptr) };
-                        encode_info_for_foreign_item(ecx,
-                                                     &mut ebml_w,
-                                                     ni,
-                                                     index,
-                                                     pt,
-                                                     abi);
-                    }
-                    // case for separate item and foreign-item tables
-                    _ => fail!("bad foreign item")
-                }
-            }
-        },
-        ..*oldvisit::default_visitor()
-    })));
+    let ecx_ptr : *int = unsafe { cast::transmute(ecx) };
+    let mut visitor = EncodeVisitor {
+        index: index,
+        items: items,
+        ecx_ptr: ecx_ptr,
+        ebml_w_for_visit_item: (*ebml_w).clone(),
+        ebml_w_for_visit_foreign_item: (*ebml_w).clone(),
+    };
+
+    visit::walk_crate(&mut visitor, crate, ());
+
     ebml_w.end_tag();
     return /*bad*/(*index).clone();
 }

src/librustc/middle/borrowck/gather_loans/mod.rs

@@ -31,7 +31,9 @@ use syntax::ast;
 use syntax::ast_util::id_range;
 use syntax::codemap::span;
 use syntax::print::pprust;
-use syntax::oldvisit;
+use syntax::visit;
+use syntax::visit::Visitor;
+use syntax::ast::{expr, fn_kind, fn_decl, Block, NodeId, stmt, pat, Local};
 
 mod lifetime;
 mod restrictions;
@@ -72,6 +74,30 @@ struct GatherLoanCtxt {
     repeating_ids: ~[ast::NodeId]
 }
 
+struct GatherLoanVisitor;
+
+impl visit::Visitor<@mut GatherLoanCtxt> for GatherLoanVisitor {
+    fn visit_expr(&mut self, ex:@expr, e:@mut GatherLoanCtxt) {
+        gather_loans_in_expr(self, ex, e);
+    }
+    fn visit_block(&mut self, b:&Block, e:@mut GatherLoanCtxt) {
+        gather_loans_in_block(self, b, e);
+    }
+    fn visit_fn(&mut self, fk:&fn_kind, fd:&fn_decl, b:&Block,
+                s:span, n:NodeId, e:@mut GatherLoanCtxt) {
+        gather_loans_in_fn(self, fk, fd, b, s, n, e);
+    }
+    fn visit_stmt(&mut self, s:@stmt, e:@mut GatherLoanCtxt) {
+        add_stmt_to_map(self, s, e);
+    }
+    fn visit_pat(&mut self, p:@pat, e:@mut GatherLoanCtxt) {
+        add_pat_to_id_range(self, p, e);
+    }
+    fn visit_local(&mut self, l:@Local, e:@mut GatherLoanCtxt) {
+        gather_loans_in_local(self, l, e);
+    }
+}
+
 pub fn gather_loans(bccx: @BorrowckCtxt,
                     decl: &ast::fn_decl,
                     body: &ast::Block)
@@ -85,64 +111,57 @@ pub fn gather_loans(bccx: @BorrowckCtxt,
         move_data: @mut MoveData::new()
     };
     glcx.gather_fn_arg_patterns(decl, body);
-    let v = oldvisit::mk_vt(@oldvisit::Visitor {
-        visit_expr: gather_loans_in_expr,
-        visit_block: gather_loans_in_block,
-        visit_fn: gather_loans_in_fn,
-        visit_stmt: add_stmt_to_map,
-        visit_pat: add_pat_to_id_range,
-        visit_local: gather_loans_in_local,
-        .. *oldvisit::default_visitor()
-    });
-    (v.visit_block)(body, (glcx, v));
+
+    let mut v = GatherLoanVisitor;
+    v.visit_block(body, glcx);
     return (glcx.id_range, glcx.all_loans, glcx.move_data);
 }
 
-fn add_pat_to_id_range(p: @ast::pat,
-                       (this, v): (@mut GatherLoanCtxt,
-                                   oldvisit::vt<@mut GatherLoanCtxt>)) {
+fn add_pat_to_id_range(v: &mut GatherLoanVisitor,
+                       p: @ast::pat,
+                       this: @mut GatherLoanCtxt) {
     // NB: This visitor function just adds the pat ids into the id
     // range. We gather loans that occur in patterns using the
     // `gather_pat()` method below. Eventually these two should be
     // brought together.
     this.id_range.add(p.id);
-    oldvisit::visit_pat(p, (this, v));
+    visit::walk_pat(v, p, this);
 }
 
-fn gather_loans_in_fn(fk: &oldvisit::fn_kind,
+fn gather_loans_in_fn(v: &mut GatherLoanVisitor,
+                      fk: &fn_kind,
                       decl: &ast::fn_decl,
                       body: &ast::Block,
                       sp: span,
                       id: ast::NodeId,
-                      (this, v): (@mut GatherLoanCtxt,
-                                  oldvisit::vt<@mut GatherLoanCtxt>)) {
+                      this: @mut GatherLoanCtxt) {
     match fk {
         // Do not visit items here, the outer loop in borrowck/mod
         // will visit them for us in turn.
-        &oldvisit::fk_item_fn(*) | &oldvisit::fk_method(*) => {
+        &visit::fk_item_fn(*) | &visit::fk_method(*) => {
             return;
         }
 
         // Visit closures as part of the containing item.
-        &oldvisit::fk_anon(*) | &oldvisit::fk_fn_block(*) => {
+        &visit::fk_anon(*) | &visit::fk_fn_block(*) => {
             this.push_repeating_id(body.id);
-            oldvisit::visit_fn(fk, decl, body, sp, id, (this, v));
+            visit::walk_fn(v, fk, decl, body, sp, id, this);
             this.pop_repeating_id(body.id);
             this.gather_fn_arg_patterns(decl, body);
         }
     }
 }
 
-fn gather_loans_in_block(blk: &ast::Block,
-                         (this, vt): (@mut GatherLoanCtxt,
-                                      oldvisit::vt<@mut GatherLoanCtxt>)) {
+fn gather_loans_in_block(v: &mut GatherLoanVisitor,
+                         blk: &ast::Block,
+                         this: @mut GatherLoanCtxt) {
     this.id_range.add(blk.id);
-    oldvisit::visit_block(blk, (this, vt));
+    visit::walk_block(v, blk, this);
 }
 
-fn gather_loans_in_local(local: @ast::Local,
-                         (this, vt): (@mut GatherLoanCtxt,
-                                      oldvisit::vt<@mut GatherLoanCtxt>)) {
+fn gather_loans_in_local(v: &mut GatherLoanVisitor,
+                         local: @ast::Local,
+                         this: @mut GatherLoanCtxt) {
     match local.init {
         None => {
             // Variable declarations without initializers are considered "moves":
@@ -173,12 +192,13 @@ fn gather_loans_in_local(local: @ast::Local,
         }
     }
 
-    oldvisit::visit_local(local, (this, vt));
+    visit::walk_local(v, local, this);
 }
 
-fn gather_loans_in_expr(ex: @ast::expr,
-                        (this, vt): (@mut GatherLoanCtxt,
-                                     oldvisit::vt<@mut GatherLoanCtxt>)) {
+
+fn gather_loans_in_expr(v: &mut GatherLoanVisitor,
+                        ex: @ast::expr,
+                        this: @mut GatherLoanCtxt) {
     let bccx = this.bccx;
     let tcx = bccx.tcx;
 
@@ -218,7 +238,7 @@ fn gather_loans_in_expr(ex: @ast::expr,
         // for the lifetime `scope_r` of the resulting ptr:
         let scope_r = ty_region(tcx, ex.span, ty::expr_ty(tcx, ex));
         this.guarantee_valid(ex.id, ex.span, base_cmt, mutbl, scope_r);
-        oldvisit::visit_expr(ex, (this, vt));
+        visit::walk_expr(v, ex, this);
       }
 
       ast::expr_assign(l, _) | ast::expr_assign_op(_, _, l, _) => {
@@ -235,7 +255,7 @@ fn gather_loans_in_expr(ex: @ast::expr,
                   // with moves etc, just ignore.
               }
           }
-          oldvisit::visit_expr(ex, (this, vt));
+          visit::walk_expr(v, ex, this);
       }
 
       ast::expr_match(ex_v, ref arms) => {
@@ -245,7 +265,7 @@ fn gather_loans_in_expr(ex: @ast::expr,
                 this.gather_pat(cmt, *pat, Some((arm.body.id, ex.id)));
             }
         }
-        oldvisit::visit_expr(ex, (this, vt));
+        visit::walk_expr(v, ex, this);
       }
 
       ast::expr_index(_, _, arg) |
@@ -259,36 +279,36 @@ fn gather_loans_in_expr(ex: @ast::expr,
           let scope_r = ty::re_scope(ex.id);
           let arg_cmt = this.bccx.cat_expr(arg);
           this.guarantee_valid(arg.id, arg.span, arg_cmt, m_imm, scope_r);
-          oldvisit::visit_expr(ex, (this, vt));
+          visit::walk_expr(v, ex, this);
       }
 
       // see explanation attached to the `root_ub` field:
       ast::expr_while(cond, ref body) => {
           // during the condition, can only root for the condition
           this.push_repeating_id(cond.id);
-          (vt.visit_expr)(cond, (this, vt));
+          v.visit_expr(cond, this);
           this.pop_repeating_id(cond.id);
 
           // during body, can only root for the body
           this.push_repeating_id(body.id);
-          (vt.visit_block)(body, (this, vt));
+          v.visit_block(body, this);
           this.pop_repeating_id(body.id);
       }
 
       // see explanation attached to the `root_ub` field:
       ast::expr_loop(ref body, _) => {
           this.push_repeating_id(body.id);
-          oldvisit::visit_expr(ex, (this, vt));
+          visit::walk_expr(v, ex, this);
           this.pop_repeating_id(body.id);
       }
 
       ast::expr_fn_block(*) => {
           gather_moves::gather_captures(this.bccx, this.move_data, ex);
-          oldvisit::visit_expr(ex, (this, vt));
+          visit::walk_expr(v, ex, this);
       }
 
       _ => {
-        oldvisit::visit_expr(ex, (this, vt));
+          visit::walk_expr(v, ex, this);
       }
     }
 }
@@ -770,14 +790,14 @@ impl GatherLoanCtxt {
 
 // Setting up info that preserve needs.
 // This is just the most convenient place to do it.
-fn add_stmt_to_map(stmt: @ast::stmt,
-                   (this, vt): (@mut GatherLoanCtxt,
-                                oldvisit::vt<@mut GatherLoanCtxt>)) {
+fn add_stmt_to_map(v: &mut GatherLoanVisitor,
+                   stmt: @ast::stmt,
+                   this: @mut GatherLoanCtxt) {
     match stmt.node {
         ast::stmt_expr(_, id) | ast::stmt_semi(_, id) => {
             this.bccx.stmt_map.insert(id);
         }
         _ => ()
     }
-    oldvisit::visit_stmt(stmt, (this, vt));
+    visit::walk_stmt(v, stmt, this);
 }