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

[Enrico Pertoso]

  Yes, it does.

  `DeclRefExpr`s are visited only by `TransferFunctions`, while `ClassifyRefs` relies mostly on binary operators, casts and unary operators to determine whether a variable is used or initialized.

  For statements like `c = (a, a)` we need to classify the `DeclRefExpr` on the LHS of the comma operator as `Ignore`, or `TransferFunctions` will treat is as an initialization.

  In the case of `(foo(a), b)`, the LHS is a `CallExpr` and `ClassifyRefs::classify` has no effect on it, while `a` is the sub-expression of an lvalue-to-rvalue cast, and will be correctly classified as `Use`.

  I added two lines to the test.

Hi rsmith,

http://llvm-reviews.chandlerc.com/D2387

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2387?vs=6033&id=6057#toc

Files:
  lib/Analysis/UninitializedValues.cpp
  test/SemaCXX/uninitialized.cpp

Index: lib/Analysis/UninitializedValues.cpp
===================================================================
--- lib/Analysis/UninitializedValues.cpp
+++ lib/Analysis/UninitializedValues.cpp
@@ -37,7 +37,7 @@
       !vd->isExceptionVariable() &&
       vd->getDeclContext() == dc) {
     QualType ty = vd->getType();
-    return ty->isScalarType() || ty->isVectorType();
+    return ty->isScalarType() || ty->isVectorType() || ty->isRecordType();
   }
   return false;
 }
@@ -359,14 +359,31 @@
 void ClassifyRefs::classify(const Expr *E, Class C) {
   // The result of a ?: could also be an lvalue.
   E = E->IgnoreParens();
-  if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E)) {
-    const Expr *TrueExpr = CO->getTrueExpr();
+  if (const AbstractConditionalOperator *ACO =
+          dyn_cast<AbstractConditionalOperator>(E)) {
+    if (isa<BinaryConditionalOperator>(ACO))
+      classify(cast<BinaryConditionalOperator>(ACO)->getCommon(), Use);
+    const Expr *TrueExpr = ACO->getTrueExpr();
     if (!isa<OpaqueValueExpr>(TrueExpr))
       classify(TrueExpr, C);
-    classify(CO->getFalseExpr(), C);
+    classify(ACO->getFalseExpr(), C);
     return;
   }
 
+  if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+    switch (BO->getOpcode()) {
+    case BO_PtrMemD:
+    case BO_PtrMemI:
+      classify(BO->getLHS(), C);
+      return;
+    case BO_Comma:
+      classify(BO->getRHS(), C);
+      return;
+    default:
+      return;
+    }
+  }
+
   FindVarResult Var = findVar(E, DC);
   if (const DeclRefExpr *DRE = Var.getDeclRefExpr())
     Classification[DRE] = std::max(Classification[DRE], C);
@@ -390,7 +407,7 @@
   // use.
   if (BO->isCompoundAssignmentOp())
     classify(BO->getLHS(), Use);
-  else if (BO->getOpcode() == BO_Assign)
+  else if (BO->getOpcode() == BO_Assign || BO->getOpcode() == BO_Comma)
     classify(BO->getLHS(), Ignore);
 }
 
@@ -401,14 +418,28 @@
     classify(UO->getSubExpr(), Use);
 }
 
+static bool IsPointerToConst(const QualType &QT) {
+  return QT->isAnyPointerType() && QT->getPointeeType().isConstQualified();
+}
+
 void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
-  // If a value is passed by const reference to a function, we should not assume
-  // that it is initialized by the call, and we conservatively do not assume
-  // that it is used.
-  for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
-       I != E; ++I)
-    if ((*I)->getType().isConstQualified() && (*I)->isGLValue())
-      classify(*I, Ignore);
+  // If a value is passed by const pointer or by const reference to a function,
+  // we should not assume that it is initialized by the call, and we
+  // conservatively do not assume that it is used.
+  for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end(); 
+       I != E; ++I) {
+    if ((*I)->isGLValue()) {
+      if ((*I)->getType().isConstQualified())
+        classify((*I), Ignore);
+      continue;
+    } else if (IsPointerToConst((*I)->getType())) {
+      const Expr *Ex = stripCasts(DC->getParentASTContext(), *I);
+      const UnaryOperator *UO = dyn_cast<UnaryOperator>(Ex);
+      if (UO && UO->getOpcode() == UO_AddrOf)
+        Ex = UO->getSubExpr();
+      classify(Ex, Ignore);
+    }
+  }
 }
 
 void ClassifyRefs::VisitCastExpr(CastExpr *CE) {
Index: test/SemaCXX/uninitialized.cpp
===================================================================
--- test/SemaCXX/uninitialized.cpp
+++ test/SemaCXX/uninitialized.cpp
@@ -67,6 +67,27 @@
   }
 }
 
+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}}
+}
+
+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}}
+  A b = b; // expected-warning {{variable 'b' is uninitialized when used within its own initialization}}
+}
+}
+
 // Test self-references with record types.
 class A {
   // Non-POD class.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2387.2.patch
Type: text/x-patch
Size: 4416 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131212/dc5e82b9/attachment.bin>