<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124256/">https://git.reviewboard.kde.org/r/124256/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Lipiec 6th, 2015, 7:47 po południu CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What about getting some unit testing? Sounds like an important piece of code...</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Its open problem. To make real-world test (more than one TU) we need compilation database. It means that we have to create real files (or write even more code to create reasonable FixedCompilationDatabase).
Even then we have to think about test of this code and not AST building code or AST matching code (which is implemented in Clang). The problem I currently have is that I can easily support all code bases I can imagine (but only correct), but I can't guarantee that these imaginations are "complete" (that I predicted everything). The problem is to check if really everything is supported, not the features I predicted (because everything is done by Clang and I assume there is no reason to test its correctness). The only thing which requires testing is my analysis. Take look at this:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">void f()
{
int x = 5;
x *= 2;
}
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Rename <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">x</code> to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">asdf</code>. Nice test. The problem is that after picking apropriate method, rest of task is done by Clang. What is worse - making setup of this test would be far more complicated than <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">my</em> code executed during refactoring.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Generaly in refactorings from this family my problem is to decide which symbols should be renamed. When I make such decision Clang can bring to me all information I need. It also gives me tool which I can use to say: "replace this <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">x</em> by that <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">y</em>". The only problem is to decide. How to check if my decisions are correct?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In above example <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">x</code> is variable without linkage - I <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">assume</em> this variable occurs only in <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">this</em> translation unit and thus compare all <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">DeclRefExpr</em> declarations canonical declaration with canonical declaration of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">x</code> (compare address of AST node, the same with <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">VarDecl</em>s). Getting canonical declaration is trivial - just call apropriate method.
What should be tested is if my reasonig is complete.</p></pre>
<br />
<p>- Maciej</p>
<br />
<p>On Lipiec 5th, 2015, 4:56 po południu CEST, Maciej Poleski wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Maciej Poleski.</div>
<p style="color: grey;"><i>Updated Lip 5, 2015, 4:56 po południu</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdev-clang
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Renaming of fields with external linkage.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Renaming of fields from classes in anonymous namespaces is also done</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Manual testing</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>clangsupport.cpp <span style="color: grey">(e22c55426a2e839ec11cbe0b2fe1e13721b0583a)</span></li>
<li>clangsupport.h <span style="color: grey">(8ed1ec90bbbc41d7c7a94d926e0951c729a6194c)</span></li>
<li>CMakeLists.txt <span style="color: grey">(875172a8407f4bd9faf330f032a280fa66c2b16f)</span></li>
<li>cmake/FindClang.cmake <span style="color: grey">(6c9bd6ef0242319122dcc7e6fd6cea8d9f0cbfbb)</span></li>
<li>refactoring/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/debug.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/debug.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/documentcache.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/documentcache.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/interface.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/interface.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/kdevrefactorings.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/kdevrefactorings.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/kdevrefactorings_disabled.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/nooprefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/nooprefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringcontext.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringcontext.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringinfo.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringinfo.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringmanager.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringmanager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamefielddeclrefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamefielddeclrefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamefielddeclturefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamefielddeclturefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamevardeclrefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamevardeclrefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/utils.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/utils.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124256/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>