<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105307/">http://git.reviewboard.kde.org/r/105307/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 18th, 2012, 12:32 p.m., <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/105307/diff/1/?file=69936#file69936line523" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/parser/tests/test_parser.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TestParser::testPreprocessor() {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">523</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QEXPECT_FAIL</span><span class="p">(</span><span class="s">""</span><span class="p">,</span> <span class="s">"Variadic arguments cannot be left empty (GCC extension)"</span><span class="p">,</span> <span class="n">Continue</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't get this comment: I thought the variadic argument *can* be left empty? what is actually happening now in kdevelop?</pre>
</blockquote>
<p>On July 19th, 2012, 9:45 a.m., <b>Alexandre Courbot</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;">Yes, that's precisely what the comment says: variadic arguments normally can be left empty, and this comment is about the error condition, which happens when they cannot.</pre>
</blockquote>
<p>On July 19th, 2012, 2:26 p.m., <b>Milian Wolff</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;">I'm sorry but I need some more info as I'm still not understanding this :-S
a) I understand from your review description, that this patch will allow KDevelop to properly handle empty variadic macro arguments
b) you add a test but expect it to fail, with the explanation that variadic args *cannot* be left empty
c) what is the actual result of the test you added there, if not "kdevelop"? which should be the result if we properly handle the empty variadic macro, no?
or is your patch only "fixing" the error reporting, i.e. empty variadic macro arguments don't report an error anymore, but still don't work properly hence the result is not "kdevelop"? If so, then please change the test fail comment to something like "Empty variadic argument not properly handled".
</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The key is that sentence in http://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html: "GNU CPP has a pair of extensions which deal with this problem. First, you are allowed to leave the variable argument out entirely".
This behavior was not properly implemented - i.e. if you left the variable argument empty in the (args...) form, the parser would trigger an error. The (args, ...) form was correctly handled though.
So this is just what this patch fixes. The additional test case also attempts to use the same macro with its variable argument and without.</pre>
<br />
<p>- Alexandre</p>
<br />
<p>On June 20th, 2012, 1:51 p.m., Alexandre Courbot wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Alexandre Courbot.</div>
<p style="color: grey;"><i>Updated June 20, 2012, 1:51 p.m.</i></p>
<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;">In GNU C, the variadic argument of a macro can be completely left out
without triggering an error (as explained in
http://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html)
</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;">Checked that the faulty case was not triggering a parsing error anymore. Checked that no regression was introduced in the testPreprocessor() test case.</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/cpp/parser/rpp/pp-engine.cpp <span style="color: grey">(ca566cb)</span></li>
<li>languages/cpp/parser/tests/test_parser.cpp <span style="color: grey">(804f379)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105307/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>