Lex arguments for __has_cpp_attribute and friends as expanded tokens

The C and C++ standards require the argument to __has_cpp_attribute and __has_c_attribute to be expanded ([cpp.cond]p5). It would make little sense to expand the argument to those operators but not expand the argument to __has_attribute and __has_declspec, so those were both also changed in this patch. Note that it might make sense for the other builtins to also expand their argument, but it wasn't as clear to me whether the behavior would be correct there, and so they were left for a future revision.
llvm · Oct 17, 2021 · 2edb89c · 2edb89c
1 parent 2e0fb00
commit 2edb89c
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 36 deletions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -110,6 +110,13 @@ Attribute Changes in Clang
   attribute is handled instead, e.g. in ``handleDeclAttribute``.
   (This was changed in order to better support attributes in code completion).
 
+- __has_cpp_attribute, __has_c_attribute, __has_attribute, and __has_declspec
+  will now macro expand their argument. This causes a change in behavior for
+  code using ``__has_cpp_attribute(__clang__::attr)`` (and same for
+  ``__has_c_attribute``) where it would previously expand to ``0`` for all
+  attributes, but will now issue an error due to the expansion of the
+  predefined ``__clang__`` macro.
+
 Windows Support
 ---------------
 

diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1293,7 +1293,7 @@ static bool EvaluateHasIncludeNext(Token &Tok,
 /// integer values.
 static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
                                             Token &Tok, IdentifierInfo *II,
-                                            Preprocessor &PP,
+                                            Preprocessor &PP, bool ExpandArgs,
                                             llvm::function_ref<
                                               int(Token &Tok,
                                                   bool &HasLexedNextTok)> Op) {
@@ -1319,7 +1319,10 @@ static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
   bool SuppressDiagnostic = false;
   while (true) {
     // Parse next token.
-    PP.LexUnexpandedToken(Tok);
+    if (ExpandArgs)
+      PP.Lex(Tok);
+    else
+      PP.LexUnexpandedToken(Tok);
 
 already_lexed:
     switch (Tok.getKind()) {
@@ -1609,21 +1612,21 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
     OS << CounterValue++;
     Tok.setKind(tok::numeric_constant);
   } else if (II == Ident__has_feature) {
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
                                            diag::err_feature_check_malformed);
         return II && HasFeature(*this, II->getName());
       });
   } else if (II == Ident__has_extension) {
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
                                            diag::err_feature_check_malformed);
         return II && HasExtension(*this, II->getName());
       });
   } else if (II == Ident__has_builtin) {
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
                                            diag::err_feature_check_malformed);
@@ -1675,20 +1678,20 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
         }
       });
   } else if (II == Ident__is_identifier) {
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [](Token &Tok, bool &HasLexedNextToken) -> int {
         return Tok.is(tok::identifier);
       });
   } else if (II == Ident__has_attribute) {
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
                                            diag::err_feature_check_malformed);
         return II ? hasAttribute(AttrSyntax::GNU, nullptr, II,
                                  getTargetInfo(), getLangOpts()) : 0;
       });
   } else if (II == Ident__has_declspec) {
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
                                            diag::err_feature_check_malformed);
