During cross field uninitialized checking, when processing an assignment,

don't mark the field as initialized until the next initializer instead of
instantly.  Since this checker is AST based, statements are processed in tree
order instead of following code flow.  This can result in different warnings
from just reordering the code.  Also changed to use one checker per constructor
instead of creating a new checker per field.

class T {
  int x, y;

  // Already warns
  T(bool b) : x(!b ? (1 + y) : (y = 5)) {}

  // New warning added here, previously (1 + y) comes after (y = 5) in the AST
  // preventing the warning.
  T(bool b) : x(b ? (y = 5) : (1 + y)) {}

};

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

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index f0925e478b18c7291d7d0fda7b089a97b8764c04..8e009e91565ad6e1d3bf1c948dde6e391c5c4c4e 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2210,15 +2210,16 @@ namespace {
     // List of Decls to generate a warning on.  Also remove Decls that become
     // initialized.
     llvm::SmallPtrSetImpl<ValueDecl*> &Decls;
+    // Vector of decls to be removed from the Decl set prior to visiting the
+    // nodes.  These Decls may have been initialized in the prior initializer.
+    llvm::SmallVector<ValueDecl*, 4> DeclsToRemove;
     // 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,
-                              llvm::SmallPtrSetImpl<ValueDecl*> &Decls,
-                              const CXXConstructorDecl *Constructor)
-      : Inherited(S.Context), S(S), Decls(Decls),
-        Constructor(Constructor) { }
+                              llvm::SmallPtrSetImpl<ValueDecl*> &Decls)
+      : Inherited(S.Context), S(S), Decls(Decls) { }
 
     void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) {
       if (isa<EnumConstantDecl>(ME->getMemberDecl()))
@@ -2307,6 +2308,20 @@ namespace {
       }
     }
 
+    void CheckInitializer(Expr *E, const CXXConstructorDecl *FieldConstructor,
+                          FieldDecl *Field) {
+      // Remove Decls that may have been initialized in the previous
+      // initializer.
+      for (ValueDecl* VD : DeclsToRemove)
+        Decls.erase(VD);
+
+      DeclsToRemove.clear();
+      Constructor = FieldConstructor;
+      Visit(E);
+      if (Field)
+        Decls.erase(Field);
+    }
+
     void VisitMemberExpr(MemberExpr *ME) {
       // All uses of unbounded reference fields will warn.
       HandleMemberExpr(ME, true /*CheckReferenceOnly*/);
@@ -2365,30 +2380,11 @@ namespace {
         if (MemberExpr *ME = dyn_cast<MemberExpr>(E->getLHS()))
           if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
             if (!FD->getType()->isReferenceType())
-              Decls.erase(FD);
+              DeclsToRemove.push_back(FD);
 
       Inherited::VisitBinaryOperator(E);
     }
   };
-  static void CheckInitExprContainsUninitializedFields(
-      Sema &S, Expr *E, llvm::SmallPtrSetImpl<ValueDecl*> &Decls,
-      const CXXConstructorDecl *Constructor) {
-    if (Decls.size() == 0)
-      return;
-
-    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, nullptr).Visit(E);
-    }
-  }
 
   // Diagnose value-uses of fields to initialize themselves, e.g.
   //   foo(foo)
@@ -2421,14 +2417,32 @@ namespace {
       }
     }
 
+    if (UninitializedFields.empty())
+      return;
+
+    UninitializedFieldVisitor UninitializedChecker(SemaRef,
+                                                   UninitializedFields);
+
     for (const auto *FieldInit : Constructor->inits()) {
-      Expr *InitExpr = FieldInit->getInit();
+      if (UninitializedFields.empty())
+        break;
 
-      CheckInitExprContainsUninitializedFields(
-          SemaRef, InitExpr, UninitializedFields, Constructor);
+      Expr *InitExpr = FieldInit->getInit();
+      if (!InitExpr)
+        continue;
 
-      if (FieldDecl *Field = FieldInit->getAnyMember())
-        UninitializedFields.erase(Field);
+      if (CXXDefaultInitExpr *Default =
+              dyn_cast<CXXDefaultInitExpr>(InitExpr)) {
+        InitExpr = Default->getExpr();
+        if (!InitExpr)
+          continue;
+        // In class initializers will point to the constructor.
+        UninitializedChecker.CheckInitializer(InitExpr, Constructor,
+                                              FieldInit->getAnyMember());
+      } else {
+        UninitializedChecker.CheckInitializer(InitExpr, nullptr,
+                                              FieldInit->getAnyMember());
+      }
     }
   }
 } // namespace

diff --git a/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp
index fa453cbb29069e018b5cd5fd7834ed5077fcf297..b0b4ef672dbd28c8b5a1b0964f9f121dca0b90eb 100644 (file)
--- a/test/SemaCXX/uninitialized.cpp
+++ b/test/SemaCXX/uninitialized.cpp
@@ -823,6 +823,20 @@ namespace cross_field_warnings {
     int d = a + b + c;
     R() : a(c = 5), b(c), c(a) {}
   };
+
+  // FIXME: Use the CFG-based analysis to give a sometimes uninitialized
+  // warning on y.
+  struct T {
+    int x;
+    int y;
+    T(bool b)
+        : x(b ? (y = 5) : (1 + y)),  // expected-warning{{field 'y' is uninitialized when used here}}
+          y(y + 1) {}
+    T(int b)
+        : x(!b ? (1 + y) : (y = 5)),  // expected-warning{{field 'y' is uninitialized when used here}}
+          y(y + 1) {}
+  };
+
 }
 
 namespace base_class {