[clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 25 18:06:01 PDT 2021
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 <thakis at 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 at 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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211025/739a8dbc/attachment-0001.html>
More information about the cfe-commits
mailing list