[PATCH] First revision of the dynamic ASTMatcher library

Samuel Benzaquen sbenza at google.com
Wed May 8 11:56:31 PDT 2013


On Wed, May 8, 2013 at 11:53 AM, Manuel Klimek <klimek at google.com> wrote:

>
>   My first gut feeling reaction is that we'll want a different solution
> than to make DynTypedNode more complex, and thus less predictable.
>

Do you want me to take this piece into a different CL?


>   We had a reason for have the explicit overloads in the ASTMatchFinder
> instead of just working on the interface - the problem is that you can
> otherwise hand in lots of matchers that are impossible to get a match
> with...
>

Can you give an example?
Not matching a Matcher<Type> without the conversion to Matcher<QualType>
seems like a bug to me. The visitor should visit the Type node.


>
>   Things we could consider:
>   1. take a DynTypedMatcher* and take ownership - together with clone(),
> this allows users to do everything they need, and doesn't lead to overload
> confusion.
>
If there is going to be a const DynTypedMatcher& overload, I think we
should remove the other ones. Otherwise you could have potentially
different behavior if you call the function directly or through something
that uses the interface, like we see with Matcher<Type>.

I'll send you a different CL with the change proposal.


>   2. only have the DynTypedMatcher interface, and somehow break
> dynamically with an error message
>   other ideas?
>

> http://llvm-reviews.chandlerc.com/D714
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130508/8f6bf218/attachment.html>


More information about the cfe-commits mailing list