code cleanup: if visit::walk_foo(a, b, c) discards b, then b should not be in its API.

[pnkfelix]

The pattern for our syntax::visit module is that each visit method has a default implementation that just walks the substructure of that node in the AST.

What I often do when implementing a visitor is cut-and-paste the trait definition (with its default methods) into my new visitor and then consider each method to determine whether it should (1.) do extra-work, and/or (2.) stop the traversal, or (3.) just use the default impl.

When I see something like:

fn visit_struct_def(&mut self, s: &StructDef, i: Ident, g: &Generics, n: NodeId, e: E) {
    walk_struct_def(self, s, i, g, n, e)
}

I might assume: "Ah, the walk routine is already going to look at i and g, so if I need to look at those in my own code, then I can just inherit the visitor's default method."

The problem with that assumption is this:

pub fn walk_struct_def<E: Clone, V: Visitor<E>>(visitor: &mut V,
                                                struct_definition: &StructDef,
                                                _: Ident,
                                                _: &Generics,
                                                _: NodeId,
                                                env: E) {
   ...
}

My proposal solution: Any fn walk_foo in visit.rs should not have any parameters that are ignored.

[pnkfelix]

(on the other hand, an argument for going with the existing pattern is that it might make it easier to macro-generate the function signatures and calls semi-automatically. The problem with that argument is that not all of the walk_foo pass all of variant's state; only some of them do. I suspect we'd be better off following the proposal I gave in the description above.)

[pongad]

I'll work on this.