[PATCH] [libcxx] Distinguish between MSVC, MSVCRT, and Windows-specific code

G M gmisocpp at gmail.com
Thu Aug 8 18:46:43 PDT 2013


Hello Everyone

The current Windows build of libcxx does not compile right now (at least
for me) but it used to.

I'm relatively new to clang and inexperienced with it but I used to be able
to build libcxx on Windows 7 64 bit, straight from svn, using either
clang++ svn or gcc 4.8.1 / mingw, with no changes required to libcxx or
it's __config. It just built on Windows "out of the box".

Since pulling Nico Rieck's patch (187593) that introduced the
_LIBCPP_MSVCRT macros, compiling under the same scenario yields compile
errors.

I have put together a patch that makes it compile for me again on Windows.
I'd appreciate any help to get it good enough to be applied ASAP as right
now the current situation appears that libcxx won't compile at all for
Windows using clang++ or g++.

Hopefully this isn't some peculiarity to my setup. Sorry in advance if it
is.

My environment is:

Windows 7 64 bit.
I'm using g++ from here
http://sourceforge.net/projects/mingwbuilds/files/host-windows/releases/4.8.1/64-bit/threads-win32/seh/
File x64-4.8.1-release-win32-seh-rev3.7z
I have the latest clang from svn built with the above g++.

If I understand Nico's patch correctly, it systematically changed code that
was like this:

#if _MSC_VER
#include "support\win32\win32_math.h"
#endif

to code like this:

#if _LIBCPP_MSVCRT
#include "support\win32\win32_math.h"
#endif

I don't know that this is wrong, but given that __config does not, nor ever
has defined _MSC_VER before, and nor does clang++ appear to define it by
default, Nico's change to make __config always define _LIBCPP_MSVCRT
whenever _WIN32 is defined, has the effect of causing more of libcxx's code
to be compiled by more compilers than was the case prior to his patch and I
think that's what is breaking the Windows build.

Using the above code from libcxx's <cmath> as an example, I think
win32_math.h. is now being opened by clang++ and g++ etc. when they
previously would have stayed out of it because _MSC_VER was the guard and
that wasn't defined by them; but _LIBCPP_MSVCRT is now the guard and is now
always defined for _Win32 in __config.

win32_math.h, win32_liimits.h, support.h are all affected in this way. This
seems to be the extent of problems resulting from the change.

If I am correct that Nico's patch does now cause these files to be opened
by clang++ and g++ etc., when they did not open them before - and if that
is not a mistake that should be reverted - then the problem appears to be
that Nico's changes just didn't go far enough. Since the contents of these
files appear to not be wholly compatible with clang++. The files appear to
need appropriate guards inside them now so that clang++ etc. only attempt
to compile as much of the above files as they are compatible with. (More on
some of the actual code incompatibilities below).

The files appear to have code for _MSC_VER, where some of that means "not
for clang, but for ms's compiler cl.exe only.". Other code might be for
clang++ when it's compiling against Mingw etc.

My attached patch attempts to provide appropriate guards (as best I can
test) to the above mentioned files and that is my proposed solution for
now. It does not undo any of Nico's patch (at least intentionally).

clang's new clang-cl.exe driver apears to NOT define _MSC_VER by default
either. If any clang variant or g++ does define _MSC_VER (I don't mean MS's
cl.exe which I know does), I would like someone let me know, and under what
circumstances, and how I can recreate that. I might then try it, but any
outcome of that should be for an additional patch IMHO.

My patch in more detail: it attempts to re-enable the situation where
libcxx compiles on Windows again "out of the box" with clang++ and g++. It
touches these files:

* win32_math.h - changed to guard code as described above that shouldn't be
compiled by "classic" clang++.

* limits/win32_limits.h - similar guards.
I've put guards to protect clang from compiling code like the following:
#define __FLT_MANT_DIG__   FLT_MANT_DIG
because it appears __FLT_MANT_DIG__ is a compiler intrinisc for clang etc.
so it seems this code should only get compiled if it's actually Microsoft's
compiler compiling it, not clang pretending to be Microsoft's compiler.

* support.h
I've commented out the strtof functions as they seem to conflict with at
least gnu headers and those that come with MS's cl.exe.

* __config
As a drive by fix, I added _LIBCPP_HAS_NO_DELETED_FUNCTIONS to __config in
the #elif defined(_LIBCPP_MSVC)
as it seemed that it should be there.

Slightly related:

* <cmath>
I couldn't compile cmath on Windows because of new issues with lgamma. I
changed a couple of lines that seems to fix things for me but this change
needs to be examined carefully as it may be wrong for other platforms. It
enables successful compilation on Windows though where without this change
it wouldn't. It affects the lines around these statements:
using ::lgamma;
using ::lgammaf;
Without my changes, algorithm and random etc. couldn't find these functions
in libcxx.
This last part, relating to gamma, is very small, but could be lifted out
as a different patch as it's a logically separate problem. But both parts
of the
patch are needed to get a clean compile on Windows so that's the rationale
for bundling them.

It's a shame there appears to be no build-bot that picks up this
configuration up as it's been "broken" for a few days. If there is such a
built bot or if someone has a plan to add one for this configuration, I'd
appreciate someone telling me a bit about it.

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


More information about the cfe-commits mailing list