On Tue, May 24, 2011 at 10:24 AM, Nico Weber <[email protected]> wrote:
> On Tue, May 24, 2011 at 10:03 AM, Manuel Klimek <[email protected]> wrote:
>> On Tue, May 24, 2011 at 8:59 AM, Nico Weber <[email protected]> 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 <[email protected]> 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
>>
>

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to