[PATCH] D14506: Porting shouldVisitImplicitCode to DataRecursiveASTVisitor.

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 17 01:27:37 PST 2015


On Tue, Nov 17, 2015 at 10:14 AM Richard Smith <richard at metafoo.co.uk>
wrote:

> LOn Nov 17, 2015 12:49 AM, "Manuel Klimek" <klimek at google.com> wrote:
> > Richard, this is still optional, right? (the AST matchers need to
> control visitation)
>
> This doesn't change RAV semantics at all (or if it does, it's a bug). If
> any of the extension points are overridden in the derived class, data
> recursion is automatically disabled for that case.
>

Ah, cool, sg.

> > On Tue, Nov 17, 2015 at 2:26 AM Argyrios Kyrtzidis via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >>
> >> W00t! That’s awesome Richard!
> >>
> >>> On Nov 16, 2015, at 5:10 PM, Richard Smith <richard at metafoo.co.uk>
> 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> 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> wrote:
> >>>>>>
> >>>>>> LGTM.
> >>>>>>
> >>>>>> > On Nov 16, 2015, at 12:32 PM, Ben Craig <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
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>>>
> >>>
> >>> <make-rav-fully-data-recursive.diff>
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151117/73ebd845/attachment.html>


More information about the cfe-commits mailing list