r197995 - Don't reserve __builtin_types_compatible_p as a C++ keyword

[Chandler Carruth]

(Sorry for missing the mailing list in my first reply -- it wasn't
intentional)

On Tue, Dec 24, 2013 at 10:06 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 25/12/2013 02:27, Chandler Carruth wrote:
>
>  On Tue, Dec 24, 2013 at 8:47 PM, Alp Toker <alp at nuanti.com <mailto:
>> alp at nuanti.com>> wrote:
>>
>>     Author: alp
>>     Date: Tue Dec 24 19:47:02 2013
>>     New Revision: 197995
>>
>>     URL: http://llvm.org/viewvc/llvm-project?rev=197995&view=rev
>>     Log:
>>     Don't reserve __builtin_types_compatible_p as a C++ keyword
>>
>>     Even g++ considers this a valid C++ identifier and it should only
>>     have been
>>     visible in C mode.
>>
>>
>> But is this desirable behavior? Is it necessary for some codebase? I
>> don't see how. Just because GCC doesn't error doesn't make this a valid C++
>> identifier -- it is a *reserved* C++ identifier at best so I don't know
>> what codebase could use this reasonably.
>>
>> You've not provided any of the actual justification for this change. As a
>> consequence, I somewhat doubt it is "obvious" to you and should probably
>> have had pre-commit review?
>>
>
> Hi Chandler,
>
> Gave a heads up that I'm working post-commit in this area, and wrote the
> type trait handling updates so it's hopefully somewhat obvious to me.
>

I'll defer to Richard or Doug here, but bluntly, *I've* at least not seen
nearly enough from you for this kind of commit to be post-commit reviewed.


>
> From a practical point of view any future type trait feature checks will
> make use of the KEY field. The 54 other clang traits are correct and this
> one was a leftover from 2007 before the KEY system was done.
>

But what benefit is derived from your change? I don't see any.


>
> From a language standards point of view, clearly we both agree that it's
> silly for user code to exist in the __* namespace.
>
> However if we're not providing something useful at
> __builtin_types_compatible_p() we might as well leave the identifier
> available in case they try to cook up a replacement in C++ with constexpr.


I don't at all understand this rationale. No library should *ever* be using
'__builtin_'.

So, to be clear, you do not have any specific codebase or functional need
motivating this change?


>>     Also drop the associated low-value diagnostic.
>>
>>
>> I don't know why you think this is a low-value diagnostic. But that goes
>> back to my original point -- why is any aspect of this change desirable?
>>
>> Before, there was one fewer token-level difference between C and C++, and
>> people compiling C code in C++ mode got a helpful diagnostic if it actually
>> used this C extension. This to me seems strictly better than trying to
>> maximize the set of reserved identifiers -- worse the ones starting with
>> "__builtin_" which seems exceptionally unlikely to be used even in a
>> standard library -- which are allowed as non-tokens...
>>
>
> I actually agree with you on ideology here as you'll have gathered from my
> posts on-list.
>

Sorry for being off list. I've not gotten any strong sense of your ideology
here.


>
> This isn't part of an attempt the minimize the list of private keywords,
> in fact if anything it's a concession before I propose exactly the
> opposite, cracking down on this more strictly.
>
> In that kind of stricter setup we can warn not just on this but also any
> other type trait even if it's disabled in the current language mode, but
> first it'll be expedient to get some buy-in :-)


I really don't have any idea what you're saying here. This sounds like you
are alluding a really significant new feature / functionality / design for
Clang's compiler-provided diagnostics, and I've not seen anything that lay
out the design you have in mind (your vague comments here don't help,
sorry). Maybe I've just missed it over the holiday season and you can point
me to the thread or proposal you've sent out that outlines the end state
you're working toward? (If there indeed hasn't been one, I think you should
work on that first rather than these types of changes. I would send it in
its own RFC thread to cfe-dev, not on this thread.)

But I can't be certain.

Can you actually address my concern? If you agree with my stance, then why
is this the right incremental step? I don't see how this makes anything
better, and it has the potential to make Clang less strict and informative
than it used to be.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131224/ad47eda2/attachment.html>