Refactor visit.rs with a `Visitor` trait

[pnkfelix]

Currently libsyntax/visit.rs provides a record that holds a collection of @fn's, using a mix of lambda-lifting and an explicit virtual method table to define traversals over the AST.

pub struct Visitor<E> {
    visit_mod: @fn(&_mod, span, node_id, E, vt<E>),
    visit_view_item: @fn(@view_item, E, vt<E>),
    ...
}

where

pub enum vt<E> { mk_vt(visitor<E>), }

So then a use of the visitor looks like this (taken from freevars.rs):

    let v = visit::mk_vt(@visit::Visitor {visit_item: ignore_item,
                                          visit_expr: walk_expr,
                                          .. *visit::default_visitor()});
    (v.visit_block)(blk, 1, v);

where ignore_item and walk_expr are defined accordingly.

The above is awkward. A more natural way to encode this would be via a Visitor trait that defined the visit methods; then instances of the trait would correspond to the accumulated state denoted above by E in the struct definition and 1 in the freevars example.

(I originally had written a @Visitor object, but there is no need to commit to using objects in the Visitor interface.)

[pnkfelix]

(The original description for this ticket used an @visitor object, but that was not what Niko intended. I'm rewriting the description now to reflect a more generic approach.)

[nikomatsakis]

I don't think we should use objects here. I envision something like this:

trait Visitor {
    fn visit_expr(&mut self, expr: @ast::expr) {
        super_expr(self, expr);
    }
    fn visit_fn(&mut self, ...);
}

// It occurs to me that we have to break "super functions" into their own
// top-level functions if we want them to be callable.
fn super_expr<V:Visitor>(
    this: &mut V,
    expr: @ast::expr)
{
        match expr.node {
            ast::expr_binary(op, left, right) => {
                self.visit_expr(left); self.visit_expr(right);
            }
            ...
        }
}                                  

and then we implement individual passes follows:

struct MyContext {
    tcx: ty::ctxt, ...
}

impl visit::Visitor for MyContext {
    fn visit_expr(&mut self, expr: @ast::expr) {
        visit::super_expr(self, expr);
    }
}

[nikomatsakis]

Incidentally, @msullivan has been working on fixing bugs in default methods. For all I know this approach may be feasible now, or there may be bugs standing in the way.

[pnkfelix]

Default methods, which block this, are tracked in #2794

[sanxiyn]

Referencing @Aatch's branch here.

[pnkfelix]

So apparently we have @pnkfelix working on this problem on branches labelled fsk-issue7081 like this, as well as @Aatch's branch that sanxiyn mentioned above, and now there is also @pcwalton working on a visit-trait branch as well, as he announced in the #rust irc channel.

Though I think pcwalton's code is putting in an @Trait style visitor; Niko previously pointed out on this isssue that it shouldn't be necessary to use an @Trait object here.

[pnkfelix]

�One detail I meant to include in my last comment: the rewrite from my branch is attempting to support the transition from (E, vt<E>) to VisitorTrait in an incremental fashion, by putting in a impl<E> VisitorTrait for (E, vt<E>) as part of visit.rs.

I saw such incremental development as a virtue of my strategy, but its possible that getting that right is just slowing my progress down.

[nikomatsakis]

Indeed. If we're going to rewrite all the visitor passes, it'd be a shame not to remove the managed boxes!