Bug fix: do not emit static const local variables with mutable members

as constants.

Refactor and simplify all the separate checks for whether a type can be
emitted as a constant.

llvm-svn: 150793

clang/lib/CodeGen/CGDecl.cpp

@@ -255,14 +255,13 @@ CodeGenFunction::AddInitializerToStaticVarDecl(const VarDecl &D,
     OldGV->eraseFromParent();
   }
 
+  GV->setConstant(CGM.isTypeConstant(D.getType(), true));
   GV->setInitializer(Init);
 
   if (hasNontrivialDestruction(D.getType())) {
     // We have a constant initializer, but a nontrivial destructor. We still
     // need to perform a guarded "initialization" in order to register the
-    // destructor. Since we're running a destructor on this variable, it can't
-    // be a constant even if it's const.
-    GV->setConstant(false);
+    // destructor.
     EmitCXXGuardedInit(D, GV, /*PerformInit*/false);
   }
 
@@ -775,7 +774,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
       // If this value is a POD array or struct with a statically
       // determinable constant initializer, there are optimizations we can do.
       //
-      // TODO: we should constant-evaluate any variable of literal type
+      // TODO: We should constant-evaluate the initializer of any variable,
       // as long as it is initialized by a constant expression. Currently,
       // isConstantInitializer produces wrong answers for structs with
       // reference or bitfield members, and a few other cases, and checking
@@ -789,17 +788,13 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
         // If the variable's a const type, and it's neither an NRVO
         // candidate nor a __block variable and has no mutable members,
         // emit it as a global instead.
-        if (CGM.getCodeGenOpts().MergeAllConstants && Ty.isConstQualified() &&
-            !NRVO && !isByRef && Ty->isLiteralType()) {
-          CXXRecordDecl *RD =
-            Ty->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
-          if (!RD || !RD->hasMutableFields()) {
-            EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
+        if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef &&
+            CGM.isTypeConstant(Ty, true)) {
+          EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
 
-            emission.Address = 0; // signal this condition to later callbacks
-            assert(emission.wasEmittedAsGlobal());
-            return emission;
-          }
+          emission.Address = 0; // signal this condition to later callbacks
+          assert(emission.wasEmittedAsGlobal());
+          return emission;
         }
 
         // Otherwise, tell the initialization code that we're in this case.

clang/lib/CodeGen/CodeGenModule.cpp

@@ -1101,18 +1101,23 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType *FTy,
                                  ExtraAttrs);
 }
 
-static bool DeclIsConstantGlobal(ASTContext &Context, const VarDecl *D,
-                                 bool ConstantInit) {
-  if (!D->getType().isConstant(Context) && !D->getType()->isReferenceType())
+/// isTypeConstant - Determine whether an object of this type can be emitted
+/// as a constant.
+///
+/// If ExcludeCtor is true, the duration when the object's constructor runs
+/// will not be considered. The caller will need to verify that the object is
+/// not written to during its construction.
+bool CodeGenModule::isTypeConstant(QualType Ty, bool ExcludeCtor) {
+  if (!Ty.isConstant(Context) && !Ty->isReferenceType())
     return false;
-  
+
   if (Context.getLangOptions().CPlusPlus) {
-    if (const RecordType *Record 
-          = Context.getBaseElementType(D->getType())->getAs<RecordType>())
-      return ConstantInit && 
-             !cast<CXXRecordDecl>(Record->getDecl())->hasMutableFields();
+    if (const CXXRecordDecl *Record
+          = Context.getBaseElementType(Ty)->getAsCXXRecordDecl())
+      return ExcludeCtor && !Record->hasMutableFields() &&
+             Record->hasTrivialDestructor();
   }
-  
+
   return true;
 }
 
@@ -1169,7 +1174,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName,
   if (D) {
     // FIXME: This code is overly simple and should be merged with other global
     // handling.
-    GV->setConstant(DeclIsConstantGlobal(Context, D, false));
+    GV->setConstant(isTypeConstant(D->getType(), false));
 
     // Set linkage and visibility in case we never see a definition.
     NamedDecl::LinkageInfo LV = D->getLinkageAndVisibility();
@@ -1450,13 +1455,11 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) {
   GV->setInitializer(Init);
 
   // If it is safe to mark the global 'constant', do so now.
-  GV->setConstant(false);
-  if (!NeedsGlobalCtor && !NeedsGlobalDtor &&
-      DeclIsConstantGlobal(Context, D, true))
-    GV->setConstant(true);
+  GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor &&
+                  isTypeConstant(D->getType(), true));
 
   GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
-  
+
   // Set the llvm linkage type as appropriate.
   llvm::GlobalValue::LinkageTypes Linkage = 
     GetLLVMLinkageVarDefinition(D, GV);

clang/lib/CodeGen/CodeGenModule.h

@@ -448,6 +448,8 @@ public:
 
   llvm::MDNode *getTBAAInfo(QualType QTy);
 
+  bool isTypeConstant(QualType QTy, bool ExcludeCtorDtor);
+
   static void DecorateInstruction(llvm::Instruction *Inst,
                                   llvm::MDNode *TBAAInfo);
 

clang/test/CodeGenCXX/static-mutable.cpp

@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -triple=i686-linux-gnu -emit-llvm -o - | FileCheck %s
+
+struct S {
+  mutable int n;
+};
+int f() {
+  // The purpose of this test is to ensure that this variable is a global
+  // not a constant.
+  // CHECK: @_ZZ1fvE1s = internal global {{.*}} { i32 12 }
+  static const S s = { 12 };
+  return ++s.n;
+}