<div dir="ltr">Was this reviewed anywhere?<div><br></div>I'll note that some internal project apparently used to check `#if __has_attribute(__unsafe_unretained)`. That used to silently (and incorrectly I think) always return 0 as far as I can tell, but now it fails with:<div><br></div><div>```</div><div>$ out/gn/bin/clang -c <a href="http://foo.mm">foo.mm</a><br>foo.mm:1:21: error: missing ')' after '__attribute__'<br>#if __has_attribute(__unsafe_unretained)<br> ^~~~~~~~~~~~~~~~~~~<br><built-in>:350:42: note: expanded from here<br>#define __unsafe_unretained __attribute__((objc_ownership(none)))<br> ~~~~~~~~~~~~~^<br>foo.mm:1:20: note: to match this '('<br>#if __has_attribute(__unsafe_unretained)<br> ^<br>1 error generated.<br></div><div>```</div><div><br></div><div>That's arguably a progression and I'm still finding out what the intent there was (maybe `#ifdef __unsafe_unretained`?).</div><div><br></div><div>So nothing to do here I think, but I thought I'd mention it.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Oct 17, 2021 at 7:58 AM Aaron Ballman via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Aaron Ballman<br>
Date: 2021-10-17T07:54:48-04:00<br>
New Revision: 2edb89c746848c52964537268bf03e7906bf2542<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542.diff</a><br>
<br>
LOG: Lex arguments for __has_cpp_attribute and friends as expanded tokens<br>
<br>
The C and C++ standards require the argument to __has_cpp_attribute and<br>
__has_c_attribute to be expanded ([cpp.cond]p5). It would make little sense<br>
to expand the argument to those operators but not expand the argument to<br>
__has_attribute and __has_declspec, so those were both also changed in this<br>
patch.<br>
<br>
Note that it might make sense for the other builtins to also expand their<br>
argument, but it wasn't as clear to me whether the behavior would be correct<br>
there, and so they were left for a future revision.<br>
<br>
Added: <br>
clang/test/Preprocessor/has_attribute_errors.cpp<br>
<br>
Modified: <br>
clang/docs/ReleaseNotes.rst<br>
clang/lib/Lex/PPMacroExpansion.cpp<br>
clang/test/Preprocessor/has_attribute.c<br>
clang/test/Preprocessor/has_attribute.cpp<br>
clang/test/Preprocessor/has_c_attribute.c<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst<br>
index 6501a4870e2a6..263eae83036df 100644<br>
--- a/clang/docs/ReleaseNotes.rst<br>
+++ b/clang/docs/ReleaseNotes.rst<br>
@@ -110,6 +110,13 @@ Attribute Changes in Clang<br>
attribute is handled instead, e.g. in ``handleDeclAttribute``.<br>
(This was changed in order to better support attributes in code completion).<br>
<br>
+- __has_cpp_attribute, __has_c_attribute, __has_attribute, and __has_declspec<br>
+ will now macro expand their argument. This causes a change in behavior for<br>
+ code using ``__has_cpp_attribute(__clang__::attr)`` (and same for<br>
+ ``__has_c_attribute``) where it would previously expand to ``0`` for all<br>
+ attributes, but will now issue an error due to the expansion of the<br>
+ predefined ``__clang__`` macro.<br>
+<br>
Windows Support<br>
---------------<br>
<br>
<br>
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp<br>
index bf19f538647e6..5a0fa5184e38b 100644<br>
--- a/clang/lib/Lex/PPMacroExpansion.cpp<br>
+++ b/clang/lib/Lex/PPMacroExpansion.cpp<br>
@@ -1293,7 +1293,7 @@ static bool EvaluateHasIncludeNext(Token &Tok,<br>
/// integer values.<br>
static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,<br>
Token &Tok, IdentifierInfo *II,<br>
- Preprocessor &PP,<br>
+ Preprocessor &PP, bool ExpandArgs,<br>
llvm::function_ref<<br>
int(Token &Tok,<br>
bool &HasLexedNextTok)> Op) {<br>
@@ -1319,7 +1319,10 @@ static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,<br>
bool SuppressDiagnostic = false;<br>
while (true) {<br>
// Parse next token.<br>
- PP.LexUnexpandedToken(Tok);<br>
+ if (ExpandArgs)<br>
+ PP.Lex(Tok);<br>
+ else<br>
+ PP.LexUnexpandedToken(Tok);<br>
<br>
already_lexed:<br>
switch (Tok.getKind()) {<br>
@@ -1609,21 +1612,21 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
OS << CounterValue++;<br>
Tok.setKind(tok::numeric_constant);<br>
} else if (II == Ident__has_feature) {<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_feature_check_malformed);<br>
return II && HasFeature(*this, II->getName());<br>
});<br>
} else if (II == Ident__has_extension) {<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_feature_check_malformed);<br>
return II && HasExtension(*this, II->getName());<br>
});<br>
} else if (II == Ident__has_builtin) {<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_feature_check_malformed);<br>
@@ -1675,12 +1678,12 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
}<br>
});<br>
} else if (II == Ident__is_identifier) {<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,<br>
[](Token &Tok, bool &HasLexedNextToken) -> int {<br>
return Tok.is(tok::identifier);<br>
});<br>
} else if (II == Ident__has_attribute) {<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_feature_check_malformed);<br>
@@ -1688,7 +1691,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
getTargetInfo(), getLangOpts()) : 0;<br>
});<br>
} else if (II == Ident__has_declspec) {<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_feature_check_malformed);<br>
@@ -1704,8 +1707,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
} else if (II == Ident__has_cpp_attribute ||<br>
II == Ident__has_c_attribute) {<br>
bool IsCXX = II == Ident__has_cpp_attribute;<br>
- EvaluateFeatureLikeBuiltinMacro(<br>
- OS, Tok, II, *this, [&](Token &Tok, bool &HasLexedNextToken) -> int {<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,<br>
+ [&](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *ScopeII = nullptr;<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(<br>
Tok, *this, diag::err_feature_check_malformed);<br>
@@ -1719,7 +1722,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
HasLexedNextToken = true;<br>
else {<br>
ScopeII = II;<br>
- LexUnexpandedToken(Tok);<br>
+ // Lex an expanded token for the attribute name.<br>
+ Lex(Tok);<br>
II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_feature_check_malformed);<br>
}<br>
@@ -1746,7 +1750,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
Tok.setKind(tok::numeric_constant);<br>
} else if (II == Ident__has_warning) {<br>
// The argument should be a parenthesized string literal.<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
std::string WarningName;<br>
SourceLocation StrStartLoc = Tok.getLocation();<br>
@@ -1777,7 +1781,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
// The argument to this builtin should be an identifier. The<br>
// builtin evaluates to 1 when that identifier names the module we are<br>
// currently building.<br>
- EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,<br>
+ EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,<br>
[this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,<br>
diag::err_expected_id_building_module);<br>
@@ -1837,28 +1841,32 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {<br>
return;<br>
} else if (II == Ident__is_target_arch) {<br>
EvaluateFeatureLikeBuiltinMacro(<br>
- OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
+ OS, Tok, II, *this, false,<br>
+ [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(<br>
Tok, *this, diag::err_feature_check_malformed);<br>
return II && isTargetArch(getTargetInfo(), II);<br>
});<br>
} else if (II == Ident__is_target_vendor) {<br>
EvaluateFeatureLikeBuiltinMacro(<br>
- OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
+ OS, Tok, II, *this, false,<br>
+ [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(<br>
Tok, *this, diag::err_feature_check_malformed);<br>
return II && isTargetVendor(getTargetInfo(), II);<br>
});<br>
} else if (II == Ident__is_target_os) {<br>
EvaluateFeatureLikeBuiltinMacro(<br>
- OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
+ OS, Tok, II, *this, false,<br>
+ [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(<br>
Tok, *this, diag::err_feature_check_malformed);<br>
return II && isTargetOS(getTargetInfo(), II);<br>
});<br>
} else if (II == Ident__is_target_environment) {<br>
EvaluateFeatureLikeBuiltinMacro(<br>
- OS, Tok, II, *this, [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
+ OS, Tok, II, *this, false,<br>
+ [this](Token &Tok, bool &HasLexedNextToken) -> int {<br>
IdentifierInfo *II = ExpectFeatureIdentifierInfo(<br>
Tok, *this, diag::err_feature_check_malformed);<br>
return II && isTargetEnvironment(getTargetInfo(), II);<br>
<br>
diff --git a/clang/test/Preprocessor/has_attribute.c b/clang/test/Preprocessor/has_attribute.c<br>
index 4970dc5904230..eef168e879103 100644<br>
--- a/clang/test/Preprocessor/has_attribute.c<br>
+++ b/clang/test/Preprocessor/has_attribute.c<br>
@@ -56,3 +56,11 @@ int has_no_volatile_attribute();<br>
<br>
#if __has_cpp_attribute(selectany) // expected-error {{function-like macro '__has_cpp_attribute' is not defined}}<br>
#endif<br>
+<br>
+// Test that macro expansion of the builtin argument works.<br>
+#define F fallthrough<br>
+<br>
+#if __has_attribute(F)<br>
+int has_fallthrough;<br>
+#endif<br>
+// CHECK: int has_fallthrough;<br>
<br>
diff --git a/clang/test/Preprocessor/has_attribute.cpp b/clang/test/Preprocessor/has_attribute.cpp<br>
index fe7d29f15de1a..bf0f9b3bc4a8f 100644<br>
--- a/clang/test/Preprocessor/has_attribute.cpp<br>
+++ b/clang/test/Preprocessor/has_attribute.cpp<br>
@@ -18,16 +18,6 @@ CXX11(clang::__fallthrough__)<br>
// CHECK: __gsl__::suppress: 0<br>
CXX11(__gsl__::suppress)<br>
<br>
-// We do somewhat support the __clang__ vendor namespace, but it is a<br>
-// predefined macro and thus we encourage users to use _Clang instead.<br>
-// Because of this, we do not support __has_cpp_attribute for that<br>
-// vendor namespace.<br>
-//<br>
-// Note, we can't use CXX11 here because it will expand __clang__ to 1<br>
-// too early.<br>
-// CHECK: 1::fallthrough: 0<br>
-__clang__::fallthrough: __has_cpp_attribute(__clang__::fallthrough)<br>
-<br>
// CHECK: _Clang::fallthrough: 201603L<br>
CXX11(_Clang::fallthrough)<br>
<br>
@@ -70,6 +60,50 @@ CXX11(unlikely)<br>
// CHECK: noreturn: 200809L<br>
// CHECK: unlikely: 201803L<br>
<br>
+namespace PR48462 {<br>
+// Test that macro expansion of the builtin argument works.<br>
+#define C clang<br>
+#define F fallthrough<br>
+#define CF clang::fallthrough<br>
+<br>
+#if __has_cpp_attribute(F)<br>
+int has_fallthrough;<br>
+#endif<br>
+// CHECK: int has_fallthrough;<br>
+<br>
+#if __has_cpp_attribute(C::F)<br>
+int has_clang_falthrough_1;<br>
+#endif<br>
+// CHECK: int has_clang_falthrough_1;<br>
+<br>
+#if __has_cpp_attribute(clang::F)<br>
+int has_clang_falthrough_2;<br>
+#endif<br>
+// CHECK: int has_clang_falthrough_2;<br>
+<br>
+#if __has_cpp_attribute(C::fallthrough)<br>
+int has_clang_falthrough_3;<br>
+#endif<br>
+// CHECK: int has_clang_falthrough_3;<br>
+<br>
+#if __has_cpp_attribute(CF)<br>
+int has_clang_falthrough_4;<br>
+#endif<br>
+// CHECK: int has_clang_falthrough_4;<br>
+<br>
+#define FUNCLIKE1(x) clang::x<br>
+#if __has_cpp_attribute(FUNCLIKE1(fallthrough))<br>
+int funclike_1;<br>
+#endif<br>
+// CHECK: int funclike_1;<br>
+<br>
+#define FUNCLIKE2(x) _Clang::x<br>
+#if __has_cpp_attribute(FUNCLIKE2(fallthrough))<br>
+int funclike_2;<br>
+#endif<br>
+// CHECK: int funclike_2;<br>
+}<br>
+<br>
// Test for Microsoft __declspec attributes<br>
<br>
#define DECLSPEC(x) x: __has_declspec_attribute(x)<br>
@@ -81,3 +115,13 @@ DECLSPEC(__uuid__)<br>
<br>
// CHECK: fallthrough: 0<br>
DECLSPEC(fallthrough)<br>
+<br>
+namespace PR48462 {<br>
+// Test that macro expansion of the builtin argument works.<br>
+#define U uuid<br>
+<br>
+#if __has_declspec_attribute(U)<br>
+int has_uuid;<br>
+#endif<br>
+// CHECK: int has_uuid;<br>
+}<br>
<br>
diff --git a/clang/test/Preprocessor/has_attribute_errors.cpp b/clang/test/Preprocessor/has_attribute_errors.cpp<br>
new file mode 100644<br>
index 0000000000000..1fc88d3f926fb<br>
--- /dev/null<br>
+++ b/clang/test/Preprocessor/has_attribute_errors.cpp<br>
@@ -0,0 +1,16 @@<br>
+// RUN: %clang_cc1 -triple i386-unknown-unknown -Eonly -verify %s<br>
+<br>
+// We warn users if they write an attribute like<br>
+// [[__clang__::fallthrough]] because __clang__ is a macro that expands to 1.<br>
+// Instead, we suggest users use [[_Clang::fallthrough]] in this situation.<br>
+// However, because __has_cpp_attribute (and __has_c_attribute) require<br>
+// expanding their argument tokens, __clang__ expands to 1 in the feature test<br>
+// macro as well. We don't currently give users a kind warning in this case,<br>
+// but we previously did not expand macros and so this would return 0. Now that<br>
+// we properly expand macros, users will now get an error about using incorrect<br>
+// syntax.<br>
+<br>
+__has_cpp_attribute(__clang__::fallthrough) // expected-error {{missing ')' after <numeric_constant>}} \<br>
+ // expected-note {{to match this '('}} \<br>
+ // expected-error {{builtin feature check macro requires a parenthesized identifier}}<br>
+<br>
<br>
diff --git a/clang/test/Preprocessor/has_c_attribute.c b/clang/test/Preprocessor/has_c_attribute.c<br>
index 670e42a97926e..36dd1c80e7802 100644<br>
--- a/clang/test/Preprocessor/has_c_attribute.c<br>
+++ b/clang/test/Preprocessor/has_c_attribute.c<br>
@@ -33,12 +33,45 @@ C2x(__gnu__::warn_unused_result)<br>
// CHECK: gnu::__warn_unused_result__: 201904L<br>
C2x(gnu::__warn_unused_result__)<br>
<br>
-// We do somewhat support the __clang__ vendor namespace, but it is a<br>
-// predefined macro and thus we encourage users to use _Clang instead.<br>
-// Because of this, we do not support __has_c_attribute for that<br>
-// vendor namespace.<br>
-//<br>
-// Note, we can't use C2x here because it will expand __clang__ to 1<br>
-// too early.<br>
-// CHECK: 1::fallthrough: 0<br>
-__clang__::fallthrough: __has_c_attribute(__clang__::fallthrough)<br>
+// Test that macro expansion of the builtin argument works.<br>
+#define C clang<br>
+#define L likely<br>
+#define CL clang::likely<br>
+#define N nodiscard<br>
+<br>
+#if __has_c_attribute(N)<br>
+int has_nodiscard;<br>
+#endif<br>
+// CHECK: int has_nodiscard;<br>
+<br>
+#if __has_c_attribute(C::L)<br>
+int has_clang_likely_1;<br>
+#endif<br>
+// CHECK: int has_clang_likely_1;<br>
+<br>
+#if __has_c_attribute(clang::L)<br>
+int has_clang_likely_2;<br>
+#endif<br>
+// CHECK: int has_clang_likely_2;<br>
+<br>
+#if __has_c_attribute(C::likely)<br>
+int has_clang_likely_3;<br>
+#endif<br>
+// CHECK: int has_clang_likely_3;<br>
+<br>
+#if __has_c_attribute(CL)<br>
+int has_clang_likely_4;<br>
+#endif<br>
+// CHECK: int has_clang_likely_4;<br>
+<br>
+#define FUNCLIKE1(x) clang::x<br>
+#if __has_c_attribute(FUNCLIKE1(likely))<br>
+int funclike_1;<br>
+#endif<br>
+// CHECK: int funclike_1;<br>
+<br>
+#define FUNCLIKE2(x) _Clang::x<br>
+#if __has_c_attribute(FUNCLIKE2(likely))<br>
+int funclike_2;<br>
+#endif<br>
+// CHECK: int funclike_2;<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>