<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/126889/">https://git.reviewboard.kde.org/r/126889/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 26th, 2016, 4:34 p.m. UTC, <b>Kevin Funk</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;">Did you test whether this test succeeds with both the old and the new implementation?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And: Nice to see you're working on this, Pedro!</p></pre>
</blockquote>
<p>On January 26th, 2016, 5:12 p.m. UTC, <b>Pedro Ferreira</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;">Good call - my code wouln't build with the old version. I have fixed it now.
The tests, however, do not pass with the old version (ie, before updated clang). The "old" implementation returns the literal used to instanciate the template, "3" and "5". The first test passes.
I don't think that behaviour is correct, since "3" and "5" aren't types.</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;">I see. Thinking about it, libclang minor version 24 was available before Clang version 3.5 was out. Since we depend on Clang 3.5 already, we can safely drop the feature check altogether.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">-> Just use the new implementation, drop the old. But still keep the tests around, they're still useful!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks</p></pre>
<br />
<p>- Kevin</p>
<br />
<p>On January 26th, 2016, 5:21 p.m. UTC, Pedro Ferreira 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 Pedro Ferreira.</div>
<p style="color: grey;"><i>Updated Jan. 26, 2016, 5:21 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevelop
</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;">This drops a TODO message and inserts a compile-time selector between clang-based template argument parsing and what appears to be manual tokenisation.</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;">This is possibly incomplete. It works for me (I'm using a KDev built with this), but would obviously appreciate comments.
Mind you, the include header in Clang for this function ("clang_Type_getTemplateArgumentAsType") says that this function does not support variadic templates, so potentially this patch is not acceptable.</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>languages/clang/codecompletion/completionhelper.cpp <span style="color: grey">(c01185a)</span></li>
<li>languages/clang/tests/test_clangutils.h <span style="color: grey">(4508907)</span></li>
<li>languages/clang/tests/test_clangutils.cpp <span style="color: grey">(1a8ecb7)</span></li>
<li>languages/clang/util/clangutils.h <span style="color: grey">(e47058e)</span></li>
<li>languages/clang/util/clangutils.cpp <span style="color: grey">(81f7b44)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126889/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>