Modify the uninitialized field visitor to detect uninitialized use across the

fields in the class.  This allows a better checking of member intiailizers and
in class initializers in regards to initialization ordering.

For instance, this code will now produce warnings:

class A {
  int x;
  int y;
  A() : x(y) {}  // y is initialized after x, warn here
  A(int): y(x) {} // default initialization of leaves x uninitialized, warn here
};

Several test cases were updated with -Wno-uninitialized to silence this warning.

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 9a6e9d0ff06c2fd01af972f82cb3cbc5fe490e16..8486ff33c5dc6d902f2279fead18577154d83ec7 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1370,6 +1370,8 @@ def warn_field_is_uninit : Warning<"field %0 is uninitialized when used here">,
 def warn_reference_field_is_uninit : Warning<
   "reference %0 is not yet bound to a value when used here">,
   InGroup<Uninitialized>;
+def note_uninit_in_this_constructor : Note<
+  "during field initialization in this constructor">;
 def warn_static_self_reference_in_init : Warning<
   "static variable %0 is suspiciously used within its own initialization">,
   InGroup<UninitializedStaticSelfInit>;

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index d4f9cc4380a77b309c11a4490b0ac80dc074f4d4..34374aa99eb91231c2b415a8868a852bd6ec1b33 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2081,20 +2081,37 @@ namespace {
   class UninitializedFieldVisitor
       : public EvaluatedExprVisitor<UninitializedFieldVisitor> {
     Sema &S;
+    // If VD is null, this visitor will only update the Decls set.
     ValueDecl *VD;
     bool isReferenceType;
+    // List of Decls to generate a warning on.
+    llvm::SmallPtrSet<ValueDecl*, 4> &Decls;
+    bool WarnOnSelfReference;
+    // If non-null, add a note to the warning pointing back to the constructor.
+    const CXXConstructorDecl *Constructor;
   public:
     typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
-    UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context),
-                                                        S(S) {
-      if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(VD))
-        this->VD = IFD->getAnonField();
-      else
-        this->VD = VD;
-      isReferenceType = this->VD->getType()->isReferenceType();
+    UninitializedFieldVisitor(Sema &S, ValueDecl *VD,
+                              llvm::SmallPtrSet<ValueDecl*, 4> &Decls,
+                              bool WarnOnSelfReference,
+                              const CXXConstructorDecl *Constructor)
+      : Inherited(S.Context), S(S), VD(VD), isReferenceType(false), Decls(Decls),
+        WarnOnSelfReference(WarnOnSelfReference), Constructor(Constructor) {
+      // When VD is null, this visitor is used to detect initialization of other
+      // fields.
+      if (VD) {
+        if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(VD))
+          this->VD = IFD->getAnonField();
+        else
+          this->VD = VD;
+        isReferenceType = this->VD->getType()->isReferenceType();
+      }
     }
 
     void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) {
+      if (!VD)
+        return;
+
       if (CheckReferenceOnly && !isReferenceType)
         return;
 
@@ -2122,15 +2139,38 @@ namespace {
       if (!isa<CXXThisExpr>(Base))
         return;
 
-      if (VD == FieldME->getMemberDecl()) {
+      ValueDecl* FoundVD = FieldME->getMemberDecl();
+
+      if (VD == FoundVD) {
+        if (!WarnOnSelfReference)
+          return;
+
         unsigned diag = isReferenceType
             ? diag::warn_reference_field_is_uninit
             : diag::warn_field_is_uninit;
         S.Diag(FieldME->getExprLoc(), diag) << VD;
+        if (Constructor)
+          S.Diag(Constructor->getLocation(),
+                 diag::note_uninit_in_this_constructor);
+        return;
+      }
+
+      if (CheckReferenceOnly)
+        return;
+
+      if (Decls.count(FoundVD)) {
+        S.Diag(FieldME->getExprLoc(), diag::warn_field_is_uninit) << FoundVD;
+        if (Constructor)
+          S.Diag(Constructor->getLocation(),
+                 diag::note_uninit_in_this_constructor);
+
       }
     }
 
     void HandleValue(Expr *E) {
+      if (!VD)
+        return;
+
       E = E->IgnoreParens();
 
       if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
@@ -2180,7 +2220,7 @@ namespace {
     }
 
     void VisitCXXConstructExpr(CXXConstructExpr *E) {
-      if (E->getNumArgs() == 1)
+      if (E->getConstructor()->isCopyConstructor())
         if (ImplicitCastExpr* ICE = dyn_cast<ImplicitCastExpr>(E->getArg(0)))
           if (ICE->getCastKind() == CK_NoOp)
             if (MemberExpr *ME = dyn_cast<MemberExpr>(ICE->getSubExpr()))
@@ -2196,11 +2236,27 @@ namespace {
 
       Inherited::VisitCXXMemberCallExpr(E);
     }
+
+    void VisitBinaryOperator(BinaryOperator *E) {
+      // If a field assignment is detected, remove the field from the
+      // uninitiailized field set.
+      if (E->getOpcode() == BO_Assign)
+        if (MemberExpr *ME = dyn_cast<MemberExpr>(E->getLHS()))
+          if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+            Decls.erase(FD);
+
+      Inherited::VisitBinaryOperator(E);
+    }
   };
-  static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E,
-                                                       ValueDecl *VD) {
+  static void CheckInitExprContainsUninitializedFields(
+      Sema &S, Expr *E, ValueDecl *VD, llvm::SmallPtrSet<ValueDecl*, 4> &Decls,
+      bool WarnOnSelfReference, const CXXConstructorDecl *Constructor = 0) {
+    if (Decls.size() == 0 && !WarnOnSelfReference)
+      return;
+
     if (E)
-      UninitializedFieldVisitor(S, VD).Visit(E);
+      UninitializedFieldVisitor(S, VD, Decls, WarnOnSelfReference, Constructor)
+          .Visit(E);
   }
 } // namespace
 
@@ -2252,11 +2308,6 @@ Sema::ActOnCXXInClassMemberInitializer(Decl *D, SourceLocation InitLoc,
 
   InitExpr = Init.release();
 
-  if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, InitLoc)
-      != DiagnosticsEngine::Ignored) {
-    CheckInitExprContainsUninitializedFields(*this, InitExpr, FD);
-  }
-
   FD->setInClassInitializer(InitExpr);
 }
 
