Simplify and refactor the uninitialized field warning.

Change the uninitialized field warnings so that field initializers are checked
inside the constructor.  Previously, in class initializers were checked
separately.  Running one set of checks also simplifies the logic for preventing
duplicate warnings.  Added new checks to warn when an uninitialized field is
used in base class initialization.  Also fixed misspelling of uninitialized
and moved all code for this warning together.

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 7abb74cd0b74098e27fe02ae8c2b0744db7769fb..21aad683eacb5a35538f79d228013c5da09ac44e 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1371,7 +1371,8 @@ 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">;
+  "during field initialization in %select{this|the implicit default}0 "
+  "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 221c53f03f6a37894b59abdfa1e4a285067d0d10..3b1f67c07feb0b75fa9f0ebb5041c93307a910d0 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2110,40 +2110,20 @@ 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.
+    // List of Decls to generate a warning on.  Also remove Decls that become
+    // initialized.
     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,
+    UninitializedFieldVisitor(Sema &S,
                               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();
-      }
-    }
+      : Inherited(S.Context), S(S), Decls(Decls),
+        Constructor(Constructor) { }
 
     void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) {
-      if (!VD)
-        return;
-
-      if (CheckReferenceOnly && !isReferenceType)
-        return;
-
       if (isa<EnumConstantDecl>(ME->getMemberDecl()))
         return;
 
@@ -2170,36 +2150,27 @@ namespace {
 
       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);
+      if (!Decls.count(FoundVD))
         return;
-      }
 
-      if (CheckReferenceOnly)
+      const bool IsReference = FoundVD->getType()->isReferenceType();
+
+      // Prevent double warnings on use of unbounded references.
+      if (IsReference != 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);
+      unsigned diag = IsReference
+          ? diag::warn_reference_field_is_uninit
+          : diag::warn_field_is_uninit;
+      S.Diag(FieldME->getExprLoc(), diag) << FoundVD;
+      if (Constructor)
+        S.Diag(Constructor->getLocation(),
+               diag::note_uninit_in_this_constructor)
+          << (Constructor->isDefaultConstructor() && Constructor->isImplicit());
 
-      }
     }
 
     void HandleValue(Expr *E) {
-      if (!VD)
-        return;
-
       E = E->IgnoreParens();
 
       if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
@@ -2236,6 +2207,7 @@ namespace {
     }
 
     void VisitMemberExpr(MemberExpr *ME) {
+      // All uses of unbounded reference fields will warn.
       HandleMemberExpr(ME, true /*CheckReferenceOnly*/);
 
       Inherited::VisitMemberExpr(ME);
@@ -2272,20 +2244,78 @@ namespace {
       if (E->getOpcode() == BO_Assign)
         if (MemberExpr *ME = dyn_cast<MemberExpr>(E->getLHS()))
           if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
-            Decls.erase(FD);
+            if (!FD->getType()->isReferenceType())
+              Decls.erase(FD);
 
       Inherited::VisitBinaryOperator(E);
     }
   };
   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)
+      Sema &S, Expr *E, llvm::SmallPtrSet<ValueDecl*, 4> &Decls,
+      const CXXConstructorDecl *Constructor) {
+    if (Decls.size() == 0)
       return;
 
-    if (E)
-      UninitializedFieldVisitor(S, VD, Decls, WarnOnSelfReference, Constructor)
-          .Visit(E);
+    if (!E)
+      return;
+
+    if (CXXDefaultInitExpr *Default = dyn_cast<CXXDefaultInitExpr>(E)) {
+      E = Default->getExpr();
+      if (!E)
+        return;
+      // In class initializers will point to the constructor.
+      UninitializedFieldVisitor(S, Decls, Constructor).Visit(E);
+    } else {
+      UninitializedFieldVisitor(S, Decls, 0).Visit(E);
+    }
+  }
+
+  // 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.
+  static void DiagnoseUninitializedFields(
+      Sema &SemaRef, const CXXConstructorDecl *Constructor) {
+
+    if (SemaRef.getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit,
+                                                    Constructor->getLocation())
+        == DiagnosticsEngine::Ignored) {
+      return;
+    }
+
+    if (Constructor->isInvalidDecl())
+      return;
+
+    const CXXRecordDecl *RD = Constructor->getParent();
+
+    // Holds fields that are uninitialized.
+    llvm::SmallPtrSet<ValueDecl*, 4> UninitializedFields;
+
+    // At the beginning, all fields are 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 (CXXConstructorDecl::init_const_iterator FieldInit =
+             Constructor->init_begin(),
+             FieldInitEnd = Constructor->init_end();
+         FieldInit != FieldInitEnd; ++FieldInit) {
+
+      Expr *InitExpr = (*FieldInit)->getInit();
+
+      CheckInitExprContainsUninitializedFields(
+          SemaRef, InitExpr, UninitializedFields, Constructor);
+
+      if (FieldDecl *Field = (*FieldInit)->getAnyMember())
+        UninitializedFields.erase(Field);
+    }
   }
 } // namespace
 
