From 44fd70a3ad43b8e7b27af79f8011c7f5b90c841f Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 18 Sep 2012 15:58:06 +0000 Subject: [PATCH] Warn about self references in in-class initializers. This makes Clang warn about self references in in-class initializers, for example: struct S { int a = a + 42; }; This basically just moves UninitializedFieldVisitor up a bit in SemaDeclCXX.cpp, and adds a call to it from ActOnCXXInClassMemberInitializer. llvm-svn: 164131 --- clang/lib/Sema/SemaDeclCXX.cpp | 195 ++++++++++++++------------- clang/test/SemaCXX/uninitialized.cpp | 28 +++- 2 files changed, 128 insertions(+), 95 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 2d7ca95f08f3..a92f2d2340c2 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -1678,6 +1678,99 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D, return Member; } +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) { + } + + void HandleExpr(Expr *E) { + if (!E) return; + + // Expressions like x(x) sometimes lack the surrounding expressions + // but need to be checked anyways. + HandleValue(E); + Visit(E); + } + + void HandleValue(Expr *E) { + E = E->IgnoreParens(); + + 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)) { + unsigned diag = VD->getType()->isReferenceType() + ? diag::warn_reference_field_is_uninit + : diag::warn_field_is_uninit; + S.Diag(ME->getExprLoc(), diag); + return; + } + } + + if (ConditionalOperator *CO = dyn_cast(E)) { + HandleValue(CO->getTrueExpr()); + HandleValue(CO->getFalseExpr()); + return; + } + + if (BinaryConditionalOperator *BCO = + dyn_cast(E)) { + HandleValue(BCO->getCommon()); + HandleValue(BCO->getFalseExpr()); + return; + } + + if (BinaryOperator *BO = dyn_cast(E)) { + switch (BO->getOpcode()) { + default: + return; + case(BO_PtrMemD): + case(BO_PtrMemI): + HandleValue(BO->getLHS()); + return; + case(BO_Comma): + HandleValue(BO->getRHS()); + return; + } + } + } + + void VisitImplicitCastExpr(ImplicitCastExpr *E) { + if (E->getCastKind() == CK_LValueToRValue) + HandleValue(E->getSubExpr()); + + Inherited::VisitImplicitCastExpr(E); + } + + 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); + } +} // namespace + /// ActOnCXXInClassMemberInitializer - This is invoked after parsing an /// in-class initializer for a non-static C++ class member, and after /// instantiating an in-class initializer in a class template. Such actions @@ -1701,6 +1794,11 @@ 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() && !FD->getDeclContext()->isDependentContext()) { @@ -2065,99 +2163,6 @@ static void CheckForDanglingReferenceOrPointer(Sema &S, ValueDecl *Member, << (unsigned)IsPointer; } -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) { - } - - void HandleExpr(Expr *E) { - if (!E) return; - - // Expressions like x(x) sometimes lack the surrounding expressions - // but need to be checked anyways. - HandleValue(E); - Visit(E); - } - - void HandleValue(Expr *E) { - E = E->IgnoreParens(); - - 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)) { - unsigned diag = VD->getType()->isReferenceType() - ? diag::warn_reference_field_is_uninit - : diag::warn_field_is_uninit; - S.Diag(ME->getExprLoc(), diag); - return; - } - } - - if (ConditionalOperator *CO = dyn_cast(E)) { - HandleValue(CO->getTrueExpr()); - HandleValue(CO->getFalseExpr()); - return; - } - - if (BinaryConditionalOperator *BCO = - dyn_cast(E)) { - HandleValue(BCO->getCommon()); - HandleValue(BCO->getFalseExpr()); - return; - } - - if (BinaryOperator *BO = dyn_cast(E)) { - switch (BO->getOpcode()) { - default: - return; - case(BO_PtrMemD): - case(BO_PtrMemI): - HandleValue(BO->getLHS()); - return; - case(BO_Comma): - HandleValue(BO->getRHS()); - return; - } - } - } - - void VisitImplicitCastExpr(ImplicitCastExpr *E) { - if (E->getCastKind() == CK_LValueToRValue) - HandleValue(E->getSubExpr()); - - Inherited::VisitImplicitCastExpr(E); - } - - 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); - } -} // namespace - MemInitResult Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init, SourceLocation IdLoc) { @@ -2191,11 +2196,13 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init, != DiagnosticsEngine::Ignored) 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 + // 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(); diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp index 0633764c1b44..baee272ed581 100644 --- a/clang/test/SemaCXX/uninitialized.cpp +++ b/clang/test/SemaCXX/uninitialized.cpp @@ -379,6 +379,25 @@ namespace statics { } } +namespace in_class_initializers { + struct S { + S() : a(a + 1) {} // expected-warning{{field is uninitialized when used here}} + int a = 42; // Note: because a is in a member initializer list, this initialization is ignored. + }; + + struct T { + T() : b(a + 1) {} // No-warning. + int a = 42; + int b; + }; + + struct U { + U() : a(b + 1), b(a + 1) {} // FIXME: Warn here. + int a = 42; // Note: because a and b are in the member initializer list, these initializers are ignored. + int b = 1; + }; +} + namespace references { int &a = a; // expected-warning{{reference 'a' is not yet bound to a value when used within its own initialization}} @@ -394,6 +413,13 @@ namespace references { struct T { T() : a(b), b(a) {} // FIXME: Warn here. int &a, &b; - int &c = c; // FIXME: Warn here. + int &c = c; // expected-warning{{reference is not yet bound to a value when used here}} + }; + + int x; + struct U { + U() : b(a) {} // No-warning. + int &a = x; + int &b; }; }