auto merge of #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 #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 pnkfelix/rust@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 pnkfelix/rust@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);
 }