[cfe-commits] r152820 - in /cfe/trunk: include/clang/AST/RecursiveASTVisitor.h test/Index/index-many-logical-ops.c

[Matt Beaumont-Gay]

On Mon, Mar 19, 2012 at 13:58, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
> On Mar 16, 2012, at 8:00 PM, Matt Beaumont-Gay wrote:
>
>> Hi Argyrios,
>>
>> This changes the behavior of RAV subclasses which override the
>> Traverse* methods. Is that intended?
>
> Yes, intended. To elaborate:
>
> There are 2 uses cases for overriding Traverse* methods:
> 1) You want to skip traversing a statement.
> 2) You want to do an action before and after traversing.
>
> Both of these are incompatible with data recursion (#1 is incompatible because returning false from Traverse* will abort the whole AST traversal, not of just the current statement).
> Subclasses of RAV can either:
>
> -Return false from shouldUseDataRecursionFor() to reenable Traverse* for these specific statements that by default use data recursion.
> -To do #1 but still use data recursion, a subclass can return false from shouldUseDataRecursionFor() for the specific statement that wants skipping, then skip it in Traverse*.
>
> We could also add data-recursion specific methods to more easily do #1 and #2 while using data-recursion, though I didn't think it was high priority because currently no RAV subclass overrides Traverse* for those 3 statement kinds.

OK, just wanted to make sure it was a feature and not a bug.

Just to give you some context on the use case here, hopefully this
will land on mainline sometime in the not-too-distant future:
http://llvm.org/viewvc/llvm-project/cfe/branches/tooling/lib/ASTMatchers/ASTMatchFinder.cpp?revision=146652&view=markup
(see MatchASTVisitor::TraverseStmt)

The actual test case where I was surprised by the behavior change is
Google-internal, but there's a good chance that Manuel will add more
tests on the branch here to catch such changes in the future.

-Matt