Add support for __declspec(thread) under -fms-extensions

[rnk]

Summary

I had to steal a bit from ScopeDepthOrObjCQuals. This prevents us from
parsing more than 63 nested function prototypes in a parameter context.
We already can't parse more than 127, which is an arbitrary limit
(PR19607). If we care about supporting this, we should add an extra
table to ASTContext for looking up large scope depth values like we do
for parameter indices greater than 254.

[rsmith]

The C++ standard recommends we support a nesting depth of at least 256 for function declarators, but... it seems unlikely anyone's going to get close to that, and as you say, we can deal with that if it ever happens.

include/clang/Basic/DiagnosticSemaKinds.td

2084–2086 ↗

(On Diff #8939)

Please give this a name involving 'declspec'. It's not as general as its name implies.

include/clang/Sema/DeclSpec.h

235 ↗

(On Diff #8939)

This looks misleading, since we're never going to see this within a DeclSpec.

lib/Sema/DeclSpec.cpp

386 ↗

(On Diff #8939)

This seems like it should be unreachable.

[rnk]

include/clang/Basic/DiagnosticSemaKinds.td

2084–2086 ↗

(On Diff #8939)

Done.

lib/Sema/DeclSpec.cpp

386 ↗

(On Diff #8939)

It's used for diagnostics now, and it uses DeclSpec::TSCS___declspec_thread.

Would you be in favor of a patch that removes the TSCS_* static const ints in DeclSpec and replaces them with uses of the Basic/Specifiers.h enumerators?

[rnk]

• Add docs and more semantic checks

[rnk]

• Add more tests

[rsmith]

lib/Sema/DeclSpec.cpp

386 ↗

(On Diff #8939)

Where is it used for diagnostics? It seems strange for someone to pass it a TSCS that didn't come from a DeclSpec, and a DeclSpec cannot have TSCS___declspec_thread.

I don't think I'd be in favor of the patch you suggested -- the DeclSpec type and the Basic type notionally mean different things (even though they have the same underlying enum), and we're just punning between them here. In particular, it does not make sense for the DeclSpec type to have a value for __declspec(thread), since a parsed decl-specifier-seq can't have such a thread storage class specifier.

Giving DeclSpec a separate enum that doesn't have a __declspec(thread) enumerator would seem more consistent with the intent here.

[rsmith]

include/clang/Basic/Attr.td

1671 ↗

(On Diff #8945)

ITYM ThreadDocs?

include/clang/Basic/AttrDocs.td

58 ↗

(On Diff #8945)

Typo 'compatiblity'

lib/Sema/SemaDecl.cpp

8928–8934 ↗

(On Diff #8945)

This seems really weird. We're already checking for non-constant initialization; do you have any idea what this check is for? You can check for 'any constructor' using CXXRecordDecl::hasUserDeclaredConstructor.

[rnk]

• Remove TSCS___declspec_thread in favor of hasAttr

[rnk]

lib/Sema/SemaDecl.cpp

8928–8934 ↗

(On Diff #8945)

Yeah, this was broken. I fixed the TLS_Static check below in r207675, and it works for __declspec(thread), so I nuked this code.

[rsmith]

LGTM

lib/Sema/SemaDeclAttr.cpp

3700–3717 ↗

(On Diff #8988)

Are you allowed to specify __declspec(thread) multiple times on the same declaration? This would appear to allow that.

3711–3714 ↗

(On Diff #8988)

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.

[rnk]

Thanks!

lib/Sema/SemaDeclAttr.cpp

3700–3717 ↗

(On Diff #8988)

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.

3711–3714 ↗

(On Diff #8988)

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.

[rnk]

Closed by commit rL207734 (authored by @rnk).

cfe/trunk/include/clang/AST/Decl.h

public:
   /// computed linkage of symbol, see getLinkage.
   StorageClass getStorageClass() const {
     return (StorageClass) VarDeclBits.SClass;
   }
   void setStorageClass(StorageClass SC);
 
   void setTSCSpec(ThreadStorageClassSpecifier TSC) {
     VarDeclBits.TSCSpec = TSC;
+    assert(VarDeclBits.TSCSpec == TSC && "truncation");
   }
   ThreadStorageClassSpecifier getTSCSpec() const {
     return static_cast<ThreadStorageClassSpecifier>(VarDeclBits.TSCSpec);
   }
-  TLSKind getTLSKind() const {
-    switch (VarDeclBits.TSCSpec) {
-    case TSCS_unspecified:
-      return TLS_None;
-    case TSCS___thread: // Fall through.
-    case TSCS__Thread_local:
-      return TLS_Static;
-    case TSCS_thread_local:
-      return TLS_Dynamic;
-    }
-    llvm_unreachable("Unknown thread storage class specifier!");
-  }
+  TLSKind getTLSKind() const;
 
   /// hasLocalStorage - Returns true if a variable with function scope
   ///  is a non-static local variable.
   bool hasLocalStorage() const {
     if (getStorageClass() == SC_None)
       // Second check is for C++11 [dcl.stc]p4.
       return !isFileVarDecl() && getTSCSpec() == TSCS_unspecified;
 

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

 }
 
 def SelectAny : InheritableAttr {
   let Spellings = [Declspec<"selectany">];
   let LangOpts = [MicrosoftExt];
   let Documentation = [Undocumented];
 }
 
+def Thread : Attr {
+  let Spellings = [Declspec<"thread">];
+  let LangOpts = [MicrosoftExt];
+  let Documentation = [ThreadDocs];
+  let Subjects = SubjectList<[Var]>;
+}
+
 def Win64 : IgnoredAttr {
   let Spellings = [Keyword<"__w64">];
   let LangOpts = [MicrosoftExt];
 }
 
 def Ptr32 : TypeAttr {
   let Spellings = [Keyword<"__ptr32">];
   let Documentation = [Undocumented];

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

 * local-dynamic
 * initial-exec
 * local-exec
 
 TLS models are mutually exclusive.
   }];
 }
 
+def ThreadDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+The ``__declspec(thread)`` attribute declares a variable with thread local
+storage.  It is available under the ``-fms-extensions`` flag for MSVC
+compatibility.  Documentation for the Visual C++ attribute is available on MSDN_.
+
+.. _MSDN: http://msdn.microsoft.com/en-us/library/9w1sdazb.aspx
+
+In Clang, ``__declspec(thread)`` is generally equivalent in functionality to the
+GNU ``__thread`` keyword.  The variable must not have a destructor and must have
+a constant initializer, if any.  The attribute only applies to variables
+declared with static storage duration, such as globals, class static data
+members, and static locals.
+  }];
+}
+
 def CarriesDependencyDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 The ``carries_dependency`` attribute specifies dependency propagation into and
 out of functions.
 
 When specified on a function or Objective-C method, the ``carries_dependency``
 attribute means that the return value carries a dependency out of the function, 

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

def warn_attribute_weak_on_local : Warning<
   "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 err_declspec_thread_on_thread_variable : Error<
+  "'__declspec(thread)' applied to variable that already has a "
+  "thread-local storage specifier">;
 def err_attribute_dll_not_extern : Error<
   "%q0 must have external linkage when declared %q1">;
 def warn_attribute_invalid_on_definition : Warning<
   "'%0' attribute cannot be specified on a definition">,
   InGroup<IgnoredAttributes>;
 def err_attribute_dll_redeclaration : Error<
   "redeclaration of %q0 cannot add %q1 attribute">;
 def err_attribute_dllimport_data_definition : Error<

 def ext_in_class_initializer_non_constant : Extension<
   "in-class initializer for static data member is not a constant expression; "
   "folding it to a constant is a GNU extension">, InGroup<GNUFoldingConstant>;
 
 def err_thread_dynamic_init : Error<
   "initializer for thread-local variable must be a constant expression">;
 def err_thread_nontrivial_dtor : Error<
   "type of thread-local variable has non-trivial destruction">;
+def err_thread_nontrivial_ctor : Error<
+  "type of thread-local variable has non-trivial construction">;
 def note_use_thread_local : Note<
   "use 'thread_local' to allow this">;
 
 // C++ anonymous unions and GNU anonymous structs/unions
 def ext_anonymous_union : Extension<
   "anonymous unions are a C11 extension">, InGroup<C11>;
 def ext_gnu_anonymous_struct : Extension<
   "anonymous structs are a GNU extension">, InGroup<GNUAnonymousStruct>;

cfe/trunk/lib/AST/Decl.cpp

return new (C, ID) VarDecl(Var, 0, SourceLocation(), SourceLocation(), 0,
                              QualType(), 0, SC_None);
 }
 
 void VarDecl::setStorageClass(StorageClass SC) {
   assert(isLegalForVariable(SC));
   VarDeclBits.SClass = SC;
 }
 
+VarDecl::TLSKind VarDecl::getTLSKind() const {
+  switch (VarDeclBits.TSCSpec) {
+  case TSCS_unspecified:
+    if (hasAttr<ThreadAttr>())
+      return TLS_Static;
+    return TLS_None;
+  case TSCS___thread: // Fall through.
+  case TSCS__Thread_local:
+      return TLS_Static;
+  case TSCS_thread_local:
+    return TLS_Dynamic;
+  }
+  llvm_unreachable("Unknown thread storage class specifier!");
+}
+
 SourceRange VarDecl::getSourceRange() const {
   if (const Expr *Init = getInit()) {
     SourceLocation InitEnd = Init->getLocEnd();
     // If Init is implicit, ignore its source range and fallback on 
     // DeclaratorDecl::getSourceRange() to handle postfix elements.
     if (InitEnd.isValid() && InitEnd != getLocation())
       return SourceRange(getOuterLocStart(), InitEnd);
   }

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

static void handleMSInheritanceAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   MSInheritanceAttr *IA = S.mergeMSInheritanceAttr(
       D, Attr.getRange(), /*BestCase=*/true,
       Attr.getAttributeSpellingListIndex(),
       (MSInheritanceAttr::Spelling)Attr.getSemanticSpelling());
   if (IA)
     D->addAttr(IA);
 }
 
+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()));
+}
+
 static void handleARMInterruptAttr(Sema &S, Decl *D,
                                    const AttributeList &Attr) {
   // Check the attribute arguments.
   if (Attr.getNumArgs() > 1) {
     S.Diag(Attr.getLoc(), diag::err_attribute_too_many_arguments)
       << Attr.getName() << 1;
     return;
   }
case AttributeList::AT_Uuid:
     handleUuidAttr(S, D, Attr);
     break;
   case AttributeList::AT_MSInheritance:
     handleMSInheritanceAttr(S, D, Attr);
     break;
   case AttributeList::AT_SelectAny:
     handleSimpleAttribute<SelectAnyAttr>(S, D, Attr);
     break;
+  case AttributeList::AT_Thread:
+    handleDeclspecThreadAttr(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/SemaCXX/declspec-thread.cpp

+// RUN: %clang_cc1 -std=c++11 -fms-extensions -verify %s
+
+__thread __declspec(thread) int a; // expected-error {{already has a thread-local storage specifier}}
+__declspec(thread) __thread int b; // expected-error {{already has a thread-local storage specifier}}
+__declspec(thread) int c(); // expected-warning {{only applies to variables}}
+__declspec(thread) int d;
+int foo();
+__declspec(thread) int e = foo(); // expected-error {{must be a constant expression}} expected-note {{thread_local}}
+
+struct HasCtor { HasCtor(); int x; };
+__declspec(thread) HasCtor f; // expected-error {{must be a constant expression}} expected-note {{thread_local}}
+
+struct HasDtor { ~HasDtor(); int x; };
+__declspec(thread) HasDtor g; // expected-error {{non-trivial destruction}} expected-note {{thread_local}}
+
+struct HasDefaultedDefaultCtor {
+  HasDefaultedDefaultCtor() = default;
+  int x;
+};
+__declspec(thread) HasDefaultedDefaultCtor h;
+
+struct HasConstexprCtor {
+  constexpr HasConstexprCtor(int x) : x(x) {}
+  int x;
+};
+__declspec(thread) HasConstexprCtor i(42);
+
+int foo() {
+  __declspec(thread) int a; // expected-error {{must have global storage}}
+  static __declspec(thread) int b;
+}
+
+extern __declspec(thread) int fwd_thread_var;
+__declspec(thread) int fwd_thread_var = 5;
+
+extern int fwd_thread_var_mismatch; // expected-note {{previous declaration}}
+__declspec(thread) int fwd_thread_var_mismatch = 5; // expected-error-re {{thread-local {{.*}} follows non-thread-local}}
+
+extern __declspec(thread) int thread_mismatch_2; // expected-note {{previous declaration}}
+int thread_mismatch_2 = 5; // expected-error-re {{non-thread-local {{.*}} follows thread-local}}
+
+typedef __declspec(thread) int tls_int_t; // expected-warning {{only applies to variables}}