Support _Clang as a scoped attribute identifier

[aaron.ballman]

Summary

Currently, we only accept clang as the scoped attribute identifier for double square bracket attributes provided by Clang, but this has the potential to conflict with user-defined macros. To help alleviate these concerns, this patch introduces the _Clang scoped attribute identifier as an alias for clang.

GCC elected to go with __gnu__ for their protected name, but we cannot follow suit due to __clang__ being a predefined macro that identifies the implementation. I added a warning with a fixit on the off chance someone attempts to use __clang__ as the scoped attribute identifier, but do not support the spelling fully (for instance, __has_cpp_attribute does not support __clang__).

[aaron.ballman]

Ping

[aaron.ballman]

Another option that @rsmith and I discussed today is perhaps using __clang or clang__ as the identifier, but perhaps this will cause more confusion about where to put underscores than _Clang would. If there are other ideas that are more palatable for the identifier, that'd be great -- I am not strongly tied to a spelling.

Adding some libc++ maintainers to see if they have opinions on the identifier picked.

[ldionne]

I'm fine with _Clang or anything else too. Thanks for doing this!

lib/Basic/Attributes.cpp

18

That was a complicated way of doing something simple.

[rsmith]

Let's do it. The diagnostic and fix-it is awesome ;)

[aaron.ballman]

Thanks for the reviews -- I've commit in r346521.

include/clang/Basic/DiagnosticParseKinds.td

   "standalone declaration">;
 
 def warn_cxx98_compat_noexcept_expr : Warning<
   "noexcept expressions are incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
 def warn_cxx98_compat_nullptr : Warning<
   "'nullptr' is incompatible with C++98">, InGroup<CXX98Compat>, DefaultIgnore;
 
+def warn_wrong_clang_attr_namespace : Warning<
+  "'__clang__' is a predefined macro name, not an attribute scope specifier; "
+  "did you mean '_Clang' instead?">, InGroup<IgnoredAttributes>;
 def ext_ns_enum_attribute : Extension<
   "attributes on %select{a namespace|an enumerator}0 declaration are "
   "a C++17 extension">, InGroup<CXX17>;
 def warn_cxx14_compat_ns_enum_attribute : Warning<
   "attributes on %select{a namespace|an enumerator}0 declaration are "
   "incompatible with C++ standards before C++17">,
   InGroup<CXXPre17CompatPedantic>, DefaultIgnore;
 def warn_cxx98_compat_alignas : Warning<"'alignas' is incompatible with C++98">,

lib/Basic/Attributes.cpp

 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/AttrSubjectMatchRules.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "llvm/ADT/StringSwitch.h"
 using namespace clang;
 
 int clang::hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope,
                         const IdentifierInfo *Attr, const TargetInfo &Target,
                         const LangOptions &LangOpts) {
   StringRef Name = Attr->getName();
   // Normalize the attribute name, __foo__ becomes foo.
   if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))
     Name = Name.substr(2, Name.size() - 4);
 
-  // Normalize the scope name, but only for gnu attributes.
+  // Normalize the scope name, but only for gnu and clang attributes.
   StringRef ScopeName = Scope ? Scope->getName() : "";
   if (ScopeName == "__gnu__")
-    ScopeName = ScopeName.slice(2, ScopeName.size() - 2);
+    ScopeName = "gnu";
+  else if (ScopeName == "_Clang")
+    ScopeName = "clang";
 
 #include "clang/Basic/AttrHasAttributeImpl.inc"
 
   return 0;
 }
 
 const char *attr::getSubjectMatchRuleSpelling(attr::SubjectMatchRule Rule) {
   switch (Rule) {
 #define ATTR_MATCH_RULE(NAME, SPELLING, IsAbstract)                            \
   case attr::NAME:                                                             \
     return SPELLING;
 #include "clang/Basic/AttrSubMatchRulesList.inc"
   }
   llvm_unreachable("Invalid subject match rule");
 }

lib/Parse/ParseDeclCXX.cpp

     if (!Tok.isAnnotation()) {
       if (IdentifierInfo *II = Tok.getIdentifierInfo()) {
         Loc = ConsumeToken();
         return II;
       }
     }
     return nullptr;
 
