On Mon, Sep 3, 2012 at 8:14 PM, Manuel Klimek <[email protected]> wrote:
> On Sat, Sep 1, 2012 at 7:41 AM, Philip Craig <[email protected]> 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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to