Use a proper visitor to recursively check for uninitialized use in constructors.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158477 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Sema/SemaDeclCXX.cpp

@@ -23,6 +23,7 @@
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclVisitor.h"
+#include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -2038,73 +2039,95 @@ static void CheckForDanglingReferenceOrPointer(Sema &S, ValueDecl *Member,
     << (unsigned)IsPointer;
 }
 
-/// Checks an initializer expression for use of uninitialized fields, such as
-/// containing the field that is being initialized. Returns true if there is an
-/// uninitialized field was used an updates the SourceLocation parameter; false
-/// otherwise.
-static bool InitExprContainsUninitializedFields(const Stmt *S,
-                                                const ValueDecl *LhsField,
-                                                SourceLocation *L) {
-  assert(isa<FieldDecl>(LhsField) || isa<IndirectFieldDecl>(LhsField));
-
-  if (isa<CallExpr>(S)) {
-    // Do not descend into function calls or constructors, as the use
-    // of an uninitialized field may be valid. One would have to inspect
-    // the contents of the function/ctor to determine if it is safe or not.
-    // i.e. Pass-by-value is never safe, but pass-by-reference and pointers
-    // may be safe, depending on what the function/ctor does.
-    return false;
-  }
-  if (const MemberExpr *ME = dyn_cast<MemberExpr>(S)) {
-    const NamedDecl *RhsField = ME->getMemberDecl();
-
-    if (const VarDecl *VD = dyn_cast<VarDecl>(RhsField)) {
-      // The member expression points to a static data member.
-      assert(VD->isStaticDataMember() && 
-             "Member points to non-static data member!");
-      (void)VD;
-      return false;
-    }
-    
-    if (isa<EnumConstantDecl>(RhsField)) {
-      // The member expression points to an enum.
-      return false;
+namespace {
+  class UninitializedFieldVisitor
+      : public EvaluatedExprVisitor<UninitializedFieldVisitor> {
+    Sema &S;
+    ValueDecl *VD;
+  public:
+    typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
+    UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context),
+                                                        S(S), VD(VD) {
     }
 
-    if (RhsField == LhsField) {
-      // Initializing a field with itself. Throw a warning.
-      // But wait; there are exceptions!
-      // Exception #1:  The field may not belong to this record.
-      // e.g. Foo(const Foo& rhs) : A(rhs.A) {}
-      const Expr *base = ME->getBase();
-      if (base != NULL && !isa<CXXThisExpr>(base->IgnoreParenCasts())) {
-        // Even though the field matches, it does not belong to this record.
-        return false;
+    void HandleExpr(Expr *E) {
+      if (!E) return;
+
+      // Expressions like x(x) sometimes lack the surrounding expressions
+      // but need to be checked anyways.
+      HandleValue(E);
+      Visit(E);
+    }
+
+    void HandleValue(Expr *E) {
+      E = E->IgnoreParens();
+
+      if (MemberExpr* ME = dyn_cast<MemberExpr>(E)) {
+        if (isa<EnumConstantDecl>(ME->getMemberDecl()))
+            return;
+        Expr* Base = E;
+        while (isa<MemberExpr>(Base)) {
+          ME = dyn_cast<MemberExpr>(Base);
+          if (VarDecl *VarD = dyn_cast<VarDecl>(ME->getMemberDecl()))
+            if (VarD->hasGlobalStorage())
+              return;
+          Base = ME->getBase();
+        }
+
+        if (VD == ME->getMemberDecl() && isa<CXXThisExpr>(Base)) {
+          S.Diag(ME->getExprLoc(), diag::warn_field_is_uninit);
+          return;
+        }
+      }
+
+      if (ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E)) {
+        HandleValue(CO->getTrueExpr());
+        HandleValue(CO->getFalseExpr());
+        return;
+      }
+
+      if (BinaryConditionalOperator *BCO =
+              dyn_cast<BinaryConditionalOperator>(E)) {
+        HandleValue(BCO->getCommon());
+        HandleValue(BCO->getFalseExpr());
+        return;
+      }
+
+      if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+        switch (BO->getOpcode()) {
+        default:
+          return;
+        case(BO_PtrMemD):
+        case(BO_PtrMemI):
+          HandleValue(BO->getLHS());
+          return;
+        case(BO_Comma):
+          HandleValue(BO->getRHS());
+          return;
+        }
       }
-      // None of the exceptions triggered; return true to indicate an
-      // uninitialized field was used.
-      *L = ME->getMemberLoc();
-      return true;
     }
-  } else if (isa<UnaryExprOrTypeTraitExpr>(S)) {
-    // sizeof/alignof doesn't reference contents, do not warn.
-    return false;
-  } else if (const UnaryOperator *UOE = dyn_cast<UnaryOperator>(S)) {
-    // address-of doesn't reference contents (the pointer may be dereferenced
-    // in the same expression but it would be rare; and weird).
-    if (UOE->getOpcode() == UO_AddrOf)
-      return false;
-  }
-  for (Stmt::const_child_range it = S->children(); it; ++it) {
-    if (!*it) {
-      // An expression such as 'member(arg ?: "")' may trigger this.
-      continue;
+
+    void VisitImplicitCastExpr(ImplicitCastExpr *E) {
+      if (E->getCastKind() == CK_LValueToRValue)
+        HandleValue(E->getSubExpr());
+
+      Inherited::VisitImplicitCastExpr(E);
     }
-    if (InitExprContainsUninitializedFields(*it, LhsField, L))
-      return true;
+
+    void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
+      Expr *Callee = E->getCallee();
+      if (isa<MemberExpr>(Callee))
+        HandleValue(Callee);
+
+      Inherited::VisitCXXMemberCallExpr(E);
+    }
+  };
+  static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E,
+                                                       ValueDecl *VD) {
+    UninitializedFieldVisitor(S, VD).HandleExpr(E);
   }
-  return false;
-}
+} // namespace
 
 MemInitResult
 Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
