<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/103448/">http://git.reviewboard.kde.org/r/103448/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Two short questions to help improving this patch:
1) about the use of rewind() in case of parse failure, is there a policy? Some functions do it, some others don't, and I cannot deduce clear rules for doing it in the code.
2) in case of parse failure, what happens to the AST elements that have been allocated midway? Existing code suggests that it is safe to just leave them this way and return false, but I have seen nothing that suggests these are collected later. Shouldn't them be freed?
I will try and come with a better version of this once I get answers to these. Looks like my AST construct is acceptable, which is a good point. Thanks for all the feedback so far!</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 19th, 2011, 6:46 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/103448/diff/1/?file=43720#file43720line2752" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/parser/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; ">bool Parser::parseInitializerList(InitializerListAST *&node)</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">2752</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">clauses</span><span class="p">)</span> <span class="k">break</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;">this is bad, as it is *not* valid c++, see
http://www.nongnu.org/hcb/#initializer-list
please pass the mimetype along or find it again (in a fast way, I think the language controller can do that) to find out whether you are in "c" or in "c++" mode and decide based on that.</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;">Is anything like this done to prevent C++11 constructs from being used in C++ files? If not, what is the point of being so strict about this very patch?
As David pointed there is no way to do that properly unless a C or C++ type is explicitly set for the file or project, something KDevelop does not support yet AFAIK. There are plenty, plenty of more serious things that the KDevelop parser accepts while gcc does not - i.e. incorrect type usage, non-matching number of arguments, etc. So unless we are aiming for a perfect C++ parser (in which case I would rather suggest to use LLVM/CLang's), let's not argue over a trailing comma.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 19th, 2011, 6:46 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/103448/diff/1/?file=43720#file43720line2845" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/parser/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; ">bool Parser::parseInitializerList(InitializerListAST *&node)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2777</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">parseAssignmentExpression</span><span class="p">(</span><span class="n">expr</span><span class="p">)</span> <span class="o">||</span> <span class="n">parseBracedInitList</span><span class="p">(</span><span class="n">expr</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2845</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">parseBracedInitList</span><span class="p">(</span><span class="n">expr</span><span class="p">)</span> <span class="o">||</span> <span class="n">parseDesignatedInitializer</span><span class="p">(</span><span class="n">expr</span><span class="p">)</span> <span class="o">||</span> <span class="n">parseAssignmentExpression</span><span class="p">(</span><span class="n">expr</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;">was the change in order intentional? I'd rather have the parseDesignatedInitializer as last resort, as it will not occur in most code (as it's a C feature and we are currently mostly aiming at c++)</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;">It is intentional. In the case of nested initializers, parseAssignmentExpression() would incorrectly take the nested one for some unknown reason. I can try to fix it and switch back to the previous order.</pre>
<br />
<p>- Alexandre</p>
<br />
<p>On December 18th, 2011, 12:23 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 and Milian Wolff.</div>
<div>By Alexandre Courbot.</div>
<p style="color: grey;"><i>Updated Dec. 18, 2011, 12:23 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;">Parser support for C99 designated initializers
Support C99 initializers in the C++ parser, e.g:
struct foo_t foo = {
.has_cake = true,
.nb_candles = 12,
};
int bar[10] = {
[1] = 15,
[9] = 25,
};</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;">Used it for a couple of weeks, ensured the parser tests all pass.</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/parser.h <span style="color: grey">(ffc3967e9bec09ff56204aab98e8f80ec6b036cf)</span></li>
<li>languages/cpp/parser/parser.cpp <span style="color: grey">(1c9d9e403500f35761ebc6deb737a4a68a53c28d)</span></li>
<li>languages/cpp/parser/tests/test_parser.h <span style="color: grey">(fa92f1ce284df0936724f74f42f9ad6d4b3c97fc)</span></li>
<li>languages/cpp/parser/tests/test_parser.cpp <span style="color: grey">(f747cfa44bdf962be33cf97841fdae739b3e1771)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103448/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>