[PATCH] D14506: Porting shouldVisitImplicitCode to DataRecursiveASTVisitor.

Craig, Ben via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 17 07:31:51 PST 2015


Looks good to me.  I'm not too worried about the compactness of the 
visitor class.

On 11/16/2015 7:10 PM, Richard Smith wrote:
> Attached patch makes RAV fully data-recursive when visiting 
> statements, except in cases where the derived class could tell the 
> difference (when it falls back to a normal recursive walk). The queue 
> representation is slightly less compact than before: instead of 
> storing a child iterator, we now store a list of all children. This 
> allows us to handle any Stmt subclass that we can traverse, not just 
> those ones that finish by traversing all their children in the usual 
> order.
>
> Thoughts?
>
> On Mon, Nov 16, 2015 at 2:28 PM, Craig, Ben via cfe-commits 
> <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>
>     I'm fine with this approach.  How about I leave the file in place,
>     but replace the contents with a "using DataRecursiveASTVisitor =
>     RecursiveASTVisitor;" and see what breaks?  That way I won't need
>     to go through a large retrofit.
>
>
>     On 11/16/2015 3:28 PM, Richard Smith wrote:
>>     Rather than trying to maintain the horrible duplication between
>>     DataRecursiveASTVisitor and RecursiveASTVisitor, can we just
>>     delete DataRecursiveASTVisitor? RecursiveASTVisitor is
>>     data-recursive too these days (and has a smarter implementation
>>     than DataRecursiveASTVisitor's from what I can see), but doesn't
>>     yet apply data recursion in so many cases.
>>
>>     On Mon, Nov 16, 2015 at 1:07 PM, Argyrios Kyrtzidis
>>     <akyrtzi at gmail.com <mailto:akyrtzi at gmail.com>> wrote:
>>
>>         LGTM.
>>
>>         > On Nov 16, 2015, at 12:32 PM, Ben Craig
>>         <ben.craig at codeaurora.org <mailto:ben.craig at codeaurora.org>>
>>         wrote:
>>         >
>>         > bcraig added a comment.
>>         >
>>         > Ping.  Note that the test is basically a copy / paste job,
>>         and the new code in DataRecursiveASTVisitor.h is a very
>>         direct translation from the 'regular' RecursiveASTVisitor.h.
>>         >
>>         >
>>         > http://reviews.llvm.org/D14506
>>         >
>>         >
>>         >
>>
>>
>
>     -- 
>     Employee of Qualcomm Innovation Center, Inc.
>     Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
>
>     _______________________________________________
>     cfe-commits mailing list
>     cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>     http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151117/b6d872f9/attachment-0001.html>


More information about the cfe-commits mailing list