@@ -1704,8 +1707,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
   } else if (II == Ident__has_cpp_attribute ||
              II == Ident__has_c_attribute) {
     bool IsCXX = II == Ident__has_cpp_attribute;
-    EvaluateFeatureLikeBuiltinMacro(
-        OS, Tok, II, *this, [&](Token &Tok, bool &HasLexedNextToken) -> int {
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,
+        [&](Token &Tok, bool &HasLexedNextToken) -> int {
           IdentifierInfo *ScopeII = nullptr;
           IdentifierInfo *II = ExpectFeatureIdentifierInfo(
               Tok, *this, diag::err_feature_check_malformed);
@@ -1719,7 +1722,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
             HasLexedNextToken = true;
           else {
             ScopeII = II;
-            LexUnexpandedToken(Tok);
+            // Lex an expanded token for the attribute name.
+            Lex(Tok);
             II = ExpectFeatureIdentifierInfo(Tok, *this,
                                              diag::err_feature_check_malformed);
           }
@@ -1746,7 +1750,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
     Tok.setKind(tok::numeric_constant);
   } else if (II == Ident__has_warning) {
     // The argument should be a parenthesized string literal.
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         std::string WarningName;
         SourceLocation StrStartLoc = Tok.getLocation();
@@ -1777,7 +1781,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
     // The argument to this builtin should be an identifier. The
     // builtin evaluates to 1 when that identifier names the module we are
     // currently building.
-    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+    EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
       [this](Token &Tok, bool &HasLexedNextToken) -> int {
         IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
                                        diag::err_expected_id_building_module);
@@ -1837,28 +1841,32 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
     return;
   } else if (II == Ident__is_target_arch) {
     EvaluateFeatureLikeBuiltinMacro(
-        OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {
+        OS, Tok, II, *this, false,
+        [this](Token &Tok, bool &HasLexedNextToken) -> int {
           IdentifierInfo *II = ExpectFeatureIdentifierInfo(
               Tok, *this, diag::err_feature_check_malformed);
           return II && isTargetArch(getTargetInfo(), II);
         });
   } else if (II == Ident__is_target_vendor) {
     EvaluateFeatureLikeBuiltinMacro(
-        OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {
+        OS, Tok, II, *this, false,
+        [this](Token &Tok, bool &HasLexedNextToken) -> int {
           IdentifierInfo *II = ExpectFeatureIdentifierInfo(
               Tok, *this, diag::err_feature_check_malformed);
           return II && isTargetVendor(getTargetInfo(), II);
         });
   } else if (II == Ident__is_target_os) {
     EvaluateFeatureLikeBuiltinMacro(
-        OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {
+        OS, Tok, II, *this, false,
+        [this](Token &Tok, bool &HasLexedNextToken) -> int {
           IdentifierInfo *II = ExpectFeatureIdentifierInfo(
               Tok, *this, diag::err_feature_check_malformed);
           return II && isTargetOS(getTargetInfo(), II);
         });
   } else if (II == Ident__is_target_environment) {
     EvaluateFeatureLikeBuiltinMacro(
-        OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {
+        OS, Tok, II, *this, false,
+        [this](Token &Tok, bool &HasLexedNextToken) -> int {
           IdentifierInfo *II = ExpectFeatureIdentifierInfo(
               Tok, *this, diag::err_feature_check_malformed);
           return II && isTargetEnvironment(getTargetInfo(), II);

diff --git a/clang/test/Preprocessor/has_attribute.c b/clang/test/Preprocessor/has_attribute.c
@@ -56,3 +56,11 @@ int has_no_volatile_attribute();
 
 #if __has_cpp_attribute(selectany) // expected-error {{function-like macro '__has_cpp_attribute' is not defined}}
 #endif
+
+// Test that macro expansion of the builtin argument works.
+#define F fallthrough
+
+#if __has_attribute(F)
+int has_fallthrough;
+#endif
+// CHECK: int has_fallthrough;
diff --git a/clang/test/Preprocessor/has_attribute.cpp b/clang/test/Preprocessor/has_attribute.cpp
@@ -18,16 +18,6 @@ CXX11(clang::__fallthrough__)
 // CHECK: __gsl__::suppress: 0
 CXX11(__gsl__::suppress)
 
-// 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.
-//
-// Note, we can't use CXX11 here because it will expand __clang__ to 1
-// too early.
-// CHECK: 1::fallthrough: 0
-__clang__::fallthrough: __has_cpp_attribute(__clang__::fallthrough)
-
 // CHECK: _Clang::fallthrough: 201603L
 CXX11(_Clang::fallthrough)
 
@@ -70,6 +60,50 @@ CXX11(unlikely)
 // CHECK: noreturn: 200809L
 // CHECK: unlikely: 201803L
 
+namespace PR48462 {
+// Test that macro expansion of the builtin argument works.
+#define C clang
+#define F fallthrough
+#define CF clang::fallthrough
+
+#if __has_cpp_attribute(F)
+int has_fallthrough;
+#endif
+// CHECK: int has_fallthrough;
+
+#if __has_cpp_attribute(C::F)
+int has_clang_falthrough_1;
+#endif
+// CHECK: int has_clang_falthrough_1;
+
+#if __has_cpp_attribute(clang::F)
+int has_clang_falthrough_2;
+#endif
+// CHECK: int has_clang_falthrough_2;
+
+#if __has_cpp_attribute(C::fallthrough)
+int has_clang_falthrough_3;
+#endif
+// CHECK: int has_clang_falthrough_3;
+
+#if __has_cpp_attribute(CF)
+int has_clang_falthrough_4;
+#endif
+// CHECK: int has_clang_falthrough_4;
+
+#define FUNCLIKE1(x) clang::x
+#if __has_cpp_attribute(FUNCLIKE1(fallthrough))
+int funclike_1;
+#endif
+// CHECK: int funclike_1;
+
+#define FUNCLIKE2(x) _Clang::x
+#if __has_cpp_attribute(FUNCLIKE2(fallthrough))
+int funclike_2;
+#endif
+// CHECK: int funclike_2;
+}
+
 // Test for Microsoft __declspec attributes
 
 #define DECLSPEC(x) x: __has_declspec_attribute(x)
@@ -81,3 +115,13 @@ DECLSPEC(__uuid__)
 
 // CHECK: fallthrough: 0
 DECLSPEC(fallthrough)
+
+namespace PR48462 {
+// Test that macro expansion of the builtin argument works.
+#define U uuid
+
+#if __has_declspec_attribute(U)
+int has_uuid;
+#endif
+// CHECK: int has_uuid;
+}
diff --git a/clang/test/Preprocessor/has_attribute_errors.cpp b/clang/test/Preprocessor/has_attribute_errors.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -Eonly -verify %s
+
+// We warn users if they write an attribute like
+// [[__clang__::fallthrough]] because __clang__ is a macro that expands to 1.
+// Instead, we suggest users use [[_Clang::fallthrough]] in this situation.
+// However, because __has_cpp_attribute (and __has_c_attribute) require
+// expanding their argument tokens, __clang__ expands to 1 in the feature test
+// macro as well. We don't currently give users a kind warning in this case,
+// but we previously did not expand macros and so this would return 0. Now that
+// we properly expand macros, users will now get an error about using incorrect
+// syntax.
+
+__has_cpp_attribute(__clang__::fallthrough) // expected-error {{missing ')' after <numeric_constant>}} \
+                                            // expected-note {{to match this '('}} \
+                                            // expected-error {{builtin feature check macro requires a parenthesized identifier}}
+
diff --git a/clang/test/Preprocessor/has_c_attribute.c b/clang/test/Preprocessor/has_c_attribute.c
@@ -33,12 +33,45 @@ C2x(__gnu__::warn_unused_result)
 // CHECK: gnu::__warn_unused_result__: 201904L
 C2x(gnu::__warn_unused_result__)
 
-// 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_c_attribute for that
-// vendor namespace.
-//
-// Note, we can't use C2x here because it will expand __clang__ to 1
-// too early.
-// CHECK: 1::fallthrough: 0
-__clang__::fallthrough: __has_c_attribute(__clang__::fallthrough)
+// Test that macro expansion of the builtin argument works.
+#define C clang
+#define L likely
+#define CL clang::likely
+#define N nodiscard
+
+#if __has_c_attribute(N)
+int has_nodiscard;
+#endif
+// CHECK: int has_nodiscard;
+
+#if __has_c_attribute(C::L)
+int has_clang_likely_1;
+#endif
+// CHECK: int has_clang_likely_1;
+
+#if __has_c_attribute(clang::L)
+int has_clang_likely_2;
+#endif
+// CHECK: int has_clang_likely_2;
+
+#if __has_c_attribute(C::likely)
+int has_clang_likely_3;
+#endif
+// CHECK: int has_clang_likely_3;
+
+#if __has_c_attribute(CL)
+int has_clang_likely_4;
+#endif
+// CHECK: int has_clang_likely_4;
+
+#define FUNCLIKE1(x) clang::x
+#if __has_c_attribute(FUNCLIKE1(likely))
+int funclike_1;
+#endif
+// CHECK: int funclike_1;
+
+#define FUNCLIKE2(x) _Clang::x
+#if __has_c_attribute(FUNCLIKE2(likely))
+int funclike_2;
+#endif
+// CHECK: int funclike_2;