Improve -Wuninitialized to take into account field ordering with initializer

lists.  Since the fields are inititalized one at a time, using a field with
lower index to initialize a higher indexed field should not be warned on.

llvm-svn: 218339

4 files changed, 142 insertions, 4 deletions

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index ae79720e383..b3098abbb09 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -8240,6 +8240,8 @@ namespace {
     bool isPODType;
     bool isReferenceType;
 
+    bool isInitList;
+    llvm::SmallVector<unsigned, 4> InitFieldIndex;
   public:
     typedef EvaluatedExprVisitor<SelfReferenceChecker> Inherited;
 
@@ -8248,6 +8250,7 @@ namespace {
       isPODType = false;
       isRecordType = false;
       isReferenceType = false;
+      isInitList = false;
       if (ValueDecl *VD = dyn_cast<ValueDecl>(OrigDecl)) {
         isPODType = VD->getType().isPODType(S.Context);
         isRecordType = VD->getType()->isRecordType();
@@ -8255,6 +8258,78 @@ namespace {
       }
     }
 
+    // For most expressions, just call the visitor.  For initializer lists,
+    // track the index of the field being initialized since fields are
+    // initialized in order allowing use of previously initialized fields.
+    void CheckExpr(Expr *E) {
+      InitListExpr *InitList = dyn_cast<InitListExpr>(E);
+      if (!InitList) {
+        Visit(E);
+        return;
+      }
+
+      // Track and increment the index here.
+      isInitList = true;
+      InitFieldIndex.push_back(0);
+      for (auto Child : InitList->children()) {
+        CheckExpr(cast<Expr>(Child));
+        ++InitFieldIndex.back();
+      }
+      InitFieldIndex.pop_back();
+    }
+
+    // Returns true if MemberExpr is checked and no futher checking is needed.
+    // Returns false if additional checking is required.
+    bool CheckInitListMemberExpr(MemberExpr *E, bool CheckReference) {
+      llvm::SmallVector<FieldDecl*, 4> Fields;
+      Expr *Base = E;
+      bool ReferenceField = false;
+
+      // Get the field memebers used.
+      while (MemberExpr *ME = dyn_cast<MemberExpr>(Base)) {
+        FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
+        if (!FD)
+          return false;
+        Fields.push_back(FD);
+        if (FD->getType()->isReferenceType())
+          ReferenceField = true;
+        Base = ME->getBase()->IgnoreParenImpCasts();
+      }
+
+      // Keep checking only if the base Decl is the same.
+      DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Base);
+      if (!DRE || DRE->getDecl() != OrigDecl)
+        return false;
+
+      // A reference field can be bound to an unininitialized field.
+      if (CheckReference && !ReferenceField)
+        return true;
+
+      // Convert FieldDecls to their index number.
+      llvm::SmallVector<unsigned, 4> UsedFieldIndex;
+      for (auto I = Fields.rbegin(), E = Fields.rend(); I != E; ++I) {
+        UsedFieldIndex.push_back((*I)->getFieldIndex());
+      }
+
+      // See if a warning is needed by checking the first difference in index
+      // numbers.  If field being used has index less than the field being
+      // initialized, then the use is safe.
+      for (auto UsedIter = UsedFieldIndex.begin(),
+                UsedEnd = UsedFieldIndex.end(),
+                OrigIter = InitFieldIndex.begin(),
+                OrigEnd = InitFieldIndex.end();
+           UsedIter != UsedEnd && OrigIter != OrigEnd; ++UsedIter, ++OrigIter) {
+        if (*UsedIter < *OrigIter)
+          return true;
+        if (*UsedIter > *OrigIter)
+          break;
+      }
+
+      // TODO: Add a different warning which will print the field names.
+      HandleDeclRefExpr(DRE);
+      return true;
+    }
+
     // For most expressions, the cast is directly above the DeclRefExpr.
     // For conditional operators, the cast can be outside the conditional
     // operator if both expressions are DeclRefExpr's.
@@ -8290,6 +8365,12 @@ namespace {
       }
 
       if (isa<MemberExpr>(E)) {
+        if (isInitList) {
+          if (CheckInitListMemberExpr(cast<MemberExpr>(E),
+                                      false /*CheckReference*/))
+            return;
+        }
+
         Expr *Base = E->IgnoreParenImpCasts();
         while (MemberExpr *ME = dyn_cast<MemberExpr>(Base)) {
           // Check for static member variables and don't warn on them.
@@ -8323,6 +8404,11 @@ namespace {
     }
 
     void VisitMemberExpr(MemberExpr *E) {
+      if (isInitList) {
+        if (CheckInitListMemberExpr(E, true /*CheckReference*/))
+          return;
+      }
+
       // Don't warn on arrays since they can be treated as pointers.
       if (E->getType()->canDecayToPointerType()) return;
 
@@ -8439,7 +8525,7 @@ namespace {
             if (DRE->getDecl() == OrigDecl)
               return;
 
-    SelfReferenceChecker(S, OrigDecl).Visit(E);
+    SelfReferenceChecker(S, OrigDecl).CheckExpr(E);
   }
 }
 
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 336fea87a50..24a9d670e87 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -602,7 +602,7 @@ struct A { constexpr A(int a, int b) : k(a + b) {} int k; };
 constexpr int fn(const A &a) { return a.k; }
 static_assert(fn(A(4,5)) == 9, "");
 
-struct B { int n; int m; } constexpr b = { 0, b.n }; // expected-warning {{uninitialized}}
+struct B { int n; int m; } constexpr b = { 0, b.n };
 struct C {
   constexpr C(C *this_) : m(42), n(this_->m) {} // ok
   int m, n;
diff --git a/clang/test/SemaCXX/constant-expression-cxx1y.cpp b/clang/test/SemaCXX/constant-expression-cxx1y.cpp
index 521526ddba4..e1929f76377 100644
--- a/clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -719,8 +719,7 @@ namespace deduced_return_type {
 namespace modify_temporary_during_construction {
   struct A { int &&temporary; int x; int y; };
   constexpr int f(int &r) { r *= 9; return r - 12; }
-  // FIXME: The 'uninitialized' warning here is bogus.
-  constexpr A a = { 6, f(a.temporary), a.temporary }; // expected-warning {{uninitialized}} expected-note {{temporary created here}}
+  constexpr A a = { 6, f(a.temporary), a.temporary }; // expected-note {{temporary created here}}
   static_assert(a.x == 42, "");
   static_assert(a.y == 54, "");
   constexpr int k = a.temporary++; // expected-error {{constant expression}} expected-note {{outside the expression that created the temporary}}
diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp
index bb2ecabfc8a..c57ee2fc37e 100644
--- a/clang/test/SemaCXX/uninitialized.cpp
+++ b/clang/test/SemaCXX/uninitialized.cpp
@@ -877,3 +877,56 @@ namespace delegating_constructor {
     int x;
   };
 }
+
+namespace init_list {
+  int num = 5;
+  struct A { int i1, i2; };
+  struct B { A a1, a2; };
+
+  A a1{1,2};
+  A a2{a2.i1 + 2};  // expected-warning{{uninitialized}}
+  A a3 = {a3.i1 + 2};  // expected-warning{{uninitialized}}
+  A a4 = A{a4.i2 + 2};  // expected-warning{{uninitialized}}
+
+  B b1 = { {}, {} };
+  B b2 = { {}, b2.a1 };
+  B b3 = { b3.a1 };  // expected-warning{{uninitialized}}
+  B b4 = { {}, b4.a2} ;  // expected-warning{{uninitialized}}
+  B b5 = { b5.a2 };  // expected-warning{{uninitialized}}
+
+  B b6 = { {b6.a1.i1} };  // expected-warning{{uninitialized}}
+  B b7 = { {0, b7.a1.i1} };
+  B b8 = { {}, {b8.a1.i1} };
+  B b9 = { {}, {0, b9.a1.i1} };
+
+  B b10 = { {b10.a1.i2} };  // expected-warning{{uninitialized}}
+  B b11 = { {0, b11.a1.i2} };  // expected-warning{{uninitialized}}
+  B b12 = { {}, {b12.a1.i2} };
+  B b13 = { {}, {0, b13.a1.i2} };
+
+  B b14 = { {b14.a2.i1} };  // expected-warning{{uninitialized}}
+  B b15 = { {0, b15.a2.i1} };  // expected-warning{{uninitialized}}
+  B b16 = { {}, {b16.a2.i1} };  // expected-warning{{uninitialized}}
+  B b17 = { {}, {0, b17.a2.i1} };
+
+  B b18 = { {b18.a2.i2} };  // expected-warning{{uninitialized}}
+  B b19 = { {0, b19.a2.i2} };  // expected-warning{{uninitialized}}
+  B b20 = { {}, {b20.a2.i2} };  // expected-warning{{uninitialized}}
+  B b21 = { {}, {0, b21.a2.i2} };  // expected-warning{{uninitialized}}
+
+  B b22 = { {b18.a2.i2 + 5} };
+
+  struct C {int a; int& b; int c; };
+  C c1 = { 0, num, 0 };
+  C c2 = { 1, num, c2.b };
+  C c3 = { c3.b, num };  // expected-warning{{uninitialized}}
+  C c4 = { 0, c4.b, 0 };  // expected-warning{{uninitialized}}
+  C c5 = { 0, c5.c, 0 };
+  C c6 = { c6.b, num, 0 };  // expected-warning{{uninitialized}}
+  C c7 = { 0, c7.a, 0 };
+
+  struct D {int &a; int &b; };
+  D d1 = { num, num };
+  D d2 = { num, d2.a };
+  D d3 = { d3.b, num };  // expected-warning{{uninitialized}}
+};