<div style="font-family: arial, helvetica, sans-serif"><font size="2">Hi Stephen,<div><br></div><div>first, really cool blog post, and thanks for providing feedback & docs and helping us making the tools more awesome - we really appreciate this!<br>
<br><div class="gmail_quote">On Mon, Jun 18, 2012 at 1:22 PM, Stephen Kelly <span dir="ltr"><<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Hi there,<br>
<br>
A few weeks ago I was getting started with the tooling branch to do some<br>
porting from Qt 4 to Qt 5 (I was off topic though :)):<br>
<br>
<a href="http://thread.gmane.org/gmane.comp.compilers.clang.devel/20433/focus=20575" target="_blank">http://thread.gmane.org/gmane.comp.compilers.clang.devel/20433/focus=20575</a><br>
<br>
I wrote a blog post with some results here:<br>
<br>
<a href="http://www.kdab.com/automated-porting-from-qt-4-to-qt-5/" target="_blank">http://www.kdab.com/automated-porting-from-qt-4-to-qt-5/</a><br>
<br>
I also wrote the following to colleagues, and thought I'd get some fact<br>
checking on it from the people who might know, so that the new API in the<br>
branch can be documented better:<br>
<br>
<br>
[Begin copy-paste]<br>
<br>
The tool works by using the CMake integration to find out what command<br>
should be used to compile the file passed to it for porting. It then parses<br>
that file and runs a visitor pattern over the parsed AST. While visiting, it<br>
checks to see if the node matches what was specified. When it gets a match<br>
it invokes the run() method of a callback.<br>
<br>
The run() method is the one that sets up a replacement for the matched AST<br>
node with some text. The text can be the new method being ported to, or some<br>
other generated string.<br>
<br>
The matched nodes can be extracted by their id and type. The type takes a<br>
while to get used to because there are many of them describing all the<br>
different parts of C++ syntax.<br>
<br>
<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html" target="_blank">http://clang.llvm.org/doxygen/classclang_1_1Decl.html</a><br>
<br>
Once we have the replacement text, we add it to the Replacements structure,<br>
which is processed after matching to to the actual changes.<br>
<br>
Knowing how to write a matcher or what type to use to extract a node by id<br>
can be tricky, but can be made easier by running 'clang -cc1 -ast-dump<br>
myfile.cpp' to see what types the parser sees.<br>
<br>
After that, the matches are actually nested function calls (the<br>
capitalisation style for functions seems funny to use Qt developers). Nested<br></blockquote><div><br></div><div>Yea, that's not set yet - please chime in with your color of the bikeshed on the review thread for the ast matchers ;) I seriously don't care, so I want people who care about one way or the other to jump in with good arguments of why their preferred way is superior :D</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
matches are usually specializations.<br>
<br>
So<br>
<br>
Expression()<br>
<br>
matches any expression. As does<br>
<br>
Expression(Expression())<br>
<br>
The inner one is redundant.<br>
<br>
Expression(Function())<br>
<br>
matches an expression which is a function, not an expression which contains<br>
inside it a function.<br>
<br>
child relationships are found with has()<br>
<br>
Expression(has(Function()))<br>
<br>
finds for example<br>
<br>
foo(bar())<br>
<br>
where foo() would be the Expression() and bar() would be the Function().<br>
<br>
To use these in the callback, we would write<br>
<br>
Id("expr", Expression(has(Id("func", Function()))))<br>
<br>
and then<br>
<br>
const Expr *Outer =<br>
Result.Nodes.getStmtAs<Expr>("expr");<br>
const Expr *Inner =<br>
Result.Nodes.getStmtAs<Expr>("func");<br>
<br>
<br>
Match expressions next to each other are by default grouped in an AllOf<br>
expression.<br>
<br>
So<br>
<br>
Method(<br>
HasName("dataChanged"),<br>
OfClass(<br>
IsDerivedFrom("QAbstractItemView")<br>
)<br>
)<br>
<br>
means 'A method with the name dataChanged where that method is on a class<br>
derived from QAbstractItemView'<br>
<br>
You do get interesting compile errors sometimes when writing matcher<br>
constructs. With enough playing around and reading other matchers I've been<br>
able to get it to do what I want each time eventually, and the result was<br>
obvious why it was correct.<br>
<br>
Apart from the code I've attached, to see what's possible (built in), you<br>
can<br>
reference the following:<br>
<br>
# This is the one referenced in the youtube talk above:<br>
<a href="http://llvm.org/svn/llvm-project/cfe/branches/tooling/tools/remove-cstr-
calls/RemoveCStrCalls.cpp" target="_blank">http://llvm.org/svn/llvm-project/cfe/branches/tooling/tools/remove-cstr-<br>
calls/RemoveCStrCalls.cpp</a><br>
<br>
# Most of the implementation<br>
<a href="http://llvm.org/svn/llvm-" target="_blank">http://llvm.org/svn/llvm-</a><br>
project/cfe/branches/tooling/include/clang/ASTMatchers/ASTMatchers.h<br>
<br>
# Unit tests<br>
<a href="http://llvm.org/svn/llvm-" target="_blank">http://llvm.org/svn/llvm-</a><br>
project/cfe/branches/tooling/unittests/ASTMatchers/ASTMatchersTest.cpp<br>
<br>
[End copy-paste]<br></blockquote><div><br></div><div>All accurate.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I was also wondering if there is an easy way to only modify the code inside<br>
the repo and not included headers? In my tool I require the user to pass the<br>
source-dir on the command line, and then manually check paths to see if they<br>
are below it, but this is error-prone.<br></blockquote><div><br></div><div>If with "manually", you mean "do a regexp match on the path", then yep, that's what I also do.</div><div>Why is that error prone?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Additionally, many of the concepts I encoded are generic, not Qt specific<br>
(such as renaming methods, enums etc). It seems that these could be re-<br>
usable, so we could either add some version of this to the clang examples,<br>
and/or see if we can define a higher level collection of ways to refactor<br>
the code. I presume google also has some collection/repo of such things<br>
already?<br></blockquote><div><br></div><div>Unfortunately not. Most of the engineers using those tools at Google have pretty non-standard and complex problems, at which point other costs start to dwarf writing the matchers and replacements.</div>
<div><br></div><div>But we are starting to work on a more principled approach that includes ready-made refactorings, targeted towards small scale use cases ("I want to rename what is under my cursor", we actually have a prototype for this in the branch).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Finally, the suggestion here:<br>
<br>
<a href="http://thread.gmane.org/gmane.comp.compilers.clang.devel/20433/focus=20759" target="_blank">http://thread.gmane.org/gmane.comp.compilers.clang.devel/20433/focus=20759</a><br>
<br>
was not quite what I needed to use to correctly match the code used. The<br>
matcher I used was this:<br>
<br>
Finder.addMatcher(<br>
Id("call",<br>
Call(<br>
Callee(Function(HasName(QtEscapeFunction))),<br>
HasArgument(<br>
0,<br>
AnyOf(<br>
BindTemporaryExpression(has(Id("ctor", ConstructorCall()))),<br>
BindTemporaryExpression(has(Id("operator",<br>
OverloadedOperatorCall()))),<br>
Id("operator", OverloadedOperatorCall()),<br>
Id("ctor", ConstructorCall()),<br>
Id("expr", Expression())<br>
)<br>
)<br>
)<br>
),<br>
&Callback);<br>
<br>
So I had to add some extra BindTemporaryExpression() into the expression<br>
which I would have preferred to be automatic, like I think some implicit<br>
cast stuff is.<br></blockquote><div><br></div><div>Yep, the temporary stuff is not yet perfect. Ideas welcome, preferably when we have it in mainline ;) (there's a review thread currently open - afaik nobody answered yet).</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks for the guidance so far! :),<br>
<br>
Steve.<br>
<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
</blockquote></div><br></div></font></div>