<div style="font-family: arial, helvetica, sans-serif"><font size="2"><div class="gmail_quote">On Wed, Jun 13, 2012 at 4:44 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Jun 1, 2012 at 3:09 PM, Richard Trieu <rtrieu at <a href="http://google.com" target="_blank">google.com</a>> wrote:<br>

> On Wed, May 30, 2012 at 5:37 PM, Chandler Carruth <chandlerc at <a href="http://google.com" target="_blank">google.com</a>>wrote:<br>
<div class="im">><br>
>> On Fri, May 18, 2012 at 3:35 PM, Richard Trieu <rtrieu at <a href="http://google.com" target="_blank">google.com</a>> wrote:<br>
>><br>
>>> Update the uninitialized field checker to catch more cases of<br>
>>> uninitialized use.  This will catch fields passed by value to function<br>
>>> calls and properly parse conditional operators.  Instead of a recursive<br>
>>> function, an EvaluatedExprVisitor will be used.<br>
>>><br>
>><br>
>> What's the compile-time impact of this? Maybe synthesize a test case with<br>
>> a crazy number of fields (all initialized) with long expressions in their<br>
>> initializers, and compare before and after? (There are some tests in INPUTS<br>
>> that you can model this after...)<br>
>><br>
><br>
> Synthesized some test cases with 4096 classes, each class having a single<br>
> field initializer with 2048 sub expressions.  My testing showed a 5-10%<br>
> slow-down for the new EvaluatedExprVisitor compared to the current<br>
> recursive function check.<br>
<br>
</div>Is that just for the check itself, or an entire -fsyntax-only compile?<br>
If it's the former, that sounds completely reasonable, given that this<br>
is able to catch many more bugs. If it's the latter, it would be worth<br>
at least wrapping the CheckInitExprContainsUninitializedFields loop<br>
with a test for diag::warn_field_is_uninit being enabled.<br>
<br>
You have a couple of "T* p"s in the patch which should be "T *p".<br>
Also, HandleValue should recurse into the RHS of a comma operator and<br>
the LHS of a .* or ->*, as well as BinaryConditionalOperator<br>
(basically, this is checking the set of potential results of the<br>
expression, as defined in CWG issue 712).<br>
<br>
Other than that, LGTM.<br>
</blockquote></div><div><br></div>Committed at r158477.  With the checking for additional expressions and test cases for them.</font></div>