[cfe-dev] Keyword warnings in libc++'s type_traits and other headers

[Alp Toker]

On 23/12/2013 00:46, Marshall Clow wrote:
> On Dec 22, 2013, at 1:58 PM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>> On 22/12/2013 21:27, Dimitry Andric wrote:
>>> Hi,
>>>
>>> I ran into a situation where a C++ program was compiled with 
>>> -Wsystem-headers.  When I did this with clang 3.4 or trunk, I got 
>>> the following keyword warnings:
>> [snipped]
>>> include/type_traits:668:8: warning: keyword '__is_unsigned' will be 
>>> made available as an identifier for the remainder of the translation 
>>> unit [-Wkeyword-compat]
>>> struct __is_unsigned : public ___is_unsigned<_Tp> {};
>>>       ^
>>> 9 warnings generated.
>>>
>>> This seems to have been introduced with r196212 in clang by Alp 
>>> Toker, but it is unfortunate the warning hits libc++. :-)  The cause 
>>> is a bunch of Embarcadero keywords defined in clang's 
>>> lib/Parse/ParseExpr.cpp, which are exactly the same as these 
>>> libc++-internal identifers.
>>>
>>> Is the attached patch acceptable as a workaround?
>>
>> Hi Dimitry,
>>
>> Thanks for looking into this. Your analysis of the situation is 
>> correct and the patch for libc++ is headed in the right direction.
>
> I am strongly against this patch on several levels.
>
> 1) I would like to understand why (and how we came to) have two 
> different sets of type traits implementations with the same name; one 
> in libc++ and the other in the Embarcadero version of clang. Or maybe 
> it’s not just in the Embarcadero version - though they are listed in 
> TokenKinds.def as "Embarcadero Type Traits”

I suspect those trait intrinsics were added to support another C++ 
standard library, the same way certain intrinsics have been added to 
clang to support libc++.

clang is used by many vendors and I'm aware of various commercial and 
free C++ standard libraries we support, both in C++ compliance mode and 
in compatibility mode.

Thankfully most of those vendors are active or responsive contributors 
to LLVM, or as in the case of MSVC there's a strong ongoing push to 
achieve drop-in compatibility from the core clang team.

None of the trait intrinsics are inherently better or worse than others, 
and all are reserved as an implementation detail intended to help clang 
provide full-spectrum C++ support.

>
> 2). I would like to have a discussion about the *desirability* of 
> having these in the compiler; clearly most of these type traits are 
> implementable purely as a library.

It's a perfectly legitimate topic to discuss the merits of library vs. 
intrinsic implementations but that conversation has to be between you 
and the vendors of those other C++ standard library developers.

If you reach a consensus and they ship your proposed changes, we can 
begin a cycle of deprecation until the intrinsics in question are no 
longer in use.

As a maintainer of this code I can state that the intrinsics are not a 
burden at present and have low cost to keep in tree, but if the time 
comes some years down the line that they are no longer in use we'll be 
sure to remove them.

Keep in mind however that some of these type traits are not 
implementable in library form, which may have been the reason intrinsics 
were selected in the first place for any particular C++ type trait.

Furthermore, even when a library version is possible, a thin header 
structure wrapping a quality compiler intrinsic can yield significantly 
efficiency gains. Consider that clang will be able to evaluate certain 
type traits instantly without requiring deserialization of AST nodes 
from pre-compiled headers or modules, sparing costly operations to 
materialize long chains of dependent declarations.

It's my hope that we can coordinate with libc++ on ground-breaking 
features like the one I described instead of nit-picking over reserved 
namespaces.

>
> 3) I *hate* the idea of using “quad underscores” to differentiate 
> names; I spent *two hours* a couple months ago trying to figure out 
> why my code was failing, only to determine that I was calling __xxx 
> instead of ___xxx. (triple underscore vs. double)  If we decide to 
> change libc++ for this, I would recommend renaming __is_void to 
> something like __libcpp_is_void - something visually different.

Underscores are a matter of style and it's your baby. Full credit to 
Dimitry for proposing the initial fix and I'm sure he'll be responsive 
to your review comments.

I agree that quad-underscores are less than ideal. Something like 
__libcpp_* is easier on the eye and will help you avoid conflicts with 
any compiler's intrinsics.

Keep in mind that where clang warns today, any other compiler already 
issues a hard error so we're making the effort to provide due notice. 
That said, we're starting to look to remove GNU header compatibility 
hacks from the parser and it was never the plan that libc++ would begin 
to use them.

tl;dr: There's no linkage or visibility so no negative impact in doing a 
search-and-replace to make your headers parsable without hacks?

Alp.



>
> — Marshall
>

-- 
http://www.nuanti.com
the browser experts