]> granicus.if.org Git - clang/commitdiff
Make a first pass at implementing -Wglobal-constructors. I'm worried that this
authorJohn McCall <rjmccall@apple.com>
Sun, 1 Aug 2010 20:20:59 +0000 (20:20 +0000)
committerJohn McCall <rjmccall@apple.com>
Sun, 1 Aug 2010 20:20:59 +0000 (20:20 +0000)
will end up bizarrely mirroring CGExprConstant, but that might be the hazard of
this feature.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@109984 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
test/SemaCXX/warn-global-constructors.cpp [new file with mode: 0644]

index b8b683cb5ac7562396e1bf405bda7b811bc47d78..6d1eb4ad12b377ea418a935cfd2e4f90a95e9e93 100644 (file)
@@ -52,6 +52,7 @@ def CXXHexFloats : DiagGroup<"c++-hex-floats">;
 def : DiagGroup<"c++0x-compat", [CXXHexFloats]>;
 def : DiagGroup<"effc++">;
 def FourByteMultiChar : DiagGroup<"four-char-constants">;
+def GlobalConstructors : DiagGroup<"global-constructors">;
 def : DiagGroup<"idiomatic-parentheses">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;
index d30f8139d905a1c26a9f6d71169484ebcc9c1904..283dab70dc196e9e090a2c41f7bc4df777b92d2a 100644 (file)
@@ -168,6 +168,13 @@ def warn_access_decl_deprecated : Warning<
   "access declarations are deprecated; use using declarations instead">,
   InGroup<Deprecated>;
 
+def warn_global_constructor : Warning<
+  "declaration requires a global constructor">,
+  InGroup<GlobalConstructors>, DefaultIgnore;
+def warn_global_destructor : Warning<
+  "declaration requires a global destructor">,
+   InGroup<GlobalConstructors>, DefaultIgnore;
+
 def err_invalid_thread : Error<
   "'__thread' is only allowed on variable declarations">;
 def err_thread_non_global : Error<
index fa09b314570b727f0908a84dedda540f9006e78d..6ff21fab0a7c1358958368e5f5e8bf8d9e855bc9 100644 (file)
@@ -3866,6 +3866,57 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init) {
   AddInitializerToDecl(dcl, move(init), /*DirectInit=*/false);
 }
 
+/// Make a reasonable guess at whether the given initializer will
+/// require a global constructor.
+static bool RequiresGlobalConstructor(Sema &S, Expr *Init) {
+  // FIXME: reproducing the logic of CGExprConstant is kindof dumb.
+  // Maybe this should be integrated into the constant-evaluator?
+  // We'd need array and struct value types.
+  //
+  // It's probably okay to still warn in the theoretical cases where
+  // IR gen can eliminate a global constructor based on
+  // initialization order (not that it actually does that
+  // optimization at the moment).
+  if (Init->isEvaluatable(S.Context)) return false;
+
+  Init = Init->IgnoreParenNoopCasts(S.Context);
+
+  // Look through reference-bindings.
+  if (CXXBindReferenceExpr *BE = dyn_cast<CXXBindReferenceExpr>(Init))
+    return RequiresGlobalConstructor(S, BE);
+
+  // A constructor call needs a global constructor if:
+  if (CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(Init)) {
+    //  - the constructor is non-trivial
+    if (!CE->getConstructor()->isTrivial()) return true;
+
+    //  - any of the argument expressions needs a global constructor
+    for (CXXConstructExpr::arg_iterator
+           I = CE->arg_begin(), E = CE->arg_end(); I != E; ++I)
+      if (RequiresGlobalConstructor(S, *I))
+        return true;
+
+    // We don't have to worry about building temporaries with
+    // non-trivial destructors because we should never have walked
+    // through the CXXExprWithTemporaries.
+
+    // So it should be emitted as a constant expression.
+    return false;
+  }
+
+  /// An initializer list requires a global constructor if any of the
+  /// components do.
+  if (InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
+    for (unsigned I = 0, E = ILE->getNumInits(); I != E; ++I)
+      if (RequiresGlobalConstructor(S, ILE->getInit(I)))
+        return true;
+    return false;
+  }
+
+  // Assume everything else does.
+  return true;
+}
+
 /// AddInitializerToDecl - Adds the initializer Init to the
 /// declaration dcl. If DirectInit is true, this is C++ direct
 /// initialization rather than copy initialization.
@@ -4065,6 +4116,11 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) {
   VDecl->setInit(Init);
 
   if (getLangOptions().CPlusPlus) {
+    if (!VDecl->isInvalidDecl() &&
+        !VDecl->getDeclContext()->isDependentContext() &&
+        VDecl->hasGlobalStorage() && RequiresGlobalConstructor(*this, Init))
+      Diag(VDecl->getLocation(), diag::warn_global_constructor);
+
     // Make sure we mark the destructor as used if necessary.
     QualType InitType = VDecl->getType();
     while (const ArrayType *Array = Context.getAsArrayType(InitType))
@@ -4270,8 +4326,15 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl,
                                               MultiExprArg(*this, 0, 0));
       if (Init.isInvalid())
         Var->setInvalidDecl();
-      else if (Init.get())
+      else if (Init.get()) {
         Var->setInit(MaybeCreateCXXExprWithTemporaries(Init.takeAs<Expr>()));
+
+        if (getLangOptions().CPlusPlus && !Var->isInvalidDecl() && 
+            Var->hasGlobalStorage() &&
+            !Var->getDeclContext()->isDependentContext() &&
+            RequiresGlobalConstructor(*this, Var->getInit()))
+          Diag(Var->getLocation(), diag::warn_global_constructor);
+      }
     }
 
     if (!Var->isInvalidDecl() && getLangOptions().CPlusPlus && Record)
index 3e48d5caeee3683a6048515e84666808c373f2cb..d9215e5bbced1fad0f3d7e29fd16e60d33845637 100644 (file)
@@ -5358,6 +5358,9 @@ void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
                           PDiag(diag::err_access_dtor_var)
                             << VD->getDeclName()
                             << VD->getType());
+
+    if (!VD->isInvalidDecl() && VD->hasGlobalStorage())
+      Diag(VD->getLocation(), diag::warn_global_destructor);
   }
 }
 
diff --git a/test/SemaCXX/warn-global-constructors.cpp b/test/SemaCXX/warn-global-constructors.cpp
new file mode 100644 (file)
index 0000000..6d0709c
--- /dev/null
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only -Wglobal-constructors %s -verify
+
+int opaque_int();
+
+namespace test0 {
+  // These should never require global constructors.
+  int a;
+  int b = 20;
+  float c = 5.0f;
+
+  // This global constructor is avoidable based on initialization order.
+  int d = b; // expected-warning {{global constructor}}
+
+  // These global constructors are unavoidable.
+  int e = opaque_int(); // expected-warning {{global constructor}}
+  int f = b; // expected-warning {{global constructor}}
+}
+
+namespace test1 {
+  struct A { int x; };
+  A a;
+  A b = A();
+  A c = { 10 };
+  A d = { opaque_int() }; // expected-warning {{global constructor}}
+}
+
+namespace test2 {
+  struct A { A(); };
+  A a; // expected-warning {{global constructor}}
+  A b[10]; // expected-warning {{global constructor}}
+  A c[10][10]; // expected-warning {{global constructor}}
+}
+
+namespace test3 {
+  struct A { ~A(); };
+  A a; // expected-warning {{global destructor}}
+  A b[10]; // expected-warning {{global destructor}}
+  A c[10][10]; // expected-warning {{global destructor}}
+}