PATCH: update RecursiveASTVisitor to visit attributes.

[Delesley Hutchins]

Thanks for the review!  New patch enclosed, now tested so that it
actually works.  :-)

I notice that we actually have two versions of RecursiveASTVisitor.h;
the second is in tools/libclang.  There are non-trivial differences
between the two, and this is a parallel maintenance problem, as the
two implementations are drifting apart.  What is the rationale for
having two versions?  Should I update both?  The test code doesn't
test the second version.

> Would it make sense for this to take a const pointer? Or perhaps even
> const reference?

Most of the other routines take non-const pointers, so I think this is
consistent with the rest of the API.

> This would probably be better served by using an attr_iterator via
> attr_begin/attr_end; it would then fix an assert crash which will
> happen if the Decl has no attributes attached to it.

Nice catch!  Done.

>> +    if (!R.getValueAsBit("ASTNode"))
>> +      continue;
>
> Do we want to filter out any statement or type attributes?

Ideally, we want RecursiveASTVisitor to visit every attribute in the
AST.  This patch only handles attributes on declarations thus far, but
tablegen should emit code for all of them; the visitor can then be
easily updated in subsequent patches.  ASTNode seems to be used as a
filter elsewhere.   Would you suggest something else?

> Spurious curly braces; but I'm wondering whether this for loop is even
> required. Couldn't you accomplish the same thing without the Args
> vector by just calling writeASTVisitorTraversal after creating the
> Argument object?

Done.  This loop structure has been cut-and-pasted throughout the
file, but you're right -- it doesn't make too much sense here.

> On a side note, I just realized we're leaking those Argument objects
> all over the place. Oops.

True.  But it doesn't matter, since this code is only run once during
compile.  Every function in the file leaks the arguments.  :-)

  -DeLesley

-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RecursiveASTVisitor.patch
Type: text/x-patch
Size: 13424 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131227/febc5fbc/attachment.bin>