Implement __declspec(selectany) under -fms-extensions

[rnk]

Summary

selectany only applies to externally visible global variables. It has
the effect of making the data linkonce. MSDN doesn't seem to require
that all definitions be the same, which means clang shouldn't use
linkonceodr.

[• rafael]

Can they be discarded? If so, please add a comment saying that they
can. If not, you need no use weak_odr.

[rnk]

• Use weak instead of linkonce.

[• rafael]

Should

static int x;
extern __declspec(selectany) int x;

work or be rejected? If rejected, LGTM with it added as a test.

[rnk]

MSVC accepts

static int x7;
extern __declspec(selectany) int x7;

But it also accepts

static int x7;
extern int x7;

That seems like a separate issue. It doesn't seem that important to me to
accept this.

Clang errors out with 'attribute declaration must precede definition',
which doesn't seem useful to have in that test.

OK to commit as is?

[rnk]

Closed by commit rL182266 (authored by @rnk).

cfe/trunk/include/clang/Basic/Attr.td

 def DLLImport : InheritableAttr {
   let Spellings = [Declspec<"dllimport">];
 }
 
 def ForceInline : InheritableAttr {
   let Spellings = [Keyword<"__forceinline">];
 }
 
+def SelectAny : InheritableAttr {
+  let Spellings = [Declspec<"selectany">];
+}
+
 def Win64 : InheritableAttr {
   let Spellings = [Keyword<"__w64">];
 }
 
 def Ptr32 : InheritableAttr {
   let Spellings = [Keyword<"__ptr32">];
 }
 

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def warn_attribute_weak_on_local : Warning<
   "__weak attribute cannot be specified on an automatic variable when ARC "
   "is not enabled">,
   InGroup<IgnoredAttributes>;
 def warn_weak_identifier_undeclared : Warning<
   "weak identifier %0 never declared">;
 def err_attribute_weak_static : Error<
   "weak declaration cannot have internal linkage">;
+def err_attribute_selectany_non_extern_data : Error<
+  "'selectany' can only be applied to data items with external linkage">;
 def warn_attribute_weak_import_invalid_on_definition : Warning<
   "'weak_import' attribute cannot be specified on a definition">,
   InGroup<IgnoredAttributes>;
 def err_attribute_weakref_not_static : Error<
   "weakref declaration must have internal linkage">;
 def err_attribute_weakref_not_global_context : Error<
   "weakref declaration of '%0' must be in a global context">;
 def err_attribute_weakref_without_alias : Error<

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

CodeGenModule::GetLLVMLinkageVarDefinition(const VarDecl *D,
                                            llvm::GlobalVariable *GV) {
   GVALinkage Linkage = getContext().GetGVALinkageForVariable(D);
   if (Linkage == GVA_Internal)
     return llvm::Function::InternalLinkage;
   else if (D->hasAttr<DLLImportAttr>())
     return llvm::Function::DLLImportLinkage;
   else if (D->hasAttr<DLLExportAttr>())
     return llvm::Function::DLLExportLinkage;
-  else if (D->hasAttr<WeakAttr>()) {
+  else if (D->hasAttr<SelectAnyAttr>()) {
+    // selectany symbols are externally visible, so use weak instead of
+    // linkonce.  MSVC optimizes away references to const selectany globals, so
+    // all definitions should be the same and ODR linkage should be used.
+    // http://msdn.microsoft.com/en-us/library/5tkz6s71.aspx
+    return llvm::GlobalVariable::WeakODRLinkage;
+  } else if (D->hasAttr<WeakAttr>()) {
     if (GV->isConstant())
       return llvm::GlobalVariable::WeakODRLinkage;
     else
       return llvm::GlobalVariable::WeakAnyLinkage;
   } else if (Linkage == GVA_TemplateInstantiation ||
              Linkage == GVA_ExplicitTemplateInstantiation)
     return llvm::GlobalVariable::WeakODRLinkage;
   else if (!getLangOpts().CPlusPlus && 

cfe/trunk/lib/Sema/SemaDecl.cpp

if (WeakAttr *Attr = ND.getAttr<WeakAttr>()) {
     }
   }
   if (WeakRefAttr *Attr = ND.getAttr<WeakRefAttr>()) {
     if (ND.isExternallyVisible()) {
       S.Diag(Attr->getLocation(), diag::err_attribute_weakref_not_static);
       ND.dropAttr<WeakRefAttr>();
     }
   }
+
+  // 'selectany' only applies to externally visible varable declarations.
+  // It does not apply to functions.
+  if (SelectAnyAttr *Attr = ND.getAttr<SelectAnyAttr>()) {
+    if (isa<FunctionDecl>(ND) || !ND.isExternallyVisible()) {
+      S.Diag(Attr->getLocation(), diag::err_attribute_selectany_non_extern_data);
+      ND.dropAttr<SelectAnyAttr>();
+    }
+  }
 }
 
 /// Given that we are within the definition of the given function,
 /// will that definition behave like C99's 'inline', where the
 /// definition is discarded except for optimization purposes?
 static bool isFunctionDefinitionDiscarded(Sema &S, FunctionDecl *FD) {
   // Try to avoid calling GetGVALinkageForFunction.
 

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

 static void handleForceInlineAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   if (!checkMicrosoftExt(S, Attr))
     return;
   D->addAttr(::new (S.Context)
              ForceInlineAttr(Attr.getRange(), S.Context,
                              Attr.getAttributeSpellingListIndex()));
 }
 
+static void handleSelectAnyAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (!checkMicrosoftExt(S, Attr))
+    return;
+  // Check linkage after possibly merging declaratinos.  See
+  // checkAttributesAfterMerging().
+  D->addAttr(::new (S.Context)
+             SelectAnyAttr(Attr.getRange(), S.Context,
+                           Attr.getAttributeSpellingListIndex()));
+}
+
 //===----------------------------------------------------------------------===//
 // Top Level Sema Entry Points
 //===----------------------------------------------------------------------===//
 
 static void ProcessNonInheritableDeclAttr(Sema &S, Scope *scope, Decl *D,
                                           const AttributeList &Attr) {
   switch (Attr.getKind()) {
   case AttributeList::AT_CUDADevice:  handleDeviceAttr      (S, D, Attr); break;
static void ProcessInheritableDeclAttr(Sema &S, Scope *scope, Decl *D,
   case AttributeList::AT_Win64:
   case AttributeList::AT_Ptr32:
   case AttributeList::AT_Ptr64:
     handlePortabilityAttr(S, D, Attr);
     break;
   case AttributeList::AT_ForceInline:
     handleForceInlineAttr(S, D, Attr);
     break;
+  case AttributeList::AT_SelectAny:
+    handleSelectAnyAttr(S, D, Attr);
+    break;
 
   // Thread safety attributes:
   case AttributeList::AT_AssertExclusiveLock:
     handleAssertExclusiveLockAttr(S, D, Attr);
     break;
   case AttributeList::AT_AssertSharedLock:
     handleAssertSharedLockAttr(S, D, Attr);
     break;

cfe/trunk/test/CodeGen/ms-declspecs.c

 // RUN: %clang_cc1 -triple i386-pc-win32 %s -emit-llvm -fms-compatibility -o - | FileCheck %s
 
+__declspec(selectany) int x1 = 1;
+const __declspec(selectany) int x2 = 2;
+// CHECK: @x1 = weak_odr global i32 1, align 4
+// CHECK: @x2 = weak_odr constant i32 2, align 4
+
 struct __declspec(align(16)) S {
   char x;
 };
 union { struct S s; } u;
 
 // CHECK: @u = {{.*}}zeroinitializer, align 16
 
 

cfe/trunk/test/SemaCXX/attr-selectany.cpp

+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
+// MSVC produces similar diagnostics.
+
+__declspec(selectany) void foo() { } // expected-error{{'selectany' can only be applied to data items with external linkage}}
+
+__declspec(selectany) int x1 = 1;
+
+const __declspec(selectany) int x2 = 2; // expected-error{{'selectany' can only be applied to data items with external linkage}}
+
+extern const __declspec(selectany) int x3 = 3;
+
+extern const int x4;
+const __declspec(selectany) int x4 = 4;
+
+// MSDN says this is incorrect, but MSVC doesn't diagnose it.
+extern __declspec(selectany) int x5;
+
+static __declspec(selectany) int x6 = 2; // expected-error{{'selectany' can only be applied to data items with external linkage}}
+
+// FIXME: MSVC accepts this and makes x7 externally visible and comdat, but keep
+// it as internal and not weak/linkonce.
+static int x7; // expected-note{{previous definition}}
+extern __declspec(selectany) int x7;  // expected-warning{{attribute declaration must precede definition}}
+
+int asdf() { return x7; }
+
+class X {
+ public:
+  X(int i) { i++; };
+  int i;
+};
+
+__declspec(selectany) X x(1);