[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 17:40:47 PDT 2021
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/999327ef/attachment-0001.html>
More information about the cfe-commits
mailing list