[cfe-commits] r168022 - in /cfe/trunk: lib/ASTMatchers/ASTMatchFinder.cpp unittests/ASTMatchers/ASTMatchersTest.cpp

[Philip Craig]

On Thu, Nov 15, 2012 at 5:11 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Wed, Nov 14, 2012 at 9:04 PM, Philip Craig <philipjcraig at gmail.com> wrote:
>> On Thu, Nov 15, 2012 at 2:20 PM, Daniel Jasper <djasper at google.com> wrote:
>>> But we are not overwriting all the Traverse* methods, so I am
>>> thinking, we might not be overwriting one somewhere in the middle of
>>> the recursion and then the ones further down the chain are not called.
>
> That shouldn't matter. If you've overridden a Traverse* function, it
> should be called.
>
>> All calls to methods that can be overwritten should be via
>> getDerived(), so methods further down the chain should still be
>> called. The comment before the TRY_TO macro hints at this. There might
>> be a call somewhere that is missing the getDerived().
>
> That's not it. RAV's dataTraverse function skips the calls to
> Traverse* in some cases, to avoid using up stack space during
> traversal. This is supposed to be skipped if the Traverse* function in
> question is overridden in the derived class (see DISPATCH_WALK in
> RAV's dataTraverseNode).

Thanks for the explanation. I think I understand how RAV works a bit
better now. Once we get into dataTraverse(), we never call Traverse*
(including TraverseStmt) again until we encounter a child for which we
don't want to do data recursion. But some users such as ASTMatchFinder
override TraverseStmt in order to wrap every node traversal. (I have
the same problem in my code.)

We could add a check to skip dataTraverse if TraverseStmt is
overridden, but that doesn't sound like a good idea because presumably
we'll have stack space problems.