Refactor the uninitialized field visitor. Also moved the calls to the visitor

later in the code so that the expressions will have addition processing first.
This catches a few additional cases of uninitialized uses of class fields.

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

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 594f79833594bd207b344d36a5f35968e629afca..c45548d3a8442648445ed5c458c40be7fbdbc734 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2073,6 +2073,7 @@ namespace {
       : public EvaluatedExprVisitor<UninitializedFieldVisitor> {
     Sema &S;
     ValueDecl *VD;
+    bool isReferenceType;
   public:
     typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
     UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context),
@@ -2081,48 +2082,44 @@ namespace {
         this->VD = IFD->getAnonField();
       else
         this->VD = VD;
+      isReferenceType = this->VD->getType()->isReferenceType();
     }
 
-    void HandleExpr(Expr *E) {
-      if (!E) return;
+    void HandleMemberExpr(MemberExpr *ME) {
+      if (isa<EnumConstantDecl>(ME->getMemberDecl()))
+        return;
 
-      // Expressions like x(x) sometimes lack the surrounding expressions
-      // but need to be checked anyways.
-      HandleValue(E);
-      Visit(E);
-    }
+      // FieldME is the inner-most MemberExpr that is not an anonymous struct
+      // or union.
+      MemberExpr *FieldME = ME;
 
-    void HandleValue(Expr *E) {
-      E = E->IgnoreParens();
+      Expr *Base = ME;
+      while (isa<MemberExpr>(Base)) {
+        ME = cast<MemberExpr>(Base);
 
-      if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
-        if (isa<EnumConstantDecl>(ME->getMemberDecl()))
+        if (isa<VarDecl>(ME->getMemberDecl()))
           return;
 
-        // FieldME is the inner-most MemberExpr that is not an anonymous struct
-        // or union.
-        MemberExpr *FieldME = ME;
-
-        Expr *Base = E;
-        while (isa<MemberExpr>(Base)) {
-          ME = cast<MemberExpr>(Base);
+        if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+          if (!FD->isAnonymousStructOrUnion())
+            FieldME = ME;
 
-          if (isa<VarDecl>(ME->getMemberDecl()))
-            return;
+        Base = ME->getBase();
+      }
 
-          if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
-            if (!FD->isAnonymousStructOrUnion())
-              FieldME = ME;
+      if (VD == FieldME->getMemberDecl() && isa<CXXThisExpr>(Base)) {
+        unsigned diag = VD->getType()->isReferenceType()
+            ? diag::warn_reference_field_is_uninit
+            : diag::warn_field_is_uninit;
+        S.Diag(FieldME->getExprLoc(), diag) << VD;
+      }
+    }
 
-          Base = ME->getBase();
-        }
+    void HandleValue(Expr *E) {
+      E = E->IgnoreParens();
 
-        if (VD == FieldME->getMemberDecl() && isa<CXXThisExpr>(Base)) {
-          unsigned diag = VD->getType()->isReferenceType()
-              ? diag::warn_reference_field_is_uninit
-              : diag::warn_field_is_uninit;
-          S.Diag(FieldME->getExprLoc(), diag) << VD;
-        }
+      if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
+        HandleMemberExpr(ME);
         return;
       }
 
@@ -2154,6 +2151,13 @@ namespace {
       }
     }
 
+    void VisitMemberExpr(MemberExpr *ME) {
+      if (isReferenceType)
+        HandleMemberExpr(ME);
+
+      Inherited::VisitMemberExpr(ME);
+    }
+
     void VisitImplicitCastExpr(ImplicitCastExpr *E) {
       if (E->getCastKind() == CK_LValueToRValue)
         HandleValue(E->getSubExpr());
@@ -2161,6 +2165,16 @@ namespace {
       Inherited::VisitImplicitCastExpr(E);
     }
 
+    void VisitCXXConstructExpr(CXXConstructExpr *E) {
+      if (E->getNumArgs() == 1)
+        if (ImplicitCastExpr* ICE = dyn_cast<ImplicitCastExpr>(E->getArg(0)))
+          if (ICE->getCastKind() == CK_NoOp)
+            if (MemberExpr *ME = dyn_cast<MemberExpr>(ICE->getSubExpr()))
+              HandleMemberExpr(ME);
+      
+      Inherited::VisitCXXConstructExpr(E);
+    }
+
     void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
       Expr *Callee = E->getCallee();
       if (isa<MemberExpr>(Callee))
@@ -2171,7 +2185,8 @@ namespace {
   };
   static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E,
                                                        ValueDecl *VD) {
-    UninitializedFieldVisitor(S, VD).HandleExpr(E);
+    if (E)
+      UninitializedFieldVisitor(S, VD).Visit(E);
   }
 } // namespace
 
