From fbb08b5ec01fbdeb6219fbba0f5edfd95c752233 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190657 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaDeclCXX.cpp | 123 ++++++++++++++++++--------------- test/SemaCXX/uninitialized.cpp | 38 ++++++++++ 2 files changed, 107 insertions(+), 54 deletions(-) diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 594f798335..c45548d3a8 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/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; - void HandleValue(Expr *E) { - E = E->IgnoreParens(); + Expr *Base = ME; + while (isa(Base)) { + ME = cast(Base); - if (MemberExpr *ME = dyn_cast(E)) { - if (isa(ME->getMemberDecl())) + 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 (FieldDecl *FD = dyn_cast(ME->getMemberDecl())) + if (!FD->isAnonymousStructOrUnion()) + FieldME = ME; - if (isa(ME->getMemberDecl())) - return; + Base = ME->getBase(); + } - if (FieldDecl *FD = dyn_cast(ME->getMemberDecl())) - if (!FD->isAnonymousStructOrUnion()) - FieldME = ME; + 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; + } + } - Base = ME->getBase(); - } + void HandleValue(Expr *E) { + E = E->IgnoreParens(); - 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; - } + if (MemberExpr *ME = dyn_cast(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(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/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp index 665cfe7e91..31895c6e02 100644 --- 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}} + }; +} -- 2.50.1