[cfe-commits] [PATCH] Add support for Microsoft __declspec(selectany)

[Douglas Gregor]

On Oct 11, 2011, at 3:47 PM, John Wiegley wrote:

> This patch authored by Eric Niebler.
> 
> ---
> include/clang/Basic/Attr.td                |    4 ++
> include/clang/Basic/DiagnosticSemaKinds.td |    3 ++
> include/clang/Sema/AttributeList.h         |    1 +
> lib/CodeGen/CodeGenModule.cpp              |    4 ++
> lib/Sema/AttributeList.cpp                 |    1 +
> lib/Sema/SemaDecl.cpp                      |   22 ++++++++++++
> lib/Sema/SemaDeclAttr.cpp                  |   50 ++++++++++++++++++++++++++--
> test/Sema/attr-selectany.c                 |   18 ++++++++++
> test/SemaCXX/attr-selectany.cpp            |   29 ++++++++++++++++
> 9 files changed, 129 insertions(+), 3 deletions(-)
> 
> diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
> index 2a4ba5c..ca1797c 100644
> --- a/include/clang/Basic/Attr.td
> +++ b/include/clang/Basic/Attr.td
> @@ -511,6 +511,10 @@ def Pascal : InheritableAttr {
>   let Spellings = ["pascal", "__pascal"];
> }
> 
> +def SelectAny : InheritableAttr {
> +  let Spellings = ["selectany"];
> +}
> +
> def TransparentUnion : InheritableAttr {
>   let Spellings = ["transparent_union"];
> }
> diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
> index c4e0168..2913397 100644
> --- a/include/clang/Basic/DiagnosticSemaKinds.td
> +++ b/include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1367,6 +1367,9 @@ def err_attribute_section_invalid_for_target : Error<
>   "argument to 'section' attribute is not valid for this target: %0">;
> def err_attribute_section_local_variable : Error<
>   "'section' attribute is not valid on local variables">;
> +def err_attribute_selectany_invalid_for_target : Error<
> +  "'selectany' attribute is only valid on the initialization of global data "
> +  "with external linkage">;
> def err_attribute_aligned_not_power_of_two : Error<
>   "requested alignment is not a power of 2">;
> def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
> diff --git a/include/clang/Sema/AttributeList.h b/include/clang/Sema/AttributeList.h
> index bcacf7a..de62ce9 100644
> --- a/include/clang/Sema/AttributeList.h
> +++ b/include/clang/Sema/AttributeList.h
> @@ -250,6 +250,7 @@ public:
>     AT_reqd_wg_size,
>     AT_scoped_lockable,
>     AT_section,
> +    AT_selectany,
>     AT_sentinel,
>     AT_shared,
>     AT_shared_lock_function,
> diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
> index aab513f..cf5d6a5 100644
> --- a/lib/CodeGen/CodeGenModule.cpp
> +++ b/lib/CodeGen/CodeGenModule.cpp
> @@ -1104,6 +1104,8 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName,
>         GV->setLinkage(llvm::GlobalValue::DLLImportLinkage);
>       else if (D->hasAttr<WeakAttr>() || D->isWeakImported())
>         GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
> +      else if (D->hasAttr<SelectAnyAttr>())
> +        GV->setLinkage(llvm::GlobalValue::LinkOnceODRLinkage);
> 
>       // Set visibility on a declaration only if it's explicit.
>       if (LV.visibilityExplicit())
> @@ -1413,6 +1415,8 @@ CodeGenModule::GetLLVMLinkageVarDefinition(const VarDecl *D,
>       return llvm::GlobalVariable::WeakODRLinkage;
>     else
>       return llvm::GlobalVariable::WeakAnyLinkage;
> +  } else if (D->hasAttr<SelectAnyAttr>()) {
> +    return llvm::GlobalValue::LinkOnceODRLinkage;
>   } else if (Linkage == GVA_TemplateInstantiation ||
>              Linkage == GVA_ExplicitTemplateInstantiation)
>     return llvm::GlobalVariable::WeakODRLinkage;
> diff --git a/lib/Sema/AttributeList.cpp b/lib/Sema/AttributeList.cpp
> index 13a0ede..26be829 100644
> --- a/lib/Sema/AttributeList.cpp
> +++ b/lib/Sema/AttributeList.cpp
> @@ -130,6 +130,7 @@ AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name) {
>     .Case("objc_gc", AT_objc_gc)
>     .Case("regparm", AT_regparm)
>     .Case("section", AT_section)
> +    .Case("selectany", AT_selectany)
>     .Case("stdcall", AT_stdcall)
>     .Case("annotate", AT_annotate)
>     .Case("fastcall", AT_fastcall)
> diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
> index b1e4a4e..d960e68 100644
> --- a/lib/Sema/SemaDecl.cpp
> +++ b/lib/Sema/SemaDecl.cpp
> @@ -4233,6 +4233,21 @@ void Sema::CheckVariableDeclaration(VarDecl *NewVD,
>     Redeclaration = true;
>     MergeVarDecl(NewVD, Previous);
>   }
> +
> +  // Microsoft's __declspec(selectany) attribute can only be used on global
> +  // variables with external linkage. The check for global-ness has already
> +  // been performed in HandleSelectAnyAttr.
> +  // If you move this elsewhere, please update the comment in HandleSelectAnyAttr.
> +  if (NewVD->hasAttr<SelectAnyAttr>() &&
> +      NewVD->getType().isConstQualified() &&
> +      NewVD->getLinkage() != ExternalLinkage) {
> +    if(Previous.empty() ||
> +      Previous.getFoundDecl()->getLinkage() != ExternalLinkage) {
> +      Diag(NewVD->getAttr<SelectAnyAttr>()->getLocation(), diag::err_attribute_selectany_invalid_for_target);
> +      NewVD->setInvalidDecl();
> +      return;
> +    }
> +  }
> }

Does Microsoft allow selectany on variables in anonymous namespace? They have external linkage according to C++, but it's represented as "unique external" linkage in Clang because they end up being local symbols. Example:

namespace {
  extern const __declspec(selectany) int x6=3;
}

> /// \brief Data used with FindOverriddenMethod
> @@ -6078,6 +6093,13 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl,
>                                  diag::err_abstract_type_in_decl,
>                                  AbstractVariableType))
>         Var->setInvalidDecl();
> +
> +      // Use of Microsoft's __declspec(selectany) requires an initializer
> +      // If you move this elsewhere, please update the comment in HandleSelectAnyAttr.
> +      if (Var->hasAttr<SelectAnyAttr>()) {
> +        Diag(Var->getLocation(), diag::err_attribute_selectany_invalid_for_target);
> +        Var->setInvalidDecl();
> +      }
>       return;

I wonder what "requires an initializer" means. If the a definition of class type is default-initialized, is it initialized? Does it matter if the default constructor is trivial? There may be corner cases here to deal with, and it might be that just identifying the declarations is not enough. (Or it may be fine; I don't have VC++ to test against).

This is looking really good. There are a few tests to write (default-initialized w/ trivial and non-trivial default constructors, definition in an anonymous namespace), to check consistency with VC++, but if those check it out is ready to go in.

	- Doug