Looks like the reason for this check was that https://github.com/llvm/llvm-project/blob/3b42fc8a07c37e47efae80c931eff7e63103e0e9/clang/include/clang/Basic/Attr.td#L1946 contains __unsafe_unretained despite it just being a macro here: http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/InitPreprocessor.cpp#1019
Is that Attr.td entry incorrect? On Mon, Oct 25, 2021 at 8:40 PM Nico Weber <tha...@chromium.org> wrote: > Was this reviewed anywhere? > > 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: > > ``` > $ out/gn/bin/clang -c foo.mm > foo.mm:1:21: error: missing ')' after '__attribute__' > #if __has_attribute(__unsafe_unretained) > ^~~~~~~~~~~~~~~~~~~ > <built-in>:350:42: note: expanded from here > #define __unsafe_unretained __attribute__((objc_ownership(none))) > ~~~~~~~~~~~~~^ > foo.mm:1:20: note: to match this '(' > #if __has_attribute(__unsafe_unretained) > ^ > 1 error generated. > ``` > > That's arguably a progression and I'm still finding out what the intent > there was (maybe `#ifdef __unsafe_unretained`?). > > So nothing to do here I think, but I thought I'd mention it. > > On Sun, Oct 17, 2021 at 7:58 AM Aaron Ballman via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> Author: Aaron Ballman >> Date: 2021-10-17T07:54:48-04:00 >> New Revision: 2edb89c746848c52964537268bf03e7906bf2542 >> >> URL: >> https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542 >> DIFF: >> https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542.diff >> >> LOG: 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. >> >> Added: >> clang/test/Preprocessor/has_attribute_errors.cpp >> >> Modified: >> clang/docs/ReleaseNotes.rst >> clang/lib/Lex/PPMacroExpansion.cpp >> clang/test/Preprocessor/has_attribute.c >> clang/test/Preprocessor/has_attribute.cpp >> clang/test/Preprocessor/has_c_attribute.c >> >> Removed: >> >> >> >> >> ################################################################################ >> diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst >> index 6501a4870e2a6..263eae83036df 100644 >> --- 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 >> index bf19f538647e6..5a0fa5184e38b 100644 >> --- 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,12 +1678,12 @@ 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); >> @@ -1688,7 +1691,7 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { >> 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 >> index 4970dc5904230..eef168e879103 100644 >> --- 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 >> index fe7d29f15de1a..bf0f9b3bc4a8f 100644 >> --- 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 >> new file mode 100644 >> index 0000000000000..1fc88d3f926fb >> --- /dev/null >> +++ 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 >> index 670e42a97926e..36dd1c80e7802 100644 >> --- 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; >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits