[cfe-commits] Patch: AST Matcher Framwork and an example tool

[Manuel Klimek]

On Tue, May 24, 2011 at 10:24 AM, Nico Weber <thakis at chromium.org> wrote:
> On Tue, May 24, 2011 at 10:03 AM, Manuel Klimek <klimek at google.com> wrote:
>> On Tue, May 24, 2011 at 8:59 AM, Nico Weber <thakis at chromium.org> wrote:
>>>
>>> Hi Manuel,
>>> this looks pretty cool.
>>> However, as far as I understand, MatchFinder can only be used in stand-alone tools using the Tooling infrastructure, because the public api is mostly limited to NewFrontendActionFactory(). Do you think it's possible to somehow make MatchASTConsumer public as well (maybe just give MatchFinder a NewASTCosumer() function), so that the DSL can be used in places were ASTConsumers are needed (such as in the "normal" rewriter infrastructure)? I only skimmed the patch, so sorry if that's a superficial question.
>>
>> Good question. So far I have no idea what the cases would be where
>> we'd want to use an ASTConsumer instead of a FrontendAction... I'm
>> currently working on a patch (based on this one) that integrates the
>> ASTMatcher stuff with the Rewriter to get in-process refactorings -
>> and so far I've not hit any roadblocks with regard to the
>> FrontendAction.
>> Do you have a code example you can point to where an ASTConsumer is required?
>
> Plugins, for example. Chromium's style checker (
> http://codesearch.google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/tools/clang/plugins/FindBadConstructs.cpp&q=findbadconstructs.cpp&sa=N&cd=1&ct=rc
> ) might be able to use the DSL parts if the ASTConsumer is exposed.
> Anyway, this is a relatively minor point for this patch (and I can't
> comment on the bigger points :-) ), so I don't want to derail this
> thread too much.

That's a really good suggestion, and I'd like to address this in a
follow-up patch by implementing an example plugin that uses the
matcher API. This patch is already way too big for my taste...

Cheers,
/Manuel

>
>> In general, I'm not completely opposed to make the MatchASTConsumer
>> public, but slightly wary, as it opens up a bigger intreface (needing
>> more details about the innards of clang), and I'd like to avoid it
>> unless there is a compelling use case.
>>
>> Cheers & thanks for the feedback,
>> /Manuel
>>
>>> Nico
>>>
>>> On Mon, May 23, 2011 at 3:03 PM, Manuel Klimek <klimek at google.com> wrote:
>>>>
>>>> This patch implements an AST matching framework that allows to write tools that match on the C++ ASTs. The main interface is in ASTMatchers.h, an example implementation of a tool that removes redundant .c_str() calls is in the example RemoveCStrCalls.cpp (patch to llvm/clang produced by running this tool will be sent out shortly in an extra email).
>>>>
>>>> Currently we have an in-language DSL that allows to write expressions such as (taken from the .c_str() example):
>>>> ConstructorCall(
>>>> HasDeclaration(Method(HasName(StringConstructor))),
>>>> ArgumentCountIs(2),
>>>> // The first argument must have the form x.c_str() or p->c_str()
>>>> // where the method is string::c_str(). We can use the copy
>>>> // constructor of string instead (or the compiler might share
>>>> // the string object).
>>>> HasArgument(
>>>> 0,
>>>> Id("call", Call(
>>>> Callee(Id("member", MemberExpression())),
>>>> Callee(Method(HasName(StringCStrMethod))),
>>>> On(Id("arg", Expression()))))),
>>>> // The second argument is the alloc object which must not be
>>>> // present explicitly.
>>>> HasArgument(
>>>> 1,
>>>> DefaultArgument()))
>>>> The next steps will be to build up better support for in-process refactorings based on the Rewriter, to build up higher-level matchers for common patterns, and to extend the low-level matcher library.
>>>> (rietveld link: http://codereview.appspot.com/4552059/)
>>>> Cheers,
>>>> /Manuel
>>
>