<div class="gmail_quote">On Mon, Apr 9, 2012 at 1:55 PM, David Blaikie wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Hi Evan,</div>

</div>
<br>
Thanks for looking into this. Did you mean to include a patch file of<br>
your current changes?<br></blockquote><div><br></div><div>Sure, glad there's interest! I wanted to see if I was on track before submitting a patch. I've attached a patch now.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


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

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
A few questions (I think I mentioned these on IRC, perhaps - or just<br>
crossed my mind):<br>
<br>
1) how well does this handle unions? struct S { union { int x, y; };<br>
S() : x() {} }; - as an addition to this. I was wondering if we could<br>
roll this checking up into the same machinery we need/could use/have<br>
some of for errors relating to competing initialization (eg: we<br>
currently correctly produce an error when you try to initialize both x<br>
and y in my example)<br></blockquote><div><br></div><div><div>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:</div>

<div><br></div><div>// warning about a</div><div>struct A { int a; };</div><div><br></div><div>// no warnings</div><div>struct B { A b; unsigned : 8; };</div><div><br></div><div>// no warnings</div><div>struct C { int c; C() : c() {} };</div>

<div><br></div><div>// warning about d but not a</div><div>struct D : A { int d; D() {} };</div><div><br></div><div>// warning about e2</div><div>struct E { int e1, e2; E() : e1() {} };</div><div><br></div><div>// two warnings about f, one for each constructor</div>

<div>struct F { int f; F() {} F(int) : f() {} F(int, int) {} };</div><div><br></div><div>// no warnings for union members</div><div>struct G { union { int g1, g2; }; };</div><div><br></div><div>// no warnings for a separate constructor</div>

<div>struct H { int h; H(); };</div><div>H::H() : h() {}</div><div><br></div><div>// warning for anonymous struct members (only when used do to laziness)</div><div>struct I { struct { int i; }; };</div><div><div><br></div>

<div>// warning about j (Wuninitialized, not Wuninitialized-member)</div><div>struct J { int j; J() : j(j) {} };</div></div><div><br></div><div>I'll make more test cases and add them to clang's tests soon.</div></div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) Is there any way to silence the warning without introducing<br>
initializations that may not be required/desired? (one interesting<br>
case I came across in Clang recently is, effectively, a discriminated<br>
union - some members are used/unused based on which enum value is<br>
currently held - there's no need to initialize the other members as<br>
they are dead)<br></blockquote><div><br></div><div>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? </div>

<div></div></div><br><div>- Evan</div>