diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index f0925e478b18..8e009e91565a 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/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 &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 DeclsToRemove; // If non-null, add a note to the warning pointing back to the constructor. const CXXConstructorDecl *Constructor; public: typedef EvaluatedExprVisitor Inherited; UninitializedFieldVisitor(Sema &S, - llvm::SmallPtrSetImpl &Decls, - const CXXConstructorDecl *Constructor) - : Inherited(S.Context), S(S), Decls(Decls), - Constructor(Constructor) { } + llvm::SmallPtrSetImpl &Decls) + : Inherited(S.Context), S(S), Decls(Decls) { } void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) { if (isa(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(E->getLHS())) if (FieldDecl *FD = dyn_cast(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 &Decls, - const CXXConstructorDecl *Constructor) { - if (Decls.size() == 0) - return; - - if (!E) - return; - - if (CXXDefaultInitExpr *Default = dyn_cast(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()) { + if (UninitializedFields.empty()) + break; + Expr *InitExpr = FieldInit->getInit(); + if (!InitExpr) + continue; - CheckInitExprContainsUninitializedFields( - SemaRef, InitExpr, UninitializedFields, Constructor); - - if (FieldDecl *Field = FieldInit->getAnyMember()) - UninitializedFields.erase(Field); + if (CXXDefaultInitExpr *Default = + dyn_cast(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/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index fa453cbb2906..b0b4ef672dbd 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/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 {