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

[G M]

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/20140107/d793b3fd/attachment.html>
-------------- next part --------------
Index: __config
===================================================================
--- __config	(revision 198582)
+++ __config	(working copy)
@@ -434,7 +434,7 @@
 #define _ALIGNAS(x) __declspec(align(x))
 #define _LIBCPP_HAS_NO_VARIADICS
 
-#define _NOEXCEPT throw()
+#define _NOEXCEPT throw ()
 #define _NOEXCEPT_(x)
 #define _NOEXCEPT_OR_FALSE(x) false
 
Index: __config
===================================================================
--- __config	(revision 198593)
+++ __config	(working copy)
@@ -434,7 +434,8 @@
 #define _ALIGNAS(x) __declspec(align(x))
 #define _LIBCPP_HAS_NO_VARIADICS
 
-#define _NOEXCEPT throw()
+// MSVC's yvals.h defines _NOEXCEPT. The definition here should match exactly.
+#define _NOEXCEPT throw ()
 #define _NOEXCEPT_(x)
 #define _NOEXCEPT_OR_FALSE(x) false