[PATCH] Make -Wuninitialized warn on pointer-to-member and comma operators.

[Enrico Pertoso]

Thanks for your observations.

On Sat, Dec 14, 2013 at 8:00 PM, Arthur O'Dwyer
<arthur.j.odwyer at gmail.com> wrote:
> On Fri, Dec 13, 2013 at 10:08 AM, Enrico Pertoso <epertoso at google.com> wrote:
>> http://llvm-reviews.chandlerc.com/D2387
> [...]
>> +void test_comma () {
>> +  int a;  // expected-note {{initialize the variable 'a' to silence this warning}}
>> +  int b = (a, a ?: 2);  // expected-warning {{variable 'a' is uninitialized when used here}}
>> +  int c = (a, a, b, c);  // expected-warning {{variable 'c' is uninitialized when used within its own initialization}}
>> +  int d;  // expected-note {{initialize the variable 'd' to silence this warning}}
>> +  int e = (foo(d), e, b); // expected-warning {{variable 'd' is uninitialized when used here}}
>> +}
>
> Incidentally, I don't see any test for the simple case of
>
>     int g;
>     int h = (g, 42);
>
> where the uninitialized variable's first use is on the *left-hand*
> side of a comma operator. (And where the uninitialized variable
> doesn't have another unrelated use elsewhere on the same line, of
> course.)

There's no such test because IMHO we shouldn't warn in this case.
When visiting an lvalue-to-rvalue cast, the sub-expression (ignoring
the parens) is classified as `Use`; according to [expr.comma]p1, the
value of the LHS expression is evaluated and discarded, i.e. in our
case only the RHS is actually "used".

If the comma is an overloaded operator, the LHS variable may be used,
but I think cases like this should be handled by the static analyzer.

> Nor
>
>     int j, j2;
>     int k = (j ?: 42);
>     int k2 = (0 ?: j);
>
> where the uninitialized variable's first use is on the LHS or RHS of a
> ?: operator.
> Does this patch propose to warn in those cases (IMHO it should)? Is
> there already a working test for those cases somewhere?
>

The first case is detected in `test_comma`, even if it appears on the
RHS of a comma.
This patch doesn't change the behavior of `ClassifyRefs::classify` on
the true and false expressions of a conditional operator. A more
explicit test would not hurt, though.

>> +namespace mem_ptr {
>> +struct A {
>> +  int x;
>> +  int y;
>> +};
>> +
>> +void foo() {
>> +  int A::* px = &A::x;
>> +  A a{ 1, a.*px }; // expected-warning {{variable 'a' is uninitialized when used within its own initialization}}
>
> Notice that the same initializer should *not* warn if struct A is
> defined as { int x; int& y; } (i.e., with member 'y' as a reference).
> http://llvm.org/bugs/show_bug.cgi?id=14921 is vaguely related.
>

If `y` is not a reference, `ClassifyRefs::classify` will be called
when visiting the lvalue-to-rvalue cast. Otherwise it's not called,
hence no warning will be issued.

> my $.02,
> –Arthur