On Mon, Oct 25, 2021 at 9:06 PM Nico Weber <tha...@chromium.org> wrote:
> 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? > Aaron: Ping on ^ :) > > 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