@@ -3779,90 +3809,6 @@ 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.
-static void DiagnoseUnitializedFields(
-    Sema &SemaRef, const CXXConstructorDecl *Constructor) {
-
-  if (SemaRef.getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit,
-                                                  Constructor->getLocation())
-      == DiagnosticsEngine::Ignored) {
-    return;
-  }
-
-  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 *Field = (*FieldInit)->getAnyMember();
-    Expr *InitExpr = (*FieldInit)->getInit();
-
-    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);
-  }
-}
-
 /// ActOnMemInitializers - Handle the member initializers for a constructor.
 void Sema::ActOnMemInitializers(Decl *ConstructorDecl,
                                 SourceLocation ColonLoc,
@@ -3928,7 +3874,7 @@ void Sema::ActOnMemInitializers(Decl *ConstructorDecl,
 
   SetCtorInitializers(Constructor, AnyErrors, MemInits);
 
-  DiagnoseUnitializedFields(*this, Constructor);
+  DiagnoseUninitializedFields(*this, Constructor);
 }
 
 void
@@ -4056,8 +4002,10 @@ void Sema::ActOnDefaultCtorInitializers(Decl *CDtorDecl) {
     return;
 
   if (CXXConstructorDecl *Constructor
-      = dyn_cast<CXXConstructorDecl>(CDtorDecl))
+      = dyn_cast<CXXConstructorDecl>(CDtorDecl)) {
     SetCtorInitializers(Constructor, /*AnyErrors=*/false);
+    DiagnoseUninitializedFields(*this, Constructor);
+  }
 }
 
 bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
@@ -8215,61 +8163,13 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation,
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(Constructor);
   }
+
+  DiagnoseUninitializedFields(*this, Constructor);
 }
 
 void Sema::ActOnFinishDelayedMemberInitializers(Decl *D) {
   // Perform any delayed checks on exception specifications.
   CheckDelayedMemberExceptionSpecs();
-
-  // 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/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp
index 2fb0482cacbc450175224fef3da3fd175fc93856..4991ebee8b311dec17bed15616ec17b303fe9ec8 100644 (file)
--- a/test/SemaCXX/uninitialized.cpp
+++ b/test/SemaCXX/uninitialized.cpp
@@ -487,7 +487,8 @@ namespace references {
   }
 
   struct T {
-    T() : a(b), b(a) {} // FIXME: Warn here.
+    T() // expected-note{{during field initialization in this constructor}}
+     : a(b), b(a) {} // expected-warning{{reference 'b' is not yet bound to a value when used here}}
     int &a, &b;
     int &c = c; // expected-warning{{reference 'c' is not yet bound to a value when used here}}
   };
@@ -552,7 +553,9 @@ namespace record_fields {
     B(char (*)[7]) : a(const_ref(a)) {}
     B(char (*)[8]) : a(pointer(&a)) {}
     B(char (*)[9]) : a(normal(a)) {}  // expected-warning {{uninitialized}}
-
+  };
+  struct C {
+    C() {} // expected-note4{{in this constructor}}
     A a1 = a1;  // expected-warning {{uninitialized}}
     A a2 = a2.get();  // expected-warning {{uninitialized}}
     A a3 = a3.num();
@@ -563,6 +566,29 @@ namespace record_fields {
     A a8 = pointer(&a8);
     A a9 = normal(a9);  // expected-warning {{uninitialized}}
   };
+  struct D {  // expected-note4{{in the implicit default constructor}}
+    A a1 = a1;  // expected-warning {{uninitialized}}
+    A a2 = a2.get();  // expected-warning {{uninitialized}}
+    A a3 = a3.num();
+    A a4 = a4.copy(a4);  // expected-warning {{uninitialized}}
+    A a5 = a5.something(a5);
+    A a6 = ref(a6);
+    A a7 = const_ref(a7);
+    A a8 = pointer(&a8);
+    A a9 = normal(a9);  // expected-warning {{uninitialized}}
+  };
+  D d;
+  struct E {
+    A a1 = a1;
+    A a2 = a2.get();
+    A a3 = a3.num();
+    A a4 = a4.copy(a4);
+    A a5 = a5.something(a5);
+    A a6 = ref(a6);
+    A a7 = const_ref(a7);
+    A a8 = pointer(&a8);
+    A a9 = normal(a9);
+  };
 }
 
 namespace cross_field_warnings {
@@ -576,13 +602,14 @@ namespace cross_field_warnings {
   struct B {
     int a = b;  // expected-warning{{field 'b' is uninitialized when used here}}
     int b;
+    B() {} // expected-note{{during field initialization in this constructor}}
   };
 
   struct C {
     int a;
     int b = a;  // expected-warning{{field 'a' is uninitialized when used here}}
     C(char (*)[1]) : a(5) {}
-    C(char (*)[2]) {}
+    C(char (*)[2]) {} // expected-note{{during field initialization in this constructor}}
   };
 
   struct D {
@@ -666,5 +693,38 @@ namespace cross_field_warnings {
     P() : x(o.get()) { }
   };
 
+  struct Q {
+    int a;
+    int b;
+    int &c;
+    Q() :
+      a(c = 5),  // expected-warning{{reference 'c' is not yet bound to a value when used here}}
+      b(c),  // expected-warning{{reference 'c' is not yet bound to a value when used here}}
+      c(a) {}
+  };
+
+  struct R {
+    int a;
+    int b;
+    int c;
+    int d = a + b + c;
+    R() : a(c = 5), b(c), c(a) {}
+  };
 }
 
+namespace base_class {
+  struct A {
+    A (int) {}
+  };
+
+  struct B : public A {
+    int x;
+    B() : A(x) {}   // expected-warning{{field 'x' is uninitialized when used here}}
+  };
+
+  struct C : public A {
+    int x;
+    int y;
+    C() : A(y = 4), x(y) {}
+  };
+}