[cfe-commits] [PATCH] RecursiveASTVisitor: fix type traversal for EnumDecl

[Philip Craig]

On Mon, Sep 3, 2012 at 8:14 PM, Manuel Klimek <klimek at google.com> wrote:
> On Sat, Sep 1, 2012 at 7:41 AM, Philip Craig <philipjcraig at gmail.com> wrote:
>> RecursiveASTVisitor was traversing D->getTypeForDecl() for EnumDecl,
>> but shouldn't (same as for other TagDecl). It also wasn't traversing
>> the C++11 integer type.
>
> Don't we still visit the type as part of the typeloc traversal after your patch?

We still visit a type, but it's a different type. Previously it
visited an EnumType (which is the type that is the result of this
declaration, not part of it), but now it visits whatever type has been
specified, if any (such as BuiltinType for int). Maybe I should have
split this into separate patches?

> Perhaps add a negative test for what we don't want to visit any more?

I can if you think it's a good idea. I didn't for two reasons.
- There's no end of things you could test for the absence of, is this
important enough?
- Most of the other TypeDecl are already skipping this type, and have
comments to that effect, but no tests. Should they have tests too?

The way I'd prefer to test this is to visit everything, compare that
against a whitelist of things we expected to visit, and fail the test
if anything unexpected was visited, rather than having a blacklist of
things not to visit. That seems like it will need a lot of test
framework changes though.