libcxx with Visual C++ : don't redefine _NOEXCEPT when already defined

[Yaron Keren]

Hi,

I am using the clang-format plug in for Visual Studio from
http://llvm.org/builds/
It reformats:

#  define _NOEXCEPT throw ()

 into

#define _NOEXCEPT throw()

removing all extra whitespaces.

A comment is certainly helpful.

The compiler catching redefinition mismatch is indeed a good thing since
I'm not sure what __config should do if the definitions don't match. It
can't just redefine _NOEXCEPT since the client may depend upon the Visual
C++ definition.

The best solution would be to replace _NOEXCEPT with _LIBCXX_NOEXCEPT so
there won't be a macro name clash with Visual C++. This is a simple change
although it will change many files.
libstdc++ library uses  _GLIBCXX_NOEXCEPT for the purpose

 Is such a change OK?
if not I'd say your patch is the next best alternative.

Yaron



2014/1/6 G M <gmisocpp at gmail.com>

> Hi Yaron
>
>
> On Tue, Jan 7, 2014 at 12:14 AM, Yaron Keren <yaron.keren at gmail.com>wrote:
>
>> Hi G M,
>>
>> This patch will solve the redefinition problem (since it's identical
>> definition) but may get lost if someone manually changes back the format to
>> be consistent with the rest of libcxx or just clang-formats the file
>> without realizing the whitespace is important.
>>
>> Yaron
>>
>
> I haven't used clang format, but I'm not aware that clang format would
> make such a change? Why would it? Have you seen it do this?
> I'm leaning to worrying  about that if it happens unless you have a better
> suggestion.
>
> If clang-format does make such changes then surely it would still do that
> if it were wrapped in a #ifndef block.
>
> And we have no guarantee that the #defines will be what we want.
>
> By always #define 'ing it the compiler will tell us if it doesn't match.
>
> If we don't define it at all, we can't be sure yvals.h will be included by
> the time we need _NOEXCEPT. The only other option I can think of is to
> include yvals.h in __config. That may actually be necessary soon, but I'd
> rather do that when we need it and not just for this problem.
>
> I think a comment might be helpful though. I've added one in this revision.
>
> If this reasoning doesn't persuade you this patch is better, I'm ok to go
> with your patch instead for now. Let me know what you think or if you have
> another idea.
>
>
>>
>>
>>
>> 2014/1/6 G M <gmisocpp at gmail.com>
>>
>>> Hi everyone (sorry for the late reply, I've been on holiday)
>>>
>>> Regarding the  _NOEXCEPT macro and it's interaction with MSVC's version
>>> of it, mentioned by yaron here:
>>>
>>> On Fri, Dec 27, 2013 at 1:52 AM, Yaron Keren <yaron.keren at gmail.com>wrote:
>>>
>>>> With Visual C++ __config redefines _NOEXCEPT resulting in a warning:
>>>>
>>>> ibcxx\include\__config(434) : warning C4005: '_NOEXCEPT' : macro
>>>> redefinition
>>>>         C:\Program Files (x86)\Microsoft Visual Studio
>>>> 12.0\VC\INCLUDE\yvals.h(25) : see previous definition of '_NOEXCEPT'
>>>>
>>>>  I checked and at least Visual C++ 2012, 2013 define _NOEXCEPT in
>>>> yvals.h.
>>>>
>>>> This patch #ifndef _NOEXCEPT to avoid the warning.
>>>>
>>>> Alternative solution could be to remove it completely, as Visual C++
>>>> seems to supply it. Maybe it was for an older version of Visual C++.
>>>>
>>>> Yaron
>>>>
>>>>
>>> I actually submitted a "fix" for this myself sometime ago but it seems
>>> it got "lost".
>>>
>>> MSVC's yvals.h (or something like that) defines _NOEXCEPT as the same
>>> thing except for spacing.
>>>
>>> My previously submitted solution which I'm attaching here again, is just
>>> to change libcxx's definition to exactly match (i.e. the white space) MS's
>>> definition that MS's yvals.h uses.
>>>
>>> This stops MSVC from complaining about duplicate definitions when
>>> yvals.h is included after __config and effectively removes 100's of
>>> warnings for the msvc build of libcxx.
>>>
>>> I can't see any risk to libcxx here on other platforms.
>>>
>>> There weren't any objections when I submitted this patch before, it was
>>> just forgotten. Any objections to this patch this time?
>>>
>>> Thanks
>>> PS As a suggestion: It'd be great if the _NOEXCEPT_ macro (as opposed to
>>> the one without a trailing _) in libcxx's __config header had a different
>>> name. It's too close to _NOEXCEPT and an easy mistake make. MS use
>>> _NOEXCEPT_OP instead for _NOEXCEPT_ which I think that's much clearer. A
>>> mechanical tool could perhaps be used to make this change? I imagine
>>> there's no will to do this though. Just a suggestion anyway.
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140106/da351420/attachment.html>