Having just written a small tree-walker myself, I want to ask what
returning "false" means. There's lots of ways you might want to "stop" in
the middle of a traversal:
1. Stop traversing altogether. This is what the original proposal says, if
I'm reading it correctly.
2. Stop walking the class hierarchy, but still traverse the children. This
doesn't seem very useful -- a client could probably get around this without
any real trouble.
3. Skip this node's children and move on to the next node. Would have to
override TraverseFoo() for this right now, but not so hard since you just
call WalkUpFromFoo() and don't look at the children.
4. Skip the rest of the /parent/'s children. This is definitely trickier.
5. Jump an arbitrary number of levels back up (i.e. #4 recursively).
Probably not very common, but powerful enough to implement #3, #4, and #1,
with each one wrapped in simple API.
Do you (or does anyone) think there are enough uses of #3, #4, or #5 to
warrant implementing something like this? A nice way to do it could be a
simple push()/pop() system -- when you say abort, it pop()s back to the
last saved node. Alternately, you could return a special value that says
how far back to pop() (such as a particular Stmt*), with special constants
for "Continue" and "Abort".
Overall, though, I like the separation of responsibility. I'm wondering a
little bit about Abramo's point of pre- and post- visiting -- sometimes you
might need the children first before you can finish the parent. (For
example, the type of an expression depends on its children...not that we'd
be using RecursiveASTVisitor to figure out types.)
Jordy
Having just written a small tree-walker myself, I want to ask what
returning "false" means. There's lots of ways you might want to "stop" in
the middle of a traversal:
1. Stop traversing altogether. This is what the original proposal says, if
I'm reading it correctly.
Yes, that's what I meant. Think of it as throwing an exception.
2. Stop walking the class hierarchy, but still traverse the children. This
doesn't seem very useful -- a client could probably get around this without
any real trouble.
Agreed.
3. Skip this node's children and move on to the next node. Would have to
override TraverseFoo() for this right now, but not so hard since you just
call WalkUpFromFoo() and don't look at the children.
Agreed. It's easy for a user to do this.
4. Skip the rest of the /parent/'s children. This is definitely trickier.
You can override TraverseParent(). Doable.
5. Jump an arbitrary number of levels back up (i.e. #4 recursively).
Probably not very common, but powerful enough to implement #3, #4, and #1,
with each one wrapped in simple API.
Do you (or does anyone) think there are enough uses of #3, #4, or #5 to
warrant implementing something like this? A nice way to do it could be a
simple push()/pop() system -- when you say abort, it pop()s back to the
last saved node. Alternately, you could return a special value that says
how far back to pop() (such as a particular Stmt*), with special constants
for "Continue" and "Abort".
I suspect we are getting a bit ahead of ourselves here. I like to
make the design as general as possible too - as long as we don't
introduce unnecessary complexity. I think the use cases you described
are rare, and I'm fine with not supporting them directly. The user
can always override enough methods to make the AST visitor do whatever
he wants to do. Or he may just create a new AST visitor class if that
makes more sense.
Overall, though, I like the separation of responsibility. I'm wondering a
little bit about Abramo's point of pre- and post- visiting -- sometimes you
might need the children first before you can finish the parent. (For
example, the type of an expression depends on its children...not that we'd
be using RecursiveASTVisitor to figure out types.)
Support for postorder traversal can be added later without breaking
existing users. We just need to add a bunch of PostVisitFoo() methods
and change WalkUpFromFoo()'s body to:
{
VisitFoo(x);
WalkUpFromBar(x);
PostVisitFoo(x); // This is new.
}
Therefore I don't intend to bloat the API with this upfront.
Thanks!
Having just written a small tree-walker myself, I want to ask what
returning "false" means. There's lots of ways you might want to "stop"
in
the middle of a traversal:
1. Stop traversing altogether. This is what the original proposal says,
if
I'm reading it correctly.
Yes, that's what I meant. Think of it as throwing an exception.
<snip>
Do you (or does anyone) think there are enough uses of #3, #4, or #5 to
warrant implementing something like this? A nice way to do it could be
a
simple push()/pop() system -- when you say abort, it pop()s back to the
last saved node. Alternately, you could return a special value that
says
how far back to pop() (such as a particular Stmt*), with special
constants
for "Continue" and "Abort".
I suspect we are getting a bit ahead of ourselves here. I like to
make the design as general as possible too - as long as we don't
introduce unnecessary complexity. I think the use cases you described
are rare, and I'm fine with not supporting them directly. The user
can always override enough methods to make the AST visitor do whatever
he wants to do. Or he may just create a new AST visitor class if that
makes more sense.
Heh, I suppose so. I started out writing with the intent to make sure
"stop" was well-defined (and eventually, clearly documented), but then got
carried away with all the possible uses. Of course it's easy to modify
behavior by subclasing RecursiveASTVisitor.
Thanks,
Jordy