[clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens

[Aaron Ballman via cfe-commits]

On Mon, Oct 18, 2021 at 3:52 PM Richard Smith <richard at metafoo.co.uk> wrote:
>
>  On Mon, 18 Oct 2021 at 12:48, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith <richard at metafoo.co.uk> wrote:
>> >
>> > On Sun, 17 Oct 2021 at 04:58, 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);
>> >
>> >
>> > How does this handle things like:
>> >
>> > #define RPAREN )
>> > #if __has_attribute(clang::fallthrough RPAREN
>> >
>> > ? I think that should be an error: the ) token should not be produced by macro expansion, analogous to the behavior of function-like macros. But I imagine unless we're careful here, we'll allow that macro expansion to terminate the "macro".
>>
>> I agree, I think that should be an error. We handle it reasonably too; I get:
>>
>> error: missing ')' after 'clang'
>>
>> though it appears there is some compiler divergence here:
>> https://godbolt.org/z/c84cb3PW7
>
>
> Oops, I meant __has_cpp_attribute, not __has_attribute. We mishandle that one just like GCC: https://godbolt.org/z/Ej84Kca16

Well that's just neat. I was trying to avoid lexing an unexpanded
token and then expanding it later if it was an identifier, but it
sounds like I should be taking that approach here. (Is that even
possible to do?)

~Aaron