+  case tok::numeric_constant: {
+    // If we got a numeric constant, check to see if it comes from a macro that
+    // corresponds to the predefined __clang__ macro. If it does, warn the user
+    // and recover by pretending they said _Clang instead.
+    if (Tok.getLocation().isMacroID()) {
+      SmallString<8> ExpansionBuf;
+      SourceLocation ExpansionLoc =
+          PP.getSourceManager().getExpansionLoc(Tok.getLocation());
+      StringRef Spelling = PP.getSpelling(ExpansionLoc, ExpansionBuf);
+      if (Spelling == "__clang__") {
+        SourceRange TokRange(
+            ExpansionLoc,
+            PP.getSourceManager().getExpansionLoc(Tok.getEndLoc()));
+        Diag(Tok, diag::warn_wrong_clang_attr_namespace)
+            << FixItHint::CreateReplacement(TokRange, "_Clang");
+        Loc = ConsumeToken();
+        return &PP.getIdentifierTable().get("_Clang");
+      }
+    }
+    return nullptr;
+  }
+
   case tok::ampamp:       // 'and'
   case tok::pipe:         // 'bitor'
   case tok::pipepipe:     // 'or'
   case tok::caret:        // 'xor'
   case tok::tilde:        // 'compl'
   case tok::amp:          // 'bitand'
   case tok::ampequal:     // 'and_eq'
   case tok::pipeequal:    // 'or_eq'

   if (!hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
                     AttrName, getTargetInfo(), getLangOpts())) {
     // Eat the left paren, then skip to the ending right paren.
     ConsumeParen();
     SkipUntil(tok::r_paren);
     return false;
   }
 
-  if (ScopeName &&
-      (ScopeName->getName() == "gnu" || ScopeName->getName() == "__gnu__")) {
+  if (ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"))) {
     // GNU-scoped attributes have some special cases to handle GNU-specific
     // behaviors.
     ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,
                           ScopeLoc, Syntax, nullptr);
     return true;
   }
 
   unsigned NumArgs;
   // Some Clang-scoped attributes have some special parsing behavior.
-  if (ScopeName && ScopeName->getName() == "clang")
-    NumArgs =
-        ParseClangAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,
-                                ScopeLoc, Syntax);
+  if (ScopeName && (ScopeName->isStr("clang") || ScopeName->isStr("_Clang")))
+    NumArgs = ParseClangAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc,
+                                      ScopeName, ScopeLoc, Syntax);
   else
     NumArgs =
         ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc,
                                  ScopeName, ScopeLoc, Syntax);
 
   if (!Attrs.empty() &&
       IsBuiltInOrStandardCXX11Attribute(AttrName, ScopeName)) {
     ParsedAttr &Attr = Attrs.back();

lib/Sema/ParsedAttr.cpp

void AttributePool::takePool(AttributePool &pool) {
   Attrs.insert(Attrs.end(), pool.Attrs.begin(), pool.Attrs.end());
   pool.Attrs.clear();
 }
 
 #include "clang/Sema/AttrParsedAttrKinds.inc"
 
 static StringRef normalizeAttrScopeName(StringRef ScopeName,
                                         ParsedAttr::Syntax SyntaxUsed) {
-  // We currently only normalize the "__gnu__" scope name to be "gnu".
-  if ((SyntaxUsed == ParsedAttr::AS_CXX11 ||
-       SyntaxUsed == ParsedAttr::AS_C2x) &&
-      ScopeName == "__gnu__")
-    ScopeName = ScopeName.slice(2, ScopeName.size() - 2);
+  // Normalize the "__gnu__" scope name to be "gnu" and the "_Clang" scope name
+  // to be "clang".
+  if (SyntaxUsed == ParsedAttr::AS_CXX11 ||
+    SyntaxUsed == ParsedAttr::AS_C2x) {
+    if (ScopeName == "__gnu__")
+      ScopeName = "gnu";
+    else if (ScopeName == "_Clang")
+      ScopeName = "clang";
+  }
   return ScopeName;
 }
 
 static StringRef normalizeAttrName(StringRef AttrName,
                                    StringRef NormalizedScopeName,
                                    ParsedAttr::Syntax SyntaxUsed) {
   // Normalize the attribute name, __foo__ becomes foo. This is only allowable
   // for GNU attributes, and attributes using the double square bracket syntax.

test/FixIt/fixit-cxx11-attributes.cpp

class [[]] [[]]
     // CHECK: fix-it:{{.*}}:{9:32-9:41}
 
   class [[]] [[]]
    attr_after_class_name_definition [[]] [[]] [[]]{}; // expected-error {{an attribute list cannot appear here}}
    // CHECK: fix-it:{{.*}}:{14:4-14:4}
    // CHECK: fix-it:{{.*}}:{14:37-14:51}
 
   class base {};
-  class [[]] [[]] final_class 
+  class [[]] [[]] final_class
     alignas(float) [[]] final // expected-error {{an attribute list cannot appear here}}
     alignas(float) [[]] [[]] alignas(float): base{}; // expected-error {{an attribute list cannot appear here}}
     // CHECK: fix-it:{{.*}}:{19:19-19:19}
     // CHECK: fix-it:{{.*}}:{20:5-20:25}
     // CHECK: fix-it:{{.*}}:{19:19-19:19}
     // CHECK: fix-it:{{.*}}:{21:5-21:44}
 
-  class [[]] [[]] final_class_another 
+  class [[]] [[]] final_class_another
     [[]] [[]] alignas(16) final // expected-error {{an attribute list cannot appear here}}
     [[]] [[]] alignas(16) [[]]{}; // expected-error {{an attribute list cannot appear here}}
     // CHECK: fix-it:{{.*}}:{27:19-27:19}
     // CHECK: fix-it:{{.*}}:{28:5-28:27}
     // CHECK: fix-it:{{.*}}:{27:19-27:19}
     // CHECK: fix-it:{{.*}}:{29:5-29:31}
 }
 
 namespace BaseSpecifier {
   struct base1 {};
   struct base2 {};
   class with_base_spec : public [[a]] // expected-error {{an attribute list cannot appear here}} expected-warning {{unknown}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:26-[[@LINE-1]]:26}:"[{{\[}}a]]"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:33-[[@LINE-2]]:39}:""
                          virtual [[b]] base1, // expected-error {{an attribute list cannot appear here}} expected-warning {{unknown}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:26}:"[{{\[}}b]]"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:34-[[@LINE-2]]:40}:""
                          virtual [[c]] // expected-error {{an attribute list cannot appear here}} expected-warning {{unknown}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:26-[[@LINE-1]]:26}:"[{{\[}}c]]"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:34-[[@LINE-2]]:40}:""
                          public [[d]] base2 {}; // expected-error {{an attribute list cannot appear here}} expected-warning {{unknown}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:26}:"[{{\[}}d]]"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:33-[[@LINE-2]]:39}:""
 }
+
+[[__clang__::annotate("test")]] void annotate3();  // expected-warning {{'__clang__' is a predefined macro name, not an attribute scope specifier; did you mean '_Clang' instead?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"_Clang"

test/Parser/cxx0x-attributes.cpp

 }
 
 #define attr_name bitand
 #define attr_name_2(x) x
 #define attr_name_3(x, y) x##y
 [[attr_name, attr_name_2(bitor), attr_name_3(com, pl)]] int macro_attrs; // expected-warning {{unknown attribute 'compl' ignored}} \
    expected-warning {{unknown attribute 'bitor' ignored}} \
    expected-warning {{unknown attribute 'bitand' ignored}}
+
+// Check that we can parse an attribute in our vendor namespace.
+[[clang::annotate("test")]] void annotate1();
+[[_Clang::annotate("test")]] void annotate2();
+// Note: __clang__ is a predefined macro, which is why _Clang is the
+// prefered "protected" vendor namespace. We support __clang__ only for
+// people expecting it to behave the same as __gnu__.
+[[__clang__::annotate("test")]] void annotate3();  // expected-warning {{'__clang__' is a predefined macro name, not an attribute scope specifier; did you mean '_Clang' instead?}}

test/Preprocessor/has_attribute.cpp

 #endif
 
 // The attribute name can be bracketed with double underscores.
 // CHECK: has_clang_fallthrough_2
 #if __has_cpp_attribute(clang::__fallthrough__)
   int has_clang_fallthrough_2();
 #endif
 
-// The scope cannot be bracketed with double underscores unless it is for gnu.
+// The scope cannot be bracketed with double underscores unless it is
+// for gnu or clang.
+// CHECK: does_not_have___gsl___suppress
+#if !__has_cpp_attribute(__gsl__::suppress)
+  int does_not_have___gsl___suppress();
+#endif
+
+// We do somewhat support the __clang__ vendor namespace, but it is a
+// predefined macro and thus we encourage users to use _Clang instead.
+// Because of this, we do not support __has_cpp_attribute for that
+// vendor namespace.
 // CHECK: does_not_have___clang___fallthrough
 #if !__has_cpp_attribute(__clang__::fallthrough)
   int does_not_have___clang___fallthrough();
 #endif
+
+// CHECK: does_have_Clang_fallthrough
+#if __has_cpp_attribute(_Clang::fallthrough)
+  int does_have_Clang_fallthrough();
+#endif
+
 // CHECK: has_gnu_const
 #if __has_cpp_attribute(__gnu__::__const__)
   int has_gnu_const();
 #endif
 
 // Test that C++11, target-specific attributes behave properly.
 
 // CHECK: does_not_have_mips16

test/SemaCXX/attr-optnone.cpp

 
 struct B2 {
   [[clang::optnone]]
   void foo();
   [[clang::optnone]]
   static void bar();
 };
 
+// Verify that we can handle the [[_Clang::optnone]] and
+// [[__clang__::optnone]] spellings.
+[[_Clang::optnone]] int foo3();
+[[__clang__::optnone]] int foo4(); // expected-warning {{'__clang__' is a predefined macro name, not an attribute scope specifier; did you mean '_Clang' instead?}}
+
+[[_Clang::optnone]] int foo5; // expected-warning {{'optnone' attribute only applies to functions}}