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.

llvm-svn: 190657

1 files changed, 69 insertions, 54 deletions

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 594f7983359..c45548d3a84 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/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,