On Mon, Nov 04, 2013 at 06:50:08AM -0800, Felix S Klock II wrote:
> Also, I was a little surprised to see the addition of
> `visit_opt_lifetime_ref`, `visit_lifetime_ref`, and
> `visit_lifetime_decl`; its not like we have analogous methods for
> visiting references to type parameters or (value-expression)
> variables.

I chose to include `opt_lifetime_ref` because when you have a type
expression like `&Foo` there is a "spot" for a lifetime that is empty
-- this "spot" is what is being visited. Perhaps it needs a better
name.

As for distinguishing between lifetime refs and lifetime decls, in the
AST there are distinct AST node types for type declarations and
references, whereas we reuse `ast::Lifetime` for both. Still the two
play semantically distinct roles. We could, and perhaps should, modify
the AST to use distinct types, even though they share the same
content.

> That, combined with the observation that there are no external
> clients of `visit_lifetime_decl` nor `visit_opt_lifetime_ref`, and
> only one invocation of `visit_lifetime_ref`, makes me wonder if we
> would be better off leaving these methods off the `Visitor` trait
> (and instead implement the necessary substructure traversal in
> `resolve_lifetime.rs` alone.

Possibly, I originally was making better use of those methods, but in
the end it's true I didn't. Still, I'm inclined to have the visitor
trait be as complete as possible -- we often cut corners there,
leaving off methods and so forth, and I think it makes working with
the visitor more annoying than it has to be. But perhaps it's
better to take the good ol' YAGNI approach.