Further adjustments to -Wglobal-constructors; works for references and direct

initializations now.

llvm-svn: 110063

clang/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 {

clang/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<CXXConstructExpr>(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<CXXBindReferenceExpr>(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<CompoundLiteralExpr>(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<InitListExpr>(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<ParenExpr>(this)->getSubExpr()->isConstantInitializer(Ctx);
+    return cast<ParenExpr>(this)->getSubExpr()
+      ->isConstantInitializer(Ctx, IsForRef);
   case UnaryOperatorClass: {
     const UnaryOperator* Exp = cast<UnaryOperator>(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<CastExpr>(this)->getSubExpr()->isConstantInitializer(Ctx);
+      return cast<CastExpr>(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<CastExpr>(this)->getSubExpr()->getType()->isIntegerType())
-      return cast<CastExpr>(this)->getSubExpr()->isConstantInitializer(Ctx);
+      return cast<CastExpr>(this)->getSubExpr()
+        ->isConstantInitializer(Ctx, false);
       
     break;
   }

clang/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<DeclRefExpr>(E->IgnoreParenCasts()))

clang/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())) {

clang/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);
       }
     }

clang/lib/Sema/SemaDeclCXX.cpp

@@ -5461,6 +5461,14 @@ void Sema::AddCXXDirectInitializerToDecl(DeclPtrTy Dcl,
   VDecl->setInit(Result.takeAs<Expr>());
   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<RecordType>())
     FinalizeVarWithDestructor(VDecl, Record);
 }

clang/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 {