[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: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

~Aaron