[cfe-commits] r84140 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/Decl.cpp lib/CodeGen/CodeGenModule.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp test/CXX/temp/temp.spec/temp.explicit/p1-emit.cpp

[Daniel Dunbar]

Hi Doug,

On Wed, Oct 14, 2009 at 2:29 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Wed Oct 14 16:29:40 2009
> New Revision: 84140
>
> URL: http://llvm.org/viewvc/llvm-project?rev=84140&view=rev
> Log:
> Give explicit and implicit instantiations of static data members of
> class templates the proper linkage.
>
> Daniel, please look over the CodeGenModule bits.

Comments below.

> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Oct 14 16:29:40 2009
> @@ -541,7 +541,12 @@
>     }
>   }
>
> -  return VD->getStorageClass() == VarDecl::Static;
> +  // Static data may be deferred, but out-of-line static data members
> +  // cannot be.
> +  // FIXME: What if the initializer has side effects?

For C++ we shouldn't be deferring anything that could have side
effects. Can you fix or file this FIXME?

> +  return VD->isInAnonymousNamespace() ||
> +         (VD->getStorageClass() == VarDecl::Static &&
> +          !(VD->isStaticDataMember() && VD->isOutOfLine()));
>  }

> +static CodeGenModule::GVALinkage
> +GetLinkageForVariable(ASTContext &Context, const VarDecl *VD) {
> +  // Everything located semantically within an anonymous namespace is
> +  // always internal.
> +  if (VD->isInAnonymousNamespace())
> +    return CodeGenModule::GVA_Internal;
> +
> +  // Handle linkage for static data members.
> +  if (VD->isStaticDataMember()) {
> +    switch (VD->getTemplateSpecializationKind()) {
> +    case TSK_Undeclared:
> +    case TSK_ExplicitSpecialization:
> +    case TSK_ExplicitInstantiationDefinition:
> +      return CodeGenModule::GVA_StrongExternal;
> +
> +    case TSK_ExplicitInstantiationDeclaration:
> +      assert(false && "Variable should not be instantiated");
> +      // Fall through to treat this like any other instantiation.

I think we are moving towards using llvm_unreachable for these.

> +    case TSK_ImplicitInstantiation:
> +      return CodeGenModule::GVA_TemplateInstantiation;
> +    }
> +  }
> +
> +  // Static variables get internal linkage.
> +  if (VD->getStorageClass() == VarDecl::Static)
> +    return CodeGenModule::GVA_Internal;
> +
> +  return CodeGenModule::GVA_StrongExternal;
> +}
> +
>  void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) {
>   llvm::Constant *Init = 0;
>   QualType ASTTy = D->getType();
> @@ -1021,9 +1057,10 @@
>   GV->setAlignment(getContext().getDeclAlignInBytes(D));
>
>   // Set the llvm linkage type as appropriate.
> +  GVALinkage Linkage = GetLinkageForVariable(getContext(), D);
>   if (D->isInAnonymousNamespace())
>     GV->setLinkage(llvm::Function::InternalLinkage);

This if can die? Never mind I see John got that.

> -  else if (D->getStorageClass() == VarDecl::Static)
> +  else if (Linkage == GVA_Internal)
>     GV->setLinkage(llvm::Function::InternalLinkage);
>   else if (D->hasAttr<DLLImportAttr>())
>     GV->setLinkage(llvm::Function::DLLImportLinkage);
> @@ -1034,7 +1071,9 @@
>       GV->setLinkage(llvm::GlobalVariable::WeakODRLinkage);
>     else
>       GV->setLinkage(llvm::GlobalVariable::WeakAnyLinkage);
> -  } else if (!CompileOpts.NoCommon &&
> +  } else if (Linkage == GVA_TemplateInstantiation)
> +    GV->setLinkage(llvm::GlobalVariable::WeakAnyLinkage);
> +  else if (!CompileOpts.NoCommon &&
>            !D->hasExternalStorage() && !D->getInit() &&
>            !D->getAttr<SectionAttr>()) {
>     GV->setLinkage(llvm::GlobalVariable::CommonLinkage);
>

Otherwise seems fine.

 - Daniel