[cfe-commits] [Patch] Change the uninitialized field check to use an EvaluatedExprVisitor which will catch more use of uninitialized use.

[Richard Trieu]

On Wed, Jun 13, 2012 at 4:44 PM, Richard Smith <richard at metafoo.co.uk>wrote:

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

Committed at r158477.  With the checking for additional expressions and
test cases for them.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120614/7db5891a/attachment.html>