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

[Argyrios Kyrtzidis]

On Mar 19, 2012, at 2:10 PM, Matt Beaumont-Gay wrote:

> 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.

We could eliminate overriding Traverse* methods, and switch to overriding

bool beforeTraverse*(Stmt *, bool &skip);
bool afterTraverse*(Stmt *);

which will work the same way with either data/call recursion.

> 
> -Matt