@@ -2153,18 +2176,16 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
     }
   }
 
-  for (unsigned i = 0; i < NumArgs; ++i) {
-    SourceLocation L;
-    if (InitExprContainsUninitializedFields(Args[i], Member, &L)) {
-      // FIXME: Return true in the case when other fields are used before being
+  if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, IdLoc)
+        != DiagnosticsEngine::Ignored)
+    for (unsigned i = 0; i < NumArgs; ++i)
+      // FIXME: Warn about the case when other fields are used before being
       // uninitialized. For example, let this field be the i'th field. When
       // initializing the i'th field, throw a warning if any of the >= i'th
       // fields are used, as they are not yet initialized.
       // Right now we are only handling the case where the i'th field uses
       // itself in its initializer.
-      Diag(L, diag::warn_field_is_uninit);
-    }
-  }
+      CheckInitExprContainsUninitializedFields(*this, Args[i], Member);
 
   SourceRange InitRange = Init->getSourceRange();
 

test/SemaCXX/constructor-initializer.cpp

@@ -126,21 +126,24 @@ struct Q {
 
 // A silly class used to demonstrate field-is-uninitialized in constructors with
 // multiple params.
+int IntParam(int i) { return 0; };
 class TwoInOne { public: TwoInOne(TwoInOne a, TwoInOne b) {} };
 class InitializeUsingSelfTest {
   bool A;
   char* B;
   int C;
   TwoInOne D;
-  InitializeUsingSelfTest(int E)
+  int E;
+  InitializeUsingSelfTest(int F)
       : A(A),  // expected-warning {{field is uninitialized when used here}}
         B((((B)))),  // expected-warning {{field is uninitialized when used here}}
         C(A && InitializeUsingSelfTest::C),  // expected-warning {{field is uninitialized when used here}}
         D(D,  // expected-warning {{field is uninitialized when used here}}
-          D) {}  // expected-warning {{field is uninitialized when used here}}
+          D), // expected-warning {{field is uninitialized when used here}}
+        E(IntParam(E)) {} // expected-warning {{field is uninitialized when used here}}
 };
 
-int IntWrapper(int i) { return 0; };
+int IntWrapper(int &i) { return 0; };
 class InitializeUsingSelfExceptions {
   int A;
   int B;

test/SemaCXX/uninitialized.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -std=c++11 -verify %s
 
 int foo(int x);
 int bar(int* x);
@@ -152,9 +152,9 @@ struct S {
 
   S(bool (*)[1]) : x(x) {} // expected-warning {{field is uninitialized when used here}}
   S(bool (*)[2]) : x(x + 1) {} // expected-warning {{field is uninitialized when used here}}
-  S(bool (*)[3]) : x(x + x) {} // expected-warning {{field is uninitialized when used here}}
+  S(bool (*)[3]) : x(x + x) {} // expected-warning 2{{field is uninitialized when used here}}
   S(bool (*)[4]) : x(static_cast<long>(x) + 1) {} // expected-warning {{field is uninitialized when used here}}
-  S(bool (*)[5]) : x(foo(x)) {} // FIXME: This should warn!
+  S(bool (*)[5]) : x(foo(x)) {} // expected-warning {{field is uninitialized when used here}}
 
   // These don't actually require the value of x and so shouldn't warn.
   S(char (*)[1]) : x(sizeof(x)) {} // rdar://8610363
@@ -213,3 +213,83 @@ int test_lambda() {
   auto f1 = [] (int x, int y) { int z; return x + y + z; }; // expected-warning{{variable 'z' is uninitialized when used here}} expected-note {{initialize the variable 'z' to silence this warning}}
   return f1(1, 2);
 }
+
+namespace {
+  struct A {
+    enum { A1 };
+    static int A2() {return 5;}
+    int A3;
+    int A4() { return 5;}
+  };
+
+  struct B {
+    A a;
+  };
+
+  struct C {
+    C() {}
+    C(int x) {}
+    static A a;
+    B b;
+  };
+  A C::a = A();
+
+  // Accessing non-static members will give a warning.
+  struct D {
+    C c;
+    D(char (*)[1]) : c(c.b.a.A1) {}
+    D(char (*)[2]) : c(c.b.a.A2()) {}
+    D(char (*)[3]) : c(c.b.a.A3) {}    // expected-warning {{field is uninitialized when used here}}
+    D(char (*)[4]) : c(c.b.a.A4()) {}  // expected-warning {{field is uninitialized when used here}}
+
+    // c::a is static, so it is already initialized
+    D(char (*)[5]) : c(c.a.A1) {}
+    D(char (*)[6]) : c(c.a.A2()) {}
+    D(char (*)[7]) : c(c.a.A3) {}
+    D(char (*)[8]) : c(c.a.A4()) {}
+  };
+
+  struct E {
+    int a, b, c;
+    E(char (*)[1]) : a(a ? b : c) {}  // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[2]) : a(b ? a : a) {} // expected-warning 2{{field is uninitialized when used here}}
+    E(char (*)[3]) : a(b ? (a) : c) {} // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[4]) : a(b ? c : (a+c)) {} // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[5]) : a(b ? c : b) {}
+
+    E(char (*)[6]) : a(a ?: a) {} // expected-warning 2{{field is uninitialized when used here}}
+    E(char (*)[7]) : a(b ?: a) {} // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[8]) : a(a ?: c) {} // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[9]) : a(b ?: c) {}
+
+    E(char (*)[10]) : a((a, a, b)) {}
+    E(char (*)[11]) : a((c + a, a + 1, b)) {} // expected-warning 2{{field is uninitialized when used here}}
+    E(char (*)[12]) : a((b + c, c, a)) {} // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[13]) : a((a, a, a, a)) {} // expected-warning {{field is uninitialized when used here}}
+    E(char (*)[14]) : a((b, c, c)) {}
+  };
+
+  struct F {
+    int a;
+    F* f;
+    F(int) {}
+    F() {}
+  };
+
+  int F::*ptr = &F::a;
+  F* F::*f_ptr = &F::f;
+  struct G {
+    F f1, f2;
+    F *f3, *f4;
+    G(char (*)[1]) : f1(f1) {} // expected-warning {{field is uninitialized when used here}}
+    G(char (*)[2]) : f2(f1) {}
+    G(char (*)[3]) : f2(F()) {}
+
+    G(char (*)[4]) : f1(f1.*ptr) {} // expected-warning {{field is uninitialized when used here}}
+    G(char (*)[5]) : f2(f1.*ptr) {}
+
+    G(char (*)[6]) : f3(f3) {}  // expected-warning {{field is uninitialized when used here}}
+    G(char (*)[7]) : f3(f3->*f_ptr) {} // expected-warning {{field is uninitialized when used here}}
+    G(char (*)[8]) : f3(new F(f3->*ptr)) {} // expected-warning {{field is uninitialized when used here}}
+  };
+}