<div dir="ltr"><div dir="auto">That all makes sense -- especially the bits about the dates needing to be different.</div><div dir="auto"><br></div><div>But, even with all that, I'm not sure why we shouldn't implement both __has_cpp_attribute AND __has_c_attribute in C++ mode?</div><div><br></div><div>The subset of C attributes which retain their C-defined semantics in C++ (which, hopefully, is all of them, but doesn't need to be) should return the appropriate C-standard date with which the (C++ mode) implementation is compatible. That is, in -std=c++11, we may have __has_cpp_attribute(deprecated) == 201309, while __has_c_attribute(deprecated) == 2020XXYY.</div><div dir="auto"><br></div><div dir="auto">I'd hope/expect that whichever version of the C++ standard updates its baseline to C2x will adopt this feature "automatically", and if it doesn't, that it'll be included explicitly. But, ISTM we probably don't need to wait for that to occur in order to implement that behavior in clang?</div><div dir="auto"><br></div><div dir="auto"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 25, 2019, 7:07 PM Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Nov 25, 2019 at 5:59 PM James Y Knight <<a href="mailto:jyknight@google.com" rel="noreferrer" target="_blank">jyknight@google.com</a>> wrote:<br>
><br>
> Isn't this unnecessarily annoying to users? You have the same syntax to use the attributes, and the attributes are expected to be compatible when named the same way, but you can't use the same #if conditional to check for availability, when writing a header intended to work in both modes?<br>
<br>
Yes, I do think it's needlessly annoying to users.<br>
<br>
WG21 adopted __has_cpp_attribute for C++2a based on the existing Clang<br>
implementation. WG14 is looking to add a similar feature testing<br>
macro, but cannot use __has_cpp_attribute due to the name and so I<br>
proposed __has_c_attribute for parity with __has_cpp_attribute and<br>
implemented it in Clang. After a lot of discussions with users,<br>
committee members, and other implementers, I realized this is silly:<br>
we could have a single macro that handles both C and C++ depending on<br>
the language mode being compiled for. Because of that, as a liaison I<br>
filed an NB comment with WG21 to have them reconsider the name<br>
__has_cpp_attribute before it shipped in a standard so that we could<br>
use a language-neutral name for both languages. In Belfast, EWG<br>
reaffirmed that they want the name __has_cpp_attribute. They found my<br>
arguments to be unpersuasive and didn't think it would be possible for<br>
the semantics of attributes to align between the languages, or to<br>
commit to such a guarantee (despite WG14 having a chartered mandate to<br>
be compatible with features adopted from C++).<br>
<br>
Rather than leaving C out in the cold because EWG came to the decision<br>
they did, we're kind of stuck with __has_c_attribute.<br>
<br>
One problem this does solve is that __has_cpp_attribute and<br>
__has_c_attribute both return a numeric value that is date-like for<br>
standard attributes, but those standard attributes are adopted into<br>
different standards at different times. Because returned values are<br>
different, it makes use harder when dealing with attribute<br>
modifications over time. This isn't impossible to solve, we could<br>
introduce macros for the return values and base their values on which<br>
language mode is being preprocessed, but we no longer have to.<br>
<br>
~Aaron<br>
<br>
><br>
> On Mon, Nov 25, 2019 at 5:35 PM Aaron Ballman via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" rel="noreferrer" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>><br>
>> Author: Aaron Ballman<br>
>> Date: 2019-11-25T17:35:12-05:00<br>
>> New Revision: d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578<br>
>><br>
>> URL: <a href="https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578" rel="noreferrer noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578</a><br>
>> DIFF: <a href="https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578.diff" rel="noreferrer noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578.diff</a><br>
>><br>
>> LOG: Disallow use of __has_c_attribute in C++ mode.<br>
>><br>
>> __has_cpp_attribute is not available in C mode, and __has_c_attribute<br>
>> should not be available in C++ mode. This also adds a test to<br>
>> demonstrate that we properly handle scoped attribute tokens even in C<br>
>> mode.<br>
>><br>
>> Added:<br>
>>     clang/test/Preprocessor/has_c_attribute.cpp<br>
>><br>
>> Modified:<br>
>>     clang/lib/Lex/PPMacroExpansion.cpp<br>
>>     clang/test/Preprocessor/has_c_attribute.c<br>
>><br>
>> Removed:<br>
>><br>
>><br>
>><br>
>> ################################################################################<br>
>> diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp<br>
>> index e6e00b1c1700..a69c4dbb3a2a 100644<br>
>> --- a/clang/lib/Lex/PPMacroExpansion.cpp<br>
>> +++ b/clang/lib/Lex/PPMacroExpansion.cpp<br>
>> @@ -370,7 +370,11 @@ void Preprocessor::RegisterBuiltinMacros() {<br>
>>    Ident__has_extension    = RegisterBuiltinMacro(*this, "__has_extension");<br>
>>    Ident__has_builtin      = RegisterBuiltinMacro(*this, "__has_builtin");<br>
>>    Ident__has_attribute    = RegisterBuiltinMacro(*this, "__has_attribute");<br>
>> -  Ident__has_c_attribute  = RegisterBuiltinMacro(*this, "__has_c_attribute");<br>
>> +  if (!LangOpts.CPlusPlus)<br>
>> +    Ident__has_c_attribute = RegisterBuiltinMacro(*this, "__has_c_attribute");<br>
>> +  else<br>
>> +    Ident__has_c_attribute = nullptr;<br>
>> +<br>
>>    Ident__has_declspec = RegisterBuiltinMacro(*this, "__has_declspec_attribute");<br>
>>    Ident__has_include      = RegisterBuiltinMacro(*this, "__has_include");<br>
>>    Ident__has_include_next = RegisterBuiltinMacro(*this, "__has_include_next");<br>
>><br>
>> diff  --git a/clang/test/Preprocessor/has_c_attribute.c b/clang/test/Preprocessor/has_c_attribute.c<br>
>> index 843a67a2646c..f8b0b364faa5 100644<br>
>> --- a/clang/test/Preprocessor/has_c_attribute.c<br>
>> +++ b/clang/test/Preprocessor/has_c_attribute.c<br>
>> @@ -1,4 +1,5 @@<br>
>>  // RUN: %clang_cc1 -fdouble-square-bracket-attributes -std=c11 -E %s -o - | FileCheck %s<br>
>> +// RUN: %clang_cc1 -std=c2x -E %s -o - | FileCheck %s<br>
>><br>
>>  // CHECK: has_fallthrough<br>
>>  #if __has_c_attribute(fallthrough)<br>
>> @@ -14,3 +15,8 @@<br>
>>  #if __has_c_attribute(__nodiscard__)<br>
>>    int has_nodiscard_underscore();<br>
>>  #endif<br>
>> +<br>
>> +// CHECK: has_clang_annotate<br>
>> +#if __has_c_attribute(clang::annotate)<br>
>> +  int has_clang_annotate();<br>
>> +#endif<br>
>><br>
>> diff  --git a/clang/test/Preprocessor/has_c_attribute.cpp b/clang/test/Preprocessor/has_c_attribute.cpp<br>
>> new file mode 100644<br>
>> index 000000000000..0bde73067178<br>
>> --- /dev/null<br>
>> +++ b/clang/test/Preprocessor/has_c_attribute.cpp<br>
>> @@ -0,0 +1,8 @@<br>
>> +// RUN: %clang_cc1 -std=c++11 %s -verify<br>
>> +<br>
>> +#if __has_c_attribute(fallthrough) // expected-error {{function-like macro '__has_c_attribute' is not defined}}<br>
>> +#endif<br>
>> +<br>
>> +#if __has_c_attribute(gnu::transparent_union) // expected-error {{function-like macro '__has_c_attribute' is not defined}}<br>
>> +#endif<br>
>> +<br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org" rel="noreferrer" target="_blank">cfe-commits@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>