[PATCH] Add support for __declspec(thread) under -fms-extensions

[Reid Kleckner]

Thanks!

================
Comment at: lib/Sema/SemaDeclAttr.cpp:3700-3717
@@ -3699,1 +3699,20 @@
 
+static void handleDeclspecThreadAttr(Sema &S, Decl *D,
+                                     const AttributeList &Attr) {
+  VarDecl *VD = cast<VarDecl>(D);
+  if (!S.Context.getTargetInfo().isTLSSupported()) {
+    S.Diag(Attr.getLoc(), diag::err_thread_unsupported);
+    return;
+  }
+  if (VD->getTSCSpec() != TSCS_unspecified) {
+    S.Diag(Attr.getLoc(), diag::err_declspec_thread_on_thread_variable);
+    return;
+  }
+  if (VD->hasLocalStorage()) {
+    S.Diag(Attr.getLoc(), diag::err_thread_non_global) << "__declspec(thread)";
+    return;
+  }
+  VD->addAttr(::new (S.Context) ThreadAttr(
+      Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
+}
+
----------------
Richard Smith wrote:
> Are you allowed to specify `__declspec(thread)` multiple times on the same declaration? This would appear to allow that.
MSVC allows it with a warning, and we accept silently.  The same is true for all of our other __declspec attributes.  If we want to issue a warning, IMO we should do it somewhere common where we can use the same diagnostic reporting.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:3711-3714
@@ +3710,6 @@
+  }
+  if (VD->hasLocalStorage()) {
+    S.Diag(Attr.getLoc(), diag::err_thread_non_global) << "__declspec(thread)";
+    return;
+  }
+  VD->addAttr(::new (S.Context) ThreadAttr(
----------------
Richard Smith wrote:
> Can you move the `hasLocalStorage` check into the common attribute subject checking side of things? That'd be more in line with what we do elsewhere, but might make the diagnostic less consistent with other thread specifiers, so I'm on the fence.
We don't currently have anything like a GlobalVariable subject but maybe we should.  I'd rather not tablegen this for now.  Now that I'm looking at the code tablegen is making for us, I feel like we should try to optimize its code size a bit.

http://reviews.llvm.org/D3551