[patch] libcxx win32 support

G M gmisocpp at gmail.com
Tue Aug 27 07:55:52 PDT 2013


Hi Everyone, especially Reid, Nico and Howard

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.

A while back Nico made a few changes that I've summarised below:
__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).
The following include guards were also changed from/to as shown below.

File      Includes                 Previous Condition  Current
Condition
__locale  locale_win32.h           #if _WIN32          #if _LIBCPP_MSVCRT
locale    locale_win32.h           #if _WIN32          #if _LIBCPP_MSVCRT
!note! else #include <nl_types.h>
cstdlib   locale_win32.h           #if _MSC_VER        #if _LIBCPP_MSVCRT
!note! use was previously inconsistent with the above line.
cmath     math_win32.h             #if _MSC_VER        #if _LIBCPP_MSVCRT
cwchar    win32\support.h          #if _WIN32          #if _LIBCPP_MSVCRT
limits    limits_win32.h           #if _MSC_VER        #if _LIBCPP_MSVCRT
And also  cstdio, cmath, and cwchar had some "using" changes
from: #if _MSC_VER using ::X; #endif
to:   #if _LIBCPP_MSVCRT using ::X; #endif

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.

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".
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.

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.

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.

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.

I'm just trying to get libcxx back to where it compiles for mingw/gnu "out
of the box".

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:

algorithm
Added an include for  win32\support.h  #if _LIBCPP_MSVCRT as algorithm now
uses _builtins. For Win32, support.h defines.

__config
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.

cmath (special attention Howard)
The cos function had LIBCPP_ALWAYS_INLINE and _LIBCPP_INLINE_VISIBILITY.
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.
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.
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.
cstdio
Changed guard for snprintf as it was causing compilation errors in VS and
Mingw without this, probably as it's a macro.

cstdlib
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.

support.h
Made _Exit a inline function instead of a macro.
Rejigged so all inclusion of files is at the beginning of the file.

limits_win32.h
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.

locale_win32.h
Reverted some more __restrict__ uses back to __restrict and made a function
iinline instead of a macro.

I've also made the above .h files system headers as it seemed appropriate
but if this is incorrect I'll revert that change.

To conclude, if this patch is applied, it should restore libcxx to at least
a compiling state for the Mingw build I use.
I've linked that here for those interested or wishing to try this toolset
before and after my patch:
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

Thanks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130828/a7f64475/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libcxx.diff
Type: application/octet-stream
Size: 11607 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130828/a7f64475/attachment.obj>


More information about the cfe-commits mailing list