[cfe-commits] [PATCH] AST matchers

[Stephen Kelly]

Manuel Klimek wrote:

> ... one of the final pieces that we have in the tooling branch are the AST
> matchers.
> 
> The main user interface is in ASTMatchers.h and ASTMatchFinder.h.

Hi,

There are some case mis-matches between documentation and implementation 
with Has/has forEach/ForEach and hasDescendant/HasDescendant, 
forEachDescendant:

+/// Example matches X, Y (matcher = Class(Has(Class(HasName("X")))
...
+template <typename ChildT>
+internal::ArgumentAdaptingMatcher<internal::HasMatcher, ChildT> has(

Apart from that I think it's a good start. Is it considered experimental or 
'stable' in any way, or is it allowed to change in any way at all (source 
incompatibly)?

I think there may be a need for some more high-level documentation, but that 
could also come later if the API is 'experimental' at this point. The scope 
of expected use might also be worth defining. 

Is it primarily for non-standard, complex problems and one-time-use tools 
(like solving Googles problems), or is it primarily for creation of sharable 
tools which can operate on any codebase (like an accurate, medium/long-term 
maintained porting tool or other medium-term maintained tools)? 

I consider this a documentation problem because I think there are ways to 
make the tool I created faster by storing information about compilations 
before doing the porting. Currently I use 'git grep' to find potential 
porting cases, which means that I might needlessly be compiling certain TUs 
multiple times because of a text match but not a semantic match.

https://github.com/KDAB/Qt4to5/blob/master/portqt4to5.py

I think this is something Chandler also mentioned there was a solution for 
in his talk (that Google records lots of information from compiles).

I don't expect all of that to be in this patch of course, but it would be 
interesting to confirm that that's the kind of things that are in scope and 
can be added in the future.

Given the extensive comments, unit tests, and the fact that I've also used 
this API a bit, I support applying this patch.

Thanks,

Steve.