[cfe-dev] warn when ctor-initializer leaves uninitialized data?

[Evan Wallace]

On Mon, Apr 9, 2012 at 1:55 PM, David Blaikie wrote:

> Hi Evan,
>
> Thanks for looking into this. Did you mean to include a patch file of
> your current changes?
>

Sure, glad there's interest! I wanted to see if I was on track before
submitting a patch. I've attached a patch now.


> We'd have to see how accurate this diagnostic was (in finding bugs,
> not noise) - my hunch would be that it's probably not accurate enough
> to turn on by default, at least. Though that doesn't mean it can't
> live under -Wuninitialized (I haven't checked what else is there, how
> accurate they are, etc).
>

I agree, I wouldn't turn it on by default as a lot of existing code
sometimes doesn't initialize members in constructor initializers for a
variety of reasons (such as performance or discriminating unions like you
mentioned). It's been great for the few personal projects I've tested
though and all the warnings it generated were legitimate. It looks like
-Wuninitialized is currently more for local flow-based uninitialized
variable analysis although it is logically similar to this warning.

A few questions (I think I mentioned these on IRC, perhaps - or just
> crossed my mind):
>
> 1) how well does this handle unions? struct S { union { int x, y; };
> S() : x() {} }; - as an addition to this. I was wondering if we could
> roll this checking up into the same machinery we need/could use/have
> some of for errors relating to competing initialization (eg: we
> currently correctly produce an error when you try to initialize both x
> and y in my example)
>

I just tested my change further and it doesn't warn on union members,
unnamed structs and bitfields, or double-warn about uninitialized members
in superclasses:

// warning about a
struct A { int a; };

// no warnings
struct B { A b; unsigned : 8; };

// no warnings
struct C { int c; C() : c() {} };

// warning about d but not a
struct D : A { int d; D() {} };

// warning about e2
struct E { int e1, e2; E() : e1() {} };

// two warnings about f, one for each constructor
struct F { int f; F() {} F(int) : f() {} F(int, int) {} };

// no warnings for union members
struct G { union { int g1, g2; }; };

// no warnings for a separate constructor
struct H { int h; H(); };
H::H() : h() {}

// warning for anonymous struct members (only when used do to laziness)
struct I { struct { int i; }; };

// warning about j (Wuninitialized, not Wuninitialized-member)
struct J { int j; J() : j(j) {} };

I'll make more test cases and add them to clang's tests soon.


> 2) Is there any way to silence the warning without introducing
> initializations that may not be required/desired? (one interesting
> case I came across in Clang recently is, effectively, a discriminated
> union - some members are used/unused based on which enum value is
> currently held - there's no need to initialize the other members as
> they are dead)
>

The way to silence the warning is currently through clang's diagnostic
pragmas, which would be another reason to have the warning be separate from
-Wuninitialized. I can look into adding something like
__attribute__((uninitialized)) if you think that would be preferable. When
I'm ready, should I submit my patch to cfe-commits?

- Evan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120409/52426543/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Wuninitialized-member.diff
Type: application/octet-stream
Size: 1951 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120409/52426543/attachment.obj>