From 4204f07fc8bffe6d320b2de95fea274ccf37a17b Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 2 Aug 2010 21:13:48 +0000 Subject: [PATCH] Further adjustments to -Wglobal-constructors; works for references and direct initializations now. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@110063 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Expr.h | 2 +- lib/AST/Expr.cpp | 46 ++++++++++++++--------- lib/Checker/CheckDeadStores.cpp | 2 +- lib/CodeGen/CGDecl.cpp | 4 +- lib/Sema/SemaDecl.cpp | 11 ++++-- lib/Sema/SemaDeclCXX.cpp | 8 ++++ test/SemaCXX/warn-global-constructors.cpp | 19 ++++++---- 7 files changed, 58 insertions(+), 34 deletions(-) diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 5f894d324e..79e7822b77 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -309,7 +309,7 @@ public: } /// isConstantInitializer - Returns true if this expression is a constant /// initializer, which can be emitted at compile-time. - bool isConstantInitializer(ASTContext &Ctx) const; + bool isConstantInitializer(ASTContext &Ctx, bool ForRef) const; /// EvalResult is a struct with detailed info about an evaluated expression. struct EvalResult { diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index a36a83ba99..7642c0de2c 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -1342,15 +1342,20 @@ bool Expr::hasAnyValueDependentArguments(Expr** Exprs, unsigned NumExprs) { return false; } -bool Expr::isConstantInitializer(ASTContext &Ctx) const { +bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef) const { // This function is attempting whether an expression is an initializer // which can be evaluated at compile-time. isEvaluatable handles most // of the cases, but it can't deal with some initializer-specific // expressions, and it can't deal with aggregates; we deal with those here, // and fall back to isEvaluatable for the other cases. - // FIXME: This function assumes the variable being assigned to - // isn't a reference type! + // If we ever capture reference-binding directly in the AST, we can + // kill the second parameter. + + if (IsForRef) { + EvalResult Result; + return EvaluateAsLValue(Result, Ctx) && !Result.HasSideEffects; + } switch (getStmtClass()) { default: break; @@ -1361,23 +1366,24 @@ bool Expr::isConstantInitializer(ASTContext &Ctx) const { case CXXTemporaryObjectExprClass: case CXXConstructExprClass: { const CXXConstructExpr *CE = cast(this); + + // Only if it's + // 1) an application of the trivial default constructor or if (!CE->getConstructor()->isTrivial()) return false; - for (CXXConstructExpr::const_arg_iterator - I = CE->arg_begin(), E = CE->arg_end(); I != E; ++I) - if (!(*I)->isConstantInitializer(Ctx)) - return false; - return true; - } - case CXXBindReferenceExprClass: { - const CXXBindReferenceExpr *RE = cast(this); - return RE->getSubExpr()->isConstantInitializer(Ctx); + if (!CE->getNumArgs()) return true; + + // 2) an elidable trivial copy construction of an operand which is + // itself a constant initializer. Note that we consider the + // operand on its own, *not* as a reference binding. + return CE->isElidable() && + CE->getArg(0)->isConstantInitializer(Ctx, false); } case CompoundLiteralExprClass: { // This handles gcc's extension that allows global initializers like // "struct x {int x;} x = (struct x) {};". // FIXME: This accepts other cases it shouldn't! const Expr *Exp = cast(this)->getInitializer(); - return Exp->isConstantInitializer(Ctx); + return Exp->isConstantInitializer(Ctx, false); } case InitListExprClass: { // FIXME: This doesn't deal with fields with reference types correctly. @@ -1386,7 +1392,7 @@ bool Expr::isConstantInitializer(ASTContext &Ctx) const { const InitListExpr *Exp = cast(this); unsigned numInits = Exp->getNumInits(); for (unsigned i = 0; i < numInits; i++) { - if (!Exp->getInit(i)->isConstantInitializer(Ctx)) + if (!Exp->getInit(i)->isConstantInitializer(Ctx, false)) return false; } return true; @@ -1394,11 +1400,12 @@ bool Expr::isConstantInitializer(ASTContext &Ctx) const { case ImplicitValueInitExprClass: return true; case ParenExprClass: - return cast(this)->getSubExpr()->isConstantInitializer(Ctx); + return cast(this)->getSubExpr() + ->isConstantInitializer(Ctx, IsForRef); case UnaryOperatorClass: { const UnaryOperator* Exp = cast(this); if (Exp->getOpcode() == UnaryOperator::Extension) - return Exp->getSubExpr()->isConstantInitializer(Ctx); + return Exp->getSubExpr()->isConstantInitializer(Ctx, false); break; } case BinaryOperatorClass: { @@ -1411,6 +1418,7 @@ bool Expr::isConstantInitializer(ASTContext &Ctx) const { return true; break; } + case CXXFunctionalCastExprClass: case CXXStaticCastExprClass: case ImplicitCastExprClass: case CStyleCastExprClass: @@ -1418,13 +1426,15 @@ bool Expr::isConstantInitializer(ASTContext &Ctx) const { // deals with both the gcc no-op struct cast extension and the // cast-to-union extension. if (getType()->isRecordType()) - return cast(this)->getSubExpr()->isConstantInitializer(Ctx); + return cast(this)->getSubExpr() + ->isConstantInitializer(Ctx, false); // Integer->integer casts can be handled here, which is important for // things like (int)(&&x-&&y). Scary but true. if (getType()->isIntegerType() && cast(this)->getSubExpr()->getType()->isIntegerType()) - return cast(this)->getSubExpr()->isConstantInitializer(Ctx); + return cast(this)->getSubExpr() + ->isConstantInitializer(Ctx, false); break; } diff --git a/lib/Checker/CheckDeadStores.cpp b/lib/Checker/CheckDeadStores.cpp index d6ea187957..be1cce96ab 100644 --- a/lib/Checker/CheckDeadStores.cpp +++ b/lib/Checker/CheckDeadStores.cpp @@ -217,7 +217,7 @@ public: // If x is EVER assigned a new value later, don't issue // a warning. This is because such initialization can be // due to defensive programming. - if (E->isConstantInitializer(Ctx)) + if (E->isConstantInitializer(Ctx, false)) return; if (DeclRefExpr *DRE=dyn_cast(E->IgnoreParenCasts())) diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index c4ce008748..0f5a4bdcfc 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -512,10 +512,10 @@ void CodeGenFunction::EmitLocalBlockVarDecl(const VarDecl &D, // If this value is an array or struct, is POD, and if the initializer is // a staticly determinable constant, try to optimize it (unless the NRVO // is already optimizing this). - if (D.getInit() && !isByRef && + if (!NRVO && D.getInit() && !isByRef && (Ty->isArrayType() || Ty->isRecordType()) && Ty->isPODType() && - D.getInit()->isConstantInitializer(getContext()) && !NRVO) { + D.getInit()->isConstantInitializer(getContext(), false)) { // If this variable is marked 'const', emit the value as a global. if (CGM.getCodeGenOpts().MergeAllConstants && Ty.isConstant(getContext())) { diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 8a656bcf16..5252188635 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -3855,7 +3855,7 @@ bool Sema::CheckForConstantInitializer(Expr *Init, QualType DclT) { // "may accept other forms of constant expressions" exception. // (We never end up here for C++, so the constant expression // rules there don't matter.) - if (Init->isConstantInitializer(Context)) + if (Init->isConstantInitializer(Context, false)) return false; Diag(Init->getExprLoc(), diag::err_init_element_not_constant) << Init->getSourceRange(); @@ -4067,8 +4067,11 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) { if (getLangOptions().CPlusPlus) { if (!VDecl->isInvalidDecl() && !VDecl->getDeclContext()->isDependentContext() && - VDecl->hasGlobalStorage() && !Init->isConstantInitializer(Context)) - Diag(VDecl->getLocation(), diag::warn_global_constructor); + VDecl->hasGlobalStorage() && + !Init->isConstantInitializer(Context, + VDecl->getType()->isReferenceType())) + Diag(VDecl->getLocation(), diag::warn_global_constructor) + << Init->getSourceRange(); // Make sure we mark the destructor as used if necessary. QualType InitType = VDecl->getType(); @@ -4281,7 +4284,7 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl, if (getLangOptions().CPlusPlus && !Var->isInvalidDecl() && Var->hasGlobalStorage() && !Var->getDeclContext()->isDependentContext() && - !Var->getInit()->isConstantInitializer(Context)) + !Var->getInit()->isConstantInitializer(Context, false)) Diag(Var->getLocation(), diag::warn_global_constructor); } } diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index a0bc5840cd..5677b5c993 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -5461,6 +5461,14 @@ void Sema::AddCXXDirectInitializerToDecl(DeclPtrTy Dcl, VDecl->setInit(Result.takeAs()); VDecl->setCXXDirectInitializer(true); + if (!VDecl->isInvalidDecl() && + !VDecl->getDeclContext()->isDependentContext() && + VDecl->hasGlobalStorage() && + !VDecl->getInit()->isConstantInitializer(Context, + VDecl->getType()->isReferenceType())) + Diag(VDecl->getLocation(), diag::warn_global_constructor) + << VDecl->getInit()->getSourceRange(); + if (const RecordType *Record = VDecl->getType()->getAs()) FinalizeVarWithDestructor(VDecl, Record); } diff --git a/test/SemaCXX/warn-global-constructors.cpp b/test/SemaCXX/warn-global-constructors.cpp index c407e407bb..a9347ea300 100644 --- a/test/SemaCXX/warn-global-constructors.cpp +++ b/test/SemaCXX/warn-global-constructors.cpp @@ -22,6 +22,11 @@ namespace test1 { A b = A(); A c = { 10 }; A d = { opaque_int() }; // expected-warning {{global constructor}} + A e = A(A()); + A f = A(a); // expected-warning {{global constructor}} + A g(a); // expected-warning {{global constructor}} + A h((A())); // expected-warning {{global constructor}} + A i((A(A()))); // expected-warning {{global constructor}} } namespace test2 { @@ -30,10 +35,9 @@ namespace test2 { A b[10]; // expected-warning {{global constructor}} A c[10][10]; // expected-warning {{global constructor}} - // FIXME: false positives! - A &d = a; // expected-warning {{global constructor}} - A &e = b[5]; // expected-warning {{global constructor}} - A &f = c[5][7]; // expected-warning {{global constructor}} + A &d = a; + A &e = b[5]; + A &f = c[5][7]; } namespace test3 { @@ -42,10 +46,9 @@ namespace test3 { A b[10]; // expected-warning {{global destructor}} A c[10][10]; // expected-warning {{global destructor}} - // FIXME: false positives! - A &d = a; // expected-warning {{global constructor}} - A &e = b[5]; // expected-warning {{global constructor}} - A &f = c[5][7]; // expected-warning {{global constructor}} + A &d = a; + A &e = b[5]; + A &f = c[5][7]; } namespace test4 { -- 2.40.0