[cfe-dev] Where to put upcoming refcatoring tools

[Manuel Klimek]

Here's what I would do:
    Id("call",
      Call(Callee(Function(HasName("::Qt::escape"))),·
           HasArgument(0, AnyOf(Id("constructor", ConstructorCall()),
Id("expression", Expression())))))));

Now you get "constructor" bound when there's a conversion going on,
and "expression" bound otherwise.
If "constructor" is bound, replace "call" with
QString::fromUtf8(<constructor-range>), otherwise replace "call" with
(<expression>).toHtmlEscaped();

This works because HasArgument by default looks through parentheses
and implicit casts, and doing what looks like a manual constructor
call there (QString("")) is actually a functional cast.

Tweaking will be necessary if you want to prevent extra parens around
expression.
If the spelling location of <expression> or <constructor> still
doesn't work when you have a macro in there, you'll need to pimp the
getText function to also allow giving you the expansion location.

Cheers,
/Manuel

On Wed, Apr 25, 2012 at 7:13 PM, Manuel Klimek <klimek at google.com> wrote:
> On Wed, Apr 25, 2012 at 6:56 PM, Stephen Kelly <steveire at gmail.com> wrote:
>> Stephen Kelly wrote:
>>
>>> Manuel Klimek wrote:
>>>>
>>>> Does
>>>> Expresssion(AllOf(
>>>>   HasType(...),
>>>>   Not(ImplicitCast())
>>>> work?
>>>>
>>>
>>> Unfortunately not. The Not(ImplicitCast()) also makes no difference.
>>>
>>> I tried adding a ConstructorCall() in there too, guessing that would match
>>> the implicit ctor calls, but that just excluded all matches from the
>>> result.
>>>
>>> I put a call to Arg->dump() in my MatchCallback implementation. It doesn't
>>> help me, but maybe you can see how I need to define my match with that? My
>>> input is this now:
>>>
>>>     Qt::escape("foo");
>>>     QString foo("bar");
>>>     Qt::escape(foo);
>>>     QObject o;
>>>     Qt::escape(o.objectName());
>>>     Qt::escape(o.property("foo").toByteArray());
>>
>> I found that I could use this to not match the Qt::escape(foo); case.
>>
>>  Finder.addMatcher(
>>      Id("call",
>>        Call(
>>          Callee(Function(HasName("::Qt::escape"))),
>>          HasArgument(
>>            0,
>>            Id("arg", BindTemporaryExpression())
>>          )
>>        )
>>      ),
>>      &Callback);
>>
>> I eventually figured out how to limit it to only the case without
>> temporaries:
>>
>>  Finder.addMatcher(
>>      Id("call",
>>        Call(
>>          Callee(Function(HasName("::Qt::escape"))),
>>          Not(HasArgument(
>>            0,
>>            Id("arg", BindTemporaryExpression())
>>          )),
>>          HasArgument(
>>            0,
>>            Id("arg", Expression())
>>          )
>>        )
>>      ),
>>      &Callback);
>>
>> I tried many other variations using Not, which I can try to list if you're
>> interested in considering the self-documenting nature of the API.
>>
>> To match the string literal, I used:
>>
>>  Finder.addMatcher(
>>      Id("call",
>>        Call(
>>          Callee(Function(HasName("::Qt::escape"))),
>>          HasArgument(
>>            0,
>>            Id("arg", BindTemporaryExpression())
>>          ),
>>          Not(hasDescendant(MemberExpression()))
>>        )
>>      ),
>>      &Callback);
>>
>> I could not use this as it gives no results:
>>
>>  Finder.addMatcher(
>>      Id("call",
>>        Call(
>>          Callee(Function(HasName("::Qt::escape"))),
>>          HasArgument(
>>            0,
>>            Id("arg", BindTemporaryExpression())
>>          ),
>>          ast_matchers::StringLiteral()
>>        )
>>      ),
>>      &Callback);
>>
>>
>> And this one does not build:
>>
>>  Finder.addMatcher(
>>      Id("call",
>>        Call(
>>          Callee(Function(HasName("::Qt::escape"))),
>>          HasArgument(
>>            0,
>>            Id("arg", BindTemporaryExpression())
>>          ),
>>          Not(hasDescendant(ast_matchers::StringLiteral()))
>>        )
>>      ),
>>      &Callback);
>>
>>
>> Finally this matches the member calls:
>>
>>  Finder.addMatcher(
>>      Id("call",
>>        Call(
>>          Callee(Function(HasName("::Qt::escape"))),
>>          HasArgument(
>>            0,
>>            BindTemporaryExpression()
>>          ),
>>          hasDescendant(Id("arg", MemberExpression()))
>>        )
>>      ),
>>      &Callback);
>>
>> But I do not get the parentheses or the arguments inside them in the result
>> (if any). I aslo presume this would not match a call of a free function, so
>> I would need more matchers for those cases? It's already getting quite
>> exhaustive and less scalable.
>>
>> It seems either I'm missing something or the tooling is not ready to handle
>> the cases I'm trying to achieve?
>
> Ok, I'm confused by your last example. The way I usually approach this
> is that I write a small cpp file with the stuff I want to match and
> then run clang -ast-dump-xml over it, and look at the resulting AST.
> Then you basically take the matchers that drill through the
> corresponding AST nodes and put them together. I can take a look at
> your specific example tomorrow (I just reread your first explanation
> of what you're trying to do) and come up with an example how I would
> approach this. That might also make up some good documentation :)
>
> Cheers,
> /Manuel
>
>>
>> Thanks,
>>
>> Steve.
>>
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev