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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
