Index: test/SemaCXX/constructor-initializer.cpp =================================================================== --- test/SemaCXX/constructor-initializer.cpp (revision 157022) +++ test/SemaCXX/constructor-initializer.cpp (working copy) @@ -126,21 +126,24 @@ // A silly class used to demonstrate field-is-uninitialized in constructors with // multiple params. +int IntParam(int i) { return 0; }; class TwoInOne { public: TwoInOne(TwoInOne a, TwoInOne b) {} }; class InitializeUsingSelfTest { bool A; char* B; int C; TwoInOne D; - InitializeUsingSelfTest(int E) + int E; + InitializeUsingSelfTest(int F) : A(A), // expected-warning {{field is uninitialized when used here}} B((((B)))), // expected-warning {{field is uninitialized when used here}} C(A && InitializeUsingSelfTest::C), // expected-warning {{field is uninitialized when used here}} D(D, // expected-warning {{field is uninitialized when used here}} - D) {} // expected-warning {{field is uninitialized when used here}} + D), // expected-warning {{field is uninitialized when used here}} + E(IntParam(E)) {} // expected-warning {{field is uninitialized when used here}} }; -int IntWrapper(int i) { return 0; }; +int IntWrapper(int &i) { return 0; }; class InitializeUsingSelfExceptions { int A; int B; Index: test/SemaCXX/uninitialized.cpp =================================================================== --- test/SemaCXX/uninitialized.cpp (revision 157022) +++ test/SemaCXX/uninitialized.cpp (working copy) @@ -152,9 +152,9 @@ S(bool (*)[1]) : x(x) {} // expected-warning {{field is uninitialized when used here}} S(bool (*)[2]) : x(x + 1) {} // expected-warning {{field is uninitialized when used here}} - S(bool (*)[3]) : x(x + x) {} // expected-warning {{field is uninitialized when used here}} + S(bool (*)[3]) : x(x + x) {} // expected-warning 2{{field is uninitialized when used here}} S(bool (*)[4]) : x(static_cast(x) + 1) {} // expected-warning {{field is uninitialized when used here}} - S(bool (*)[5]) : x(foo(x)) {} // FIXME: This should warn! + S(bool (*)[5]) : x(foo(x)) {} // expected-warning {{field is uninitialized when used here}} // These don't actually require the value of x and so shouldn't warn. S(char (*)[1]) : x(sizeof(x)) {} // rdar://8610363 @@ -213,3 +213,38 @@ auto f1 = [] (int x, int y) { int z; return x + y + z; }; // expected-warning {{C++11 requires lambda with omitted result type to consist of a single return statement}} expected-warning{{variable 'z' is uninitialized when used here}} expected-note {{initialize the variable 'z' to silence this warning}} return f1(1, 2); } + +namespace { + struct A { + enum { A1 }; + static int A2() {return 5;} + int A3; + int A4() { return 5;} + }; + + struct B { + A a; + }; + + struct C { + C() {} + C(int x) {} + static A a; + B b; + }; + A C::a = A(); + + struct D { + C c; + D(char (*)[1]) : c(c.b.a.A1) {} + D(char (*)[2]) : c(c.b.a.A2()) {} + D(char (*)[3]) : c(c.b.a.A3) {} // expected-warning {{field is uninitialized when used here}} + D(char (*)[4]) : c(c.b.a.A4()) {} // expected-warning {{field is uninitialized when used here}} + + // c::a is static, so it is already initialized + D(char (*)[5]) : c(c.a.A1) {} + D(char (*)[6]) : c(c.a.A2()) {} + D(char (*)[7]) : c(c.a.A3) {} + D(char (*)[8]) : c(c.a.A4()) {} + }; +} Index: lib/Sema/SemaDeclCXX.cpp =================================================================== --- lib/Sema/SemaDeclCXX.cpp (revision 157022) +++ lib/Sema/SemaDeclCXX.cpp (working copy) @@ -23,6 +23,7 @@ #include "clang/AST/CharUnits.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/DeclVisitor.h" +#include "clang/AST/EvaluatedExprVisitor.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecordLayout.h" #include "clang/AST/RecursiveASTVisitor.h" @@ -2009,73 +2010,73 @@ << (unsigned)IsPointer; } -/// Checks an initializer expression for use of uninitialized fields, such as -/// containing the field that is being initialized. Returns true if there is an -/// uninitialized field was used an updates the SourceLocation parameter; false -/// otherwise. -static bool InitExprContainsUninitializedFields(const Stmt *S, - const ValueDecl *LhsField, - SourceLocation *L) { - assert(isa(LhsField) || isa(LhsField)); +namespace { + class UninitializedFieldVisitor + : public EvaluatedExprVisitor { + Sema &S; + ValueDecl *VD; + public: + typedef EvaluatedExprVisitor Inherited; + UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context), + S(S), VD(VD) { + } - if (isa(S)) { - // Do not descend into function calls or constructors, as the use - // of an uninitialized field may be valid. One would have to inspect - // the contents of the function/ctor to determine if it is safe or not. - // i.e. Pass-by-value is never safe, but pass-by-reference and pointers - // may be safe, depending on what the function/ctor does. - return false; - } - if (const MemberExpr *ME = dyn_cast(S)) { - const NamedDecl *RhsField = ME->getMemberDecl(); + void HandleExpr(Expr *E) { + if (!E) return; - if (const VarDecl *VD = dyn_cast(RhsField)) { - // The member expression points to a static data member. - assert(VD->isStaticDataMember() && - "Member points to non-static data member!"); - (void)VD; - return false; + // Expressions like x(x) sometimes lack the surrounding expressions + // but need to be checked anyways. + HandleValue(E); + Visit(E); } - - if (isa(RhsField)) { - // The member expression points to an enum. - return false; - } - if (RhsField == LhsField) { - // Initializing a field with itself. Throw a warning. - // But wait; there are exceptions! - // Exception #1: The field may not belong to this record. - // e.g. Foo(const Foo& rhs) : A(rhs.A) {} - const Expr *base = ME->getBase(); - if (base != NULL && !isa(base->IgnoreParenCasts())) { - // Even though the field matches, it does not belong to this record. - return false; + void HandleValue(Expr *E) { + E = E->IgnoreParenImpCasts(); + + if (MemberExpr* ME = dyn_cast(E)) { + if (isa(ME->getMemberDecl())) + return; + Expr* Base = E; + while (isa(Base)) { + ME = dyn_cast(Base); + if (VarDecl *VarD = dyn_cast(ME->getMemberDecl())) + if (VarD->hasGlobalStorage()) + return; + Base = ME->getBase(); + } + + if (VD == ME->getMemberDecl() && isa(Base)) { + S.Diag(ME->getExprLoc(), diag::warn_field_is_uninit); + return; + } } - // None of the exceptions triggered; return true to indicate an - // uninitialized field was used. - *L = ME->getMemberLoc(); - return true; + + if (ConditionalOperator *CO = dyn_cast(E)) { + HandleValue(CO->getTrueExpr()); + HandleValue(CO->getFalseExpr()); + } } - } else if (isa(S)) { - // sizeof/alignof doesn't reference contents, do not warn. - return false; - } else if (const UnaryOperator *UOE = dyn_cast(S)) { - // address-of doesn't reference contents (the pointer may be dereferenced - // in the same expression but it would be rare; and weird). - if (UOE->getOpcode() == UO_AddrOf) - return false; - } - for (Stmt::const_child_range it = S->children(); it; ++it) { - if (!*it) { - // An expression such as 'member(arg ?: "")' may trigger this. - continue; + + void VisitImplicitCastExpr(ImplicitCastExpr *E) { + if (E->getCastKind() == CK_LValueToRValue) + HandleValue(E->getSubExpr()); + + Inherited::VisitImplicitCastExpr(E); } - if (InitExprContainsUninitializedFields(*it, LhsField, L)) - return true; + + void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) { + Expr *Callee = E->getCallee(); + if (isa(Callee)) + HandleValue(Callee); + + Inherited::VisitCXXMemberCallExpr(E); + } + }; + static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E, + ValueDecl *VD) { + UninitializedFieldVisitor(S, VD).HandleExpr(E); } - return false; -} +} // namespace MemInitResult Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init, @@ -2105,18 +2106,14 @@ Args = InitList->getInits(); NumArgs = InitList->getNumInits(); } - for (unsigned i = 0; i < NumArgs; ++i) { - SourceLocation L; - if (InitExprContainsUninitializedFields(Args[i], Member, &L)) { - // FIXME: Return true in the case when other fields are used before being - // uninitialized. 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. - Diag(L, diag::warn_field_is_uninit); - } - } + for (unsigned i = 0; i < NumArgs; ++i) + // FIXME: Warn about the case when other fields are used before being + // uninitialized. 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. + CheckInitExprContainsUninitializedFields(*this, Args[i], Member); SourceRange InitRange = Init->getSourceRange();