@@ -3702,15 +3753,9 @@ bool CheckRedundantUnionInit(Sema &S,
 // Diagnose value-uses of fields to initialize themselves, e.g.
 //   foo(foo)
 // where foo is not also a parameter to the constructor.
+// Also diagnose across field uninitialized use such as
+//   x(y), y(x)
 // TODO: implement -Wuninitialized and fold this into that framework.
-// FIXME: Warn about the case when other fields are used before being
-// initialized. 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.
-// Also need to take into account that some fields may be initialized by
-// in-class initializers, see C++11 [class.base.init]p9.
 static void DiagnoseUnitializedFields(
     Sema &SemaRef, const CXXConstructorDecl *Constructor) {
 
@@ -3720,19 +3765,72 @@ static void DiagnoseUnitializedFields(
     return;
   }
 
-  for (CXXConstructorDecl::init_const_iterator
-           FieldInit = Constructor->init_begin(),
+  const CXXRecordDecl *RD = Constructor->getParent();
+
+  // Holds fields that are uninitialized.
+  llvm::SmallPtrSet<ValueDecl*, 4> UninitializedFields;
+
+  for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end();
+       I != E; ++I) {
+    if (FieldDecl *FD = dyn_cast<FieldDecl>(*I)) {
+      UninitializedFields.insert(FD);
+    } else if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(*I)) {
+      UninitializedFields.insert(IFD->getAnonField());
+    }
+  }
+
+  // Fields already checked when processing the in class initializers.
+  llvm::SmallPtrSet<ValueDecl*, 4>
+      InClassUninitializedFields = UninitializedFields;
+
+  for (CXXConstructorDecl::init_const_iterator FieldInit =
+           Constructor->init_begin(),
            FieldInitEnd = Constructor->init_end();
        FieldInit != FieldInitEnd; ++FieldInit) {
 
-    FieldDecl *FD = (*FieldInit)->getAnyMember();
-    if (!FD)
-      continue;
+    FieldDecl *Field = (*FieldInit)->getAnyMember();
     Expr *InitExpr = (*FieldInit)->getInit();
 
-    if (!isa<CXXDefaultInitExpr>(InitExpr)) {
-      CheckInitExprContainsUninitializedFields(SemaRef, InitExpr, FD);
+    if (!Field) {
+      CheckInitExprContainsUninitializedFields(
+          SemaRef, InitExpr, 0, UninitializedFields,
+          false/*WarnOnSelfReference*/);
+      continue;
     }
+
+    if (CXXDefaultInitExpr *Default = dyn_cast<CXXDefaultInitExpr>(InitExpr)) {
+      // This field is initialized with an in-class initailzer.  Remove the
+      // fields already checked to prevent duplicate warnings.
+      llvm::SmallPtrSet<ValueDecl*, 4> DiffSet = UninitializedFields;
+      for (llvm::SmallPtrSet<ValueDecl*, 4>::iterator
+               I = InClassUninitializedFields.begin(),
+               E = InClassUninitializedFields.end();
+           I != E; ++I) {
+        DiffSet.erase(*I);
+      }
+      CheckInitExprContainsUninitializedFields(
+            SemaRef, Default->getExpr(), Field, DiffSet,
+            DiffSet.count(Field), Constructor);
+
+      // Update the unitialized field sets.
+      CheckInitExprContainsUninitializedFields(
+            SemaRef, Default->getExpr(), 0, UninitializedFields,
+            false);
+      CheckInitExprContainsUninitializedFields(
+            SemaRef, Default->getExpr(), 0, InClassUninitializedFields,
+            false);
+    } else {
+      CheckInitExprContainsUninitializedFields(
+          SemaRef, InitExpr, Field, UninitializedFields,
+          UninitializedFields.count(Field));
+      if (Expr* InClassInit = Field->getInClassInitializer()) {
+        CheckInitExprContainsUninitializedFields(
+            SemaRef, InClassInit, 0, InClassUninitializedFields,
+            false);
+      }
+    }
+    UninitializedFields.erase(Field);
+    InClassUninitializedFields.erase(Field);
   }
 }
 
@@ -8057,6 +8155,56 @@ void Sema::ActOnFinishDelayedMemberInitializers(Decl *D) {
   // Check that any explicitly-defaulted methods have exception specifications
   // compatible with their implicit exception specifications.
   CheckDelayedExplicitlyDefaultedMemberExceptionSpecs();
+
+  // Once all the member initializers are processed, perform checks to see if
+  // any unintialized use is happeneing.
+  if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit,
+                                          D->getLocation())
+      == DiagnosticsEngine::Ignored)
+    return;
+
+  CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D);
+  if (!RD) return;
+
+  // Holds fields that are uninitialized.
+  llvm::SmallPtrSet<ValueDecl*, 4> UninitializedFields;
+
+  // In the beginning, every field is uninitialized.
+  for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end();
+       I != E; ++I) {
+    if (FieldDecl *FD = dyn_cast<FieldDecl>(*I)) {
+      UninitializedFields.insert(FD);
+    } else if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(*I)) {
+      UninitializedFields.insert(IFD->getAnonField());
+    }
+  }
+
+  for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end();
+       I != E; ++I) {
+    FieldDecl *FD = dyn_cast<FieldDecl>(*I);
+    if (!FD)
+      if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(*I))
+        FD = IFD->getAnonField();
+
+    if (!FD)
+      continue;
+
+    Expr *InitExpr = FD->getInClassInitializer();
+    if (!InitExpr) {
+      // Uninitialized reference types will give an error.
+      // Record types with an initializer are default initialized.
+      QualType FieldType = FD->getType();
+      if (FieldType->isReferenceType() || FieldType->isRecordType())
+        UninitializedFields.erase(FD);
+      continue;
+    }
+
+    CheckInitExprContainsUninitializedFields(
+        *this, InitExpr, FD, UninitializedFields,
+        UninitializedFields.count(FD)/*WarnOnSelfReference*/);
+
+    UninitializedFields.erase(FD);
+  }
 }
 
 namespace {

diff --git a/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp b/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp
index d1fbe766d527fc8ba430852042b4e89ba0d7df37..22667ac14bb5245a0b5f35ea2156561cebac44c4 100644 (file)
--- a/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp
+++ b/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1y %s -verify
+// RUN: %clang_cc1 -Wno-uninitialized -std=c++1y %s -verify
 
 // expected-no-diagnostics
 

diff --git a/test/Parser/cxx-ambig-init-templ.cpp b/test/Parser/cxx-ambig-init-templ.cpp
index 88ab61c3ab64f1adcc35ccc4388b923a352bf418..ac79f77e152db75f2af25e240e689da85721f84a 100644 (file)
--- a/test/Parser/cxx-ambig-init-templ.cpp
+++ b/test/Parser/cxx-ambig-init-templ.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -Wno-uninitialized -std=c++11 -verify %s
 
 template<int> struct c { c(int) = delete; typedef void val; operator int() const; };
 

diff --git a/test/SemaCXX/constexpr-value-init.cpp b/test/SemaCXX/constexpr-value-init.cpp
index d137bd88db0bbf0ecce83cd60b908a1f96c94db7..e5b7db50eb65f06c5e9937cc5f39ce4b78da9d9b 100644 (file)
--- a/test/SemaCXX/constexpr-value-init.cpp
+++ b/test/SemaCXX/constexpr-value-init.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -std=c++11 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++11 -fsyntax-only -verify
 
 struct A {
   constexpr A() : a(b + 1), b(a + 1) {} // expected-note {{outside its lifetime}}

diff --git a/test/SemaCXX/cxx0x-class.cpp b/test/SemaCXX/cxx0x-class.cpp
index 074591e7063cd225c11994c10e997d697b4a6c20..2b1338f97fd776edc04d001c0d4b64d0e671d8a6 100644 (file)
--- a/test/SemaCXX/cxx0x-class.cpp
+++ b/test/SemaCXX/cxx0x-class.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-error=static-float-init %s 
+// RUN: %clang_cc1 -Wno-uninitialized -fsyntax-only -verify -std=c++11 -Wno-error=static-float-init %s 
 
 int vs = 0;
 

diff --git a/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp
index 31895c6e02ce3c1524d59633d2a5193f926c1a11..2fb0482cacbc450175224fef3da3fd175fc93856 100644 (file)
--- a/test/SemaCXX/uninitialized.cpp
+++ b/test/SemaCXX/uninitialized.cpp
@@ -340,7 +340,10 @@ namespace {
   };
 
   struct E {
-    int a, b, c;
+    int b = 1;
+    int c = 1;
+    int a;  // This field needs to be last to prevent the cross field
+            // uninitialized warning.
     E(char (*)[1]) : a(a ? b : c) {}  // expected-warning {{field 'a' is uninitialized when used here}}
     E(char (*)[2]) : a(b ? a : a) {} // expected-warning 2{{field 'a' is uninitialized when used here}}
     E(char (*)[3]) : a(b ? (a) : c) {} // expected-warning {{field 'a' is uninitialized when used here}}
@@ -459,7 +462,7 @@ namespace in_class_initializers {
   };
 
   struct U {
-    U() : a(b + 1), b(a + 1) {} // FIXME: Warn here.
+    U() : a(b + 1), b(a + 1) {} // expected-warning{{field 'b' is uninitialized when used here}}
     int a = 42; // Note: because a and b are in the member initializer list, these initializers are ignored.
     int b = 1;
   };
@@ -561,3 +564,107 @@ namespace record_fields {
     A a9 = normal(a9);  // expected-warning {{uninitialized}}
   };
 }
+
+namespace cross_field_warnings {
+  struct A {
+    int a, b;
+    A() {}
+    A(char (*)[1]) : b(a) {}  // expected-warning{{field 'a' is uninitialized when used here}}
+    A(char (*)[2]) : a(b) {}  // expected-warning{{field 'b' is uninitialized when used here}}
+  };
+
+  struct B {
+    int a = b;  // expected-warning{{field 'b' is uninitialized when used here}}
+    int b;
+  };
+
+  struct C {
+    int a;
+    int b = a;  // expected-warning{{field 'a' is uninitialized when used here}}
+    C(char (*)[1]) : a(5) {}
+    C(char (*)[2]) {}
+  };
+
+  struct D {
+    int a;
+    int &b;
+    int &c = a;
+    int d = b;
+    D() : b(a) {}
+  };
+
+  struct E {
+    int a;
+    int get();
+    static int num();
+    E() {}
+    E(int) {}
+  };
+
+  struct F {
+    int a;
+    E e;
+    int b;
+    F(char (*)[1]) : a(e.get()) {}  // expected-warning{{field 'e' is uninitialized when used here}}
+    F(char (*)[2]) : a(e.num()) {}
+    F(char (*)[3]) : e(a) {}  // expected-warning{{field 'a' is uninitialized when used here}}
+    F(char (*)[4]) : a(4), e(a) {}
+    F(char (*)[5]) : e(b) {}  // expected-warning{{field 'b' is uninitialized when used here}}
+    F(char (*)[6]) : e(b), b(4) {}  // expected-warning{{field 'b' is uninitialized when used here}}
+  };
+
+  struct G {
+    G(const A&) {};
+  };
+
+  struct H {
+    A a1;
+    G g;
+    A a2;
+    H() : g(a1) {}
+    H(int) : g(a2) {}
+  };
+
+  struct I {
+    I(int*) {}
+  };
+
+  struct J : public I {
+    int *a;
+    int *b;
+    int c;
+    J() : I((a = new int(5))), b(a), c(*a) {}
+  };
+
+  struct K {
+    int a = (b = 5);
+    int b = b + 5;
+  };
+
+  struct L {
+    int a = (b = 5);
+    int b = b + 5;  // expected-warning{{field 'b' is uninitialized when used here}}
+    L() : a(5) {}  // expected-note{{during field initialization in this constructor}}
+  };
+
+  struct M { };
+
+  struct N : public M {
+    int a;
+    int b;
+    N() : b(a) { }  // expected-warning{{field 'a' is uninitialized when used here}}
+  };
+
+  struct O {
+    int x = 42;
+    int get() { return x; }
+  };
+
+  struct P {
+    O o;
+    int x = o.get();
+    P() : x(o.get()) { }
+  };
+
+}
+

diff --git a/test/SemaCXX/warn-unused-private-field.cpp b/test/SemaCXX/warn-unused-private-field.cpp
index 661442db9e9d1394fc20f612fc368d214b6b64d7..932a7dcea1daa927d8b2cf5049ae6a77c6901339 100644 (file)
--- a/test/SemaCXX/warn-unused-private-field.cpp
+++ b/test/SemaCXX/warn-unused-private-field.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wunused-private-field -Wused-but-marked-unused -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-private-field -Wused-but-marked-unused -Wno-uninitialized -verify -std=c++11 %s
 
 class NotFullyDefined {
  public: