From 1bc22c12cb8c2a7b6d2f849033a73c4befceefca Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Fri, 13 Sep 2013 03:20:53 +0000 Subject: [PATCH] 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 --- clang/lib/Sema/SemaDeclCXX.cpp | 127 +++++++++++++++------------ clang/test/SemaCXX/uninitialized.cpp | 38 ++++++++ 2 files changed, 109 insertions(+), 56 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 594f79833594..c45548d3a844 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -2073,6 +2073,7 @@ namespace { : public EvaluatedExprVisitor { Sema &S; ValueDecl *VD; + bool isReferenceType; public: typedef EvaluatedExprVisitor 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(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; + + Expr *Base = ME; + while (isa(Base)) { + ME = cast(Base); + + if (isa(ME->getMemberDecl())) + return; + + if (FieldDecl *FD = dyn_cast(ME->getMemberDecl())) + if (!FD->isAnonymousStructOrUnion()) + FieldME = ME; + + Base = ME->getBase(); + } + + if (VD == FieldME->getMemberDecl() && isa(Base)) { + unsigned diag = VD->getType()->isReferenceType() + ? diag::warn_reference_field_is_uninit + : diag::warn_field_is_uninit; + S.Diag(FieldME->getExprLoc(), diag) << VD; + } } void HandleValue(Expr *E) { E = E->IgnoreParens(); if (MemberExpr *ME = dyn_cast(E)) { - if (isa(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(Base)) { - ME = cast(Base); - - if (isa(ME->getMemberDecl())) - return; - - if (FieldDecl *FD = dyn_cast(ME->getMemberDecl())) - if (!FD->isAnonymousStructOrUnion()) - FieldME = ME; - - Base = ME->getBase(); - } - - if (VD == FieldME->getMemberDecl() && isa(Base)) { - unsigned diag = VD->getType()->isReferenceType() - ? diag::warn_reference_field_is_uninit - : diag::warn_field_is_uninit; - S.Diag(FieldME->getExprLoc(), diag) << VD; - } + 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(E->getArg(0))) + if (ICE->getCastKind() == CK_NoOp) + if (MemberExpr *ME = dyn_cast(ICE->getSubExpr())) + HandleMemberExpr(ME); + + Inherited::VisitCXXConstructExpr(E); + } + void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) { Expr *Callee = E->getCallee(); if (isa(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(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/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 665cfe7e9118..31895c6e02ce 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/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}} + }; +}