Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Thu, Oct 28, 2021 at 9:55 AM Nico Weber wrote:
>
> On Mon, Oct 25, 2021 at 9:06 PM Nico Weber 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 ^ :)
Oh, perhaps my original reply got dropped somewhere along the way.
Here is what I had written before:
I did some digging:
The attribute was added three years ago in
https://github.com/llvm/llvm-project/commit/e43e2b3667f9bc29c23be5235b53a7d7afcdb181
The empty macro was added six years ago in
https://github.com/llvm/llvm-project/commit/28ea04fc4caa40a04220c61bbcadd9d692aa0c7f
The nonempty macro was added eleven years ago in
https://github.com/llvm/llvm-project/commit/5d36a8cc700ef258df055baf95a248f85f36
I think the entry in Attr.td is correct-ish. It specifies there's an
attribute that's spelled with a keyword spelling, but TokenKinds.def
does not define a keyword for __unsafe_unretained (and keywords
defined in Attr.td are not automagically turned into keywords in
TokenKinds.def); this acts as a way to connect the macro to the
attribute for maintainers. So there is no __unsafe_unretained keyword,
it's actually a macro which is sometimes not defined, sometimes
defined to be empty, and sometimes defined to be another attribute.
The type attribute from Attr.td is still used by the type system and
is added automatically when the lifetime qualifier is OCL_ExplicitNone
(https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaType.cpp#L6706).
However, I say "correct-ish" because I think the keyword spelling used
in Attr.td is incorrect -- ObjCInertUnsafeUnretainedAttr is a
different attribute than the one you get from the expansion of
__unsafe_unretained, so while the attribute and the macro are related,
they're not the same thing. As best I can tell,
ObjCInertUnsafeUnretainedAttr can only ever be created implicitly, so
I think the entry in Attr.td should have its spelling removed and a
comment added that says `// This attribute has no spellings as it is
only ever created implicitly.` as with other implicit-defined
attributes. Richard, do you agree?
All that said, __has_attribute() does not tell you anything about
keyword attributes, only GNU-style attributes. __unsafe_unretained is
not a GNU spelling, so I agree that the construct never returned true
for you before.
~Aaron
>
>>
>>
>> On Mon, Oct 25, 2021 at 8:40 PM Nico Weber 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)
>>> ^~~
>>> :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
>>> 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
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, Oct 25, 2021 at 9:06 PM Nico Weber 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 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)
>> ^~~
>> :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 ,
>>> /// integer values.
>>> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream&
>>> OS,
>>> Token , IdentifierInfo
>>> *II,
>>> -Preprocessor ,
>>> +Preprocessor , bool
>>> ExpandArgs,
>>> llvm::function_ref<
>>>int(Token ,
>>>bool
>>> )> 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
>>> +
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, Oct 25, 2021 at 8:41 PM Nico Weber wrote:
>
> Was this reviewed anywhere?
Post-commit review (as attribute code owner).
> 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)
> ^~~
> :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.
Oh, that's interesting! If think `#ifdef __unsafe_unretained` is
likely what they were going for, but there are circumstances where
it's defined to be an empty expansion, so that might not be fully
correct.
> 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?
I did some digging:
The attribute was added three years ago in
https://github.com/llvm/llvm-project/commit/e43e2b3667f9bc29c23be5235b53a7d7afcdb181
The empty macro was added six years ago in
https://github.com/llvm/llvm-project/commit/28ea04fc4caa40a04220c61bbcadd9d692aa0c7f
The nonempty macro was added eleven years ago in
https://github.com/llvm/llvm-project/commit/5d36a8cc700ef258df055baf95a248f85f36
I think the entry in Attr.td is correct-ish. It specifies there's an
attribute that's spelled with a keyword spelling, but TokenKinds.def
does not define a keyword for __unsafe_unretained (and keywords
defined in Attr.td are not automagically turned into keywords in
TokenKinds.def); this acts as a way to connect the macro to the
attribute for maintainers. So there is no __unsafe_unretained keyword,
it's actually a macro which is sometimes not defined, sometimes
defined to be empty, and sometimes defined to be another attribute.
The type attribute from Attr.td is still used by the type system and
is added automatically when the lifetime qualifier is OCL_ExplicitNone
(https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaType.cpp#L6706).
However, I say "correct-ish" because I think the keyword spelling used
in Attr.td is incorrect -- ObjCInertUnsafeUnretainedAttr is a
different attribute than the one you get from the expansion of
__unsafe_unretained, so while the attribute and the macro are related,
they're not the same thing. As best I can tell,
ObjCInertUnsafeUnretainedAttr can only ever be created implicitly, so
I think the entry in Attr.td should have its spelling removed and a
comment added that says `// This attribute has no spellings as it is
only ever created implicitly.` as with other implicit-defined
attributes. Richard, do you agree?
All that said, __has_attribute() does not tell you anything about
keyword attributes, only GNU-style attributes. __unsafe_unretained is
not a GNU spelling, so I agree that the construct never returned true
for you before.
~Aaron
>
> On Sun, Oct 17, 2021 at 7:58 AM Aaron Ballman via cfe-commits
> 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:
>>
>>
>>
>>
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
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 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)
> ^~~
> :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 ,
>> /// integer values.
>> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream&
>> OS,
>> Token , IdentifierInfo
>> *II,
>> -Preprocessor ,
>> +Preprocessor , bool
>> ExpandArgs,
>> llvm::function_ref<
>>int(Token ,
>>bool
>> )> 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 )
>> {
>> OS << CounterValue++;
>>
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
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)
^~~
: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 ,
> /// integer values.
> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
> Token , IdentifierInfo
> *II,
> -Preprocessor ,
> +Preprocessor , bool
> ExpandArgs,
> llvm::function_ref<
>int(Token ,
>bool )>
> 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 ) {
> 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 , bool ) -> 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,
> +
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, Oct 18, 2021 at 4:13 PM Richard Smith wrote:
>
> On Mon, 18 Oct 2021 at 12:56, Aaron Ballman wrote:
>>
>> On Mon, Oct 18, 2021 at 3:52 PM Richard Smith wrote:
>> >
>> > On Mon, 18 Oct 2021 at 12:48, Aaron Ballman
>> > wrote:
>> >>
>> >> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith
>> >> wrote:
>> >> >
>> >> > On Sun, 17 Oct 2021 at 04:58, Aaron Ballman via cfe-commits
>> >> > 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 ,
>> >> >> /// integer values.
>> >> >> static void
>> >> >> EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
>> >> >> Token ,
>> >> >> IdentifierInfo *II,
>> >> >> -Preprocessor ,
>> >> >> +Preprocessor , bool
>> >> >> ExpandArgs,
>> >> >> llvm::function_ref<
>> >> >>int(Token ,
>> >> >>bool
>> >> >> )> 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
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, 18 Oct 2021 at 12:56, Aaron Ballman wrote:
> On Mon, Oct 18, 2021 at 3:52 PM Richard Smith
> wrote:
> >
> > On Mon, 18 Oct 2021 at 12:48, Aaron Ballman
> wrote:
> >>
> >> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith
> wrote:
> >> >
> >> > On Sun, 17 Oct 2021 at 04:58, 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 ,
> >> >> /// integer values.
> >> >> static void
> EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
> >> >> Token ,
> IdentifierInfo *II,
> >> >> -Preprocessor ,
> >> >> +Preprocessor , bool
> ExpandArgs,
> >> >> llvm::function_ref<
> >> >>int(Token ,
> >> >>bool
> )> 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
Digging a bit more: https://godbolt.org/z/4Yo578b8n
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, Oct 18, 2021 at 3:52 PM Richard Smith wrote:
>
> On Mon, 18 Oct 2021 at 12:48, Aaron Ballman wrote:
>>
>> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith wrote:
>> >
>> > On Sun, 17 Oct 2021 at 04:58, Aaron Ballman via cfe-commits
>> > 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 ,
>> >> /// integer values.
>> >> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream&
>> >> OS,
>> >> Token , IdentifierInfo
>> >> *II,
>> >> -Preprocessor ,
>> >> +Preprocessor , bool
>> >> ExpandArgs,
>> >> llvm::function_ref<
>> >>int(Token ,
>> >>bool
>> >> )> 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
___
cfe-commits mailing
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, 18 Oct 2021 at 12:48, Aaron Ballman wrote:
> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith
> wrote:
> >
> > On Sun, 17 Oct 2021 at 04:58, 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 ,
> >> /// integer values.
> >> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream&
> OS,
> >> Token , IdentifierInfo
> *II,
> >> -Preprocessor ,
> >> +Preprocessor , bool
> ExpandArgs,
> >> llvm::function_ref<
> >>int(Token ,
> >>bool
> )> 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Mon, Oct 18, 2021 at 3:33 PM Richard Smith wrote:
>
> On Sun, 17 Oct 2021 at 04:58, Aaron Ballman via cfe-commits
> 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 ,
>> /// integer values.
>> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
>> Token , IdentifierInfo *II,
>> -Preprocessor ,
>> +Preprocessor , bool
>> ExpandArgs,
>> llvm::function_ref<
>>int(Token ,
>>bool )>
>> 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
On Sun, 17 Oct 2021 at 04:58, 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 ,
> /// integer values.
> static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
> Token , IdentifierInfo
> *II,
> -Preprocessor ,
> +Preprocessor , bool
> ExpandArgs,
> llvm::function_ref<
>int(Token ,
>bool )>
> 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".
already_lexed:
> switch (Tok.getKind()) {
> @@ -1609,21 +1612,21 @@ void Preprocessor::ExpandBuiltinMacro(Token ) {
> 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 , bool ) -> 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 , bool ) -> 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 , bool ) -> int {
> IdentifierInfo *II =
[clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
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 ,
/// integer values.
static void EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
Token , IdentifierInfo *II,
-Preprocessor ,
+Preprocessor , bool ExpandArgs,
llvm::function_ref<
int(Token ,
bool )> 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 ) {
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 , bool ) -> 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 , bool ) -> 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 , bool ) -> int {
IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
diag::err_feature_check_malformed);
@@ -1675,12 +1678,12 @@ void Preprocessor::ExpandBuiltinMacro(Token ) {
}
});
} else if (II == Ident__is_identifier) {
-EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
[](Token , bool ) -> int {
return Tok.is(tok::identifier);
});
} else if (II == Ident__has_attribute) {
-EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
+
