<div dir="ltr"><div>Hi Everyone, especially Reid, Nico and Howard</div><div> </div><div>I've attached a revision of win32 patch I submitted earlier for review. The earlier patch was requested to be split up into smaller pieces. The other parts of the patch have now been reviewed and applied. This is the remaining piece for review.</div>
<p>A while back Nico made a few changes that I've summarised below:</p><div>__config introduced _LIBCPP_MSVCRT when WIN32 is defined, effectively making them synonymous and also _LIBCPP_MSVC that becomes synonymous with MS's compiler (I think).<br>
</div><div>The following include guards were also changed from/to as shown below.</div><p>File Includes Previous Condition Current Condition <br>__locale locale_win32.h #if _WIN32 #if _LIBCPP_MSVCRT<br>
locale locale_win32.h #if _WIN32 #if _LIBCPP_MSVCRT !note! else #include <nl_types.h><br>cstdlib locale_win32.h #if _MSC_VER #if _LIBCPP_MSVCRT !note! use was previously inconsistent with the above line.<br>
cmath math_win32.h #if _MSC_VER #if _LIBCPP_MSVCRT<br>cwchar win32\support.h #if _WIN32 #if _LIBCPP_MSVCRT<br>limits limits_win32.h #if _MSC_VER #if _LIBCPP_MSVCRT</p>
<div>And also cstdio, cmath, and cwchar had some "using" changes<br>from: #if _MSC_VER using ::X; #endif<br>to: #if _LIBCPP_MSVCRT using ::X; #endif<br></div><div><p>As can be seen by the table above, the effect of the earlier change means that some files that might have previously only compiled #if _MSC_VER are now instead getting compiled in the more general _WIN32 mode, letting clang etc. in which is a situation they aren't prepared for.</p>
<p>I wonder if at that time those files were written _MSC_VER might have been meant "for Microsoft's compiler only" but I don't know, But in any case, I think this is what broke the mingw/gnu toolset that I use so it no longer compiled "out of the box".</p>
</div><div>I don't know the intent of the changes above and without that I personally find the new macro's more confusing than just using #if _WIN32 etc. here, but I don't feel confident enough to argue that case.</div>
<div> </div><div>So instead I have just attached a patch that is intended to get the gnu/mingw build I use (linked in a prior email) compiling again with Nico's above changes as they are and hopefully shed some more light on the general situation.</div>
<div> </div><div>The effect of Nico's changes means all of the files named Win32 compile on win32 in all occasions. I perhaps find that fact a clearer mental model than before so it might be better, but I've never quite understood the model previously anyway. I'm just trying to fix things the way they are so they'll can compile again. I am wondering if the include situation might help support the clang-cl compiler mode when that gets a work out. But right now that isn't my main focus here.</div>
<div> </div><div>Note libcxx does not compile without error with Visual Studio but it never has done so or at least not in a long while, to my knowledge. I've tried test compiling libcxx with VS2013 anyway too to try the _MSC_VER/MSVC paths and double check my work. My patch improves the situation here a little error/warning count wise consequently, but that isn't the main aim of my patch.</div>
<div> </div><div>I'm just trying to get libcxx back to where it compiles for mingw/gnu "out of the box".</div><div> </div><div>Here's a summary of the files my patch touches to achieve that, and roughly why but see the patch content itself for full details of the changes:</div>
<div> </div><div>algorithm</div><div>Added an include for win32\support.h #if _LIBCPP_MSVCRT as algorithm now uses _builtins. For Win32, support.h defines.</div><p>__config<br>Added a space to_NOEXCEPT to make the definition with Visual Studio exactly the same. Seems to reduce messages from VS. I can't see what otherwise harmful effect the change should inflict.</p>
<p>cmath (special attention Howard)<br>The cos function had LIBCPP_ALWAYS_INLINE and _LIBCPP_INLINE_VISIBILITY.<br>This seems to be the only instance of using these two attributes together in the whole of libcxx. Other similar functions do not have this. I think this one off of instance is a mistake.<br>
Howard please review this cos function change as this as it could effect Apple if it wasn't a mistake and I am wrong to have changed it.<br>I also removed the #else condition guard on ::lgamma/f guard as it caused compilation errors in random that otherwise couldn't reference the function with it guarded.</p>
<div>cstdio<br>Changed guard for snprintf as it was causing compilation errors in VS and Mingw without this, probably as it's a macro.</div><div> </div><div>cstdlib</div><div>Changed to include support.h instead of locale.h as couldn't see the need for locale.h and seems to compile file with the lesser file.</div>
<div> </div><div>support.h<br>Made _Exit a inline function instead of a macro.<br>Rejigged so all inclusion of files is at the beginning of the file.</div><div> </div><div>limits_win32.h</div><div>Rejigged to put includes at the top and added an additional support item CHAR_BIT as it seems libcxx now requires this as of recent libcxx changes.</div>
<div> </div><div>locale_win32.h</div><div>Reverted some more __restrict__ uses back to __restrict and made a function iinline instead of a macro.</div><div> </div><div>I've also made the above .h files system headers as it seemed appropriate but if this is incorrect I'll revert that change.</div>
<div> </div><div>To conclude, if this patch is applied, it should restore libcxx to at least a compiling state for the Mingw build I use.</div><div>I've linked that here for those interested or wishing to try this toolset before and after my patch:</div>
<div><a href="http://sourceforge.net/projects/mingwbuilds/files/host-windows/releases/4.8.1/64-bit/threads-win32/seh/x64-4.8.1-release-win32-seh-rev3.7z/download">http://sourceforge.net/projects/mingwbuilds/files/host-windows/releases/4.8.1/64-bit/threads-win32/seh/x64-4.8.1-release-win32-seh-rev3.7z/download</a></div>
<div> </div><div>Thanks<br></div></div>