@@ -2198,11 +2213,6 @@ Sema::ActOnCXXInClassMemberInitializer(Decl *D, SourceLocation InitLoc,
     return;
   }
 
-  if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, InitLoc)
-      != DiagnosticsEngine::Ignored) {
-    CheckInitExprContainsUninitializedFields(*this, InitExpr, FD);
-  }
-
   ExprResult Init = InitExpr;
   if (!FD->getType()->isDependentType() && !InitExpr->isTypeDependent()) {
     InitializedEntity Entity = InitializedEntity::InitializeMember(FD);
@@ -2228,6 +2238,11 @@ 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);
 }
 
@@ -2555,10 +2570,6 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
   if (Member->isInvalidDecl())
     return true;
 
-  // Diagnose value-uses of fields to initialize themselves, e.g.
-  //   foo(foo)
-  // where foo is not also a parameter to the constructor.
-  // TODO: implement -Wuninitialized and fold this into that framework.
   MultiExprArg Args;
   if (ParenListExpr *ParenList = dyn_cast<ParenListExpr>(Init)) {
     Args = MultiExprArg(ParenList->getExprs(), ParenList->getNumExprs());
@@ -2569,19 +2580,6 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
     Args = Init;
   }
 
-  if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, IdLoc)
-        != DiagnosticsEngine::Ignored)
-    for (unsigned i = 0, e = Args.size(); i != e; ++i)
-      // 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.
-      CheckInitExprContainsUninitializedFields(*this, Args[i], Member);
-
   SourceRange InitRange = Init->getSourceRange();
 
   if (Member->getType()->isDependentType() || Init->isTypeDependent()) {
@@ -2621,6 +2619,23 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
     Init = MemberInit.get();
   }
 
+  // Diagnose value-uses of fields to initialize themselves, e.g.
+  //   foo(foo)
+  // where foo is not also a parameter to the constructor.
+  // 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.
+  if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, IdLoc)
+      != DiagnosticsEngine::Ignored) {
+    CheckInitExprContainsUninitializedFields(*this, Init, Member);
+  }
+
   if (DirectMember) {
     return new (Context) CXXCtorInitializer(Context, DirectMember, IdLoc,
                                             InitRange.getBegin(), Init,

diff --git a/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp
index 665cfe7e91182e7b6b25097ec94765ed9e5cd3c2..31895c6e02ce3c1524d59633d2a5193f926c1a11 100644 (file)
--- a/test/SemaCXX/uninitialized.cpp
+++ b/test/SemaCXX/uninitialized.cpp
@@ -523,3 +523,41 @@ namespace lambdas {
     A a2([&] { return a2.x; }); // ok
   }
 }
+
+namespace record_fields {
+  struct A {
+    A() {}
+    A get();
+    static A num();
+    static A copy(A);
+    static A something(A&);
+  };
+
+  A ref(A&);
+  A const_ref(const A&);
+  A pointer(A*);
+  A normal(A);
+
+  struct B {
+    A a;
+    B(char (*)[1]) : a(a) {}  // expected-warning {{uninitialized}}
+    B(char (*)[2]) : a(a.get()) {}  // expected-warning {{uninitialized}}
+    B(char (*)[3]) : a(a.num()) {}
+    B(char (*)[4]) : a(a.copy(a)) {}  // expected-warning {{uninitialized}}
+    B(char (*)[5]) : a(a.something(a)) {}
+    B(char (*)[6]) : a(ref(a)) {}
+    B(char (*)[7]) : a(const_ref(a)) {}
+    B(char (*)[8]) : a(pointer(&a)) {}
+    B(char (*)[9]) : a(normal(a)) {}  // expected-warning {{uninitialized}}
+
+    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}}
+  };
+}