[cfe-dev] [PATCH] Libc++ Windows fixes

[Ruben Van Boxem]

2011/9/23 Ruben Van Boxem <vanboxem.ruben at gmail.com>

> 2011/9/22 Ruben Van Boxem <vanboxem.ruben at gmail.com>
>
>> 2011/9/22 Ruben Van Boxem <vanboxem.ruben at gmail.com>
>>
>>> 2011/9/22 Eli Friedman <eli.friedman at gmail.com>
>>>
>>>> On Thu, Sep 22, 2011 at 12:35 PM, Ruben Van Boxem
>>>> <vanboxem.ruben at gmail.com> wrote:
>>>> > 2011/9/22 Howard Hinnant <hhinnant at apple.com>
>>>> >>
>>>> >> Patch committed revision 140328.
>>>> >
>>>> > Thanks!
>>>> >
>>>> > Now that I've got your attention, on to more... involved... matters
>>>> :-)
>>>> >
>>>> > In order to support libc++'s current implementation using
>>>> > cat[gets|open|close] and nl_types.h (which I ignored for now), I would
>>>> need
>>>> > to pull in some external code, namely mingw's libcatgets and a
>>>> supporting
>>>> > iconv library. These can all go under support/win32 and leave all
>>>> other
>>>> > platforms unchanged of course, and leave the rest of libc++ as
>>>> coherent as
>>>> > possible on all platforms. I understand Howard has an acceptable
>>>> aversion to
>>>> > including too much foreign code in libc++, and hope you can see the
>>>> need for
>>>> > this here.
>>>> >
>>>> > There are some issues though:
>>>> >  - libcatgets is licensed under GPLv2. If I can go through with this,
>>>> I can
>>>> > ask/plead/beg the original author to allow a license change or ask him
>>>> to
>>>> > make an exception for libc++. I know of no existing alternative.
>>>> >  - for the iconv part, GNU libiconv is of course out of the question.
>>>> There
>>>> > are two projects of interest: APR-iconv (Apache License 2.0) and
>>>> win-iconv
>>>> > (BSD 2-clause license), the latter being tiny but primitive. The first
>>>> is a
>>>> > truly first class implementation as far as I can see, and with minor
>>>> > refactoring could be included in the libc++ Windows build.
>>>> >  - I am not an Open Source license lawyer, and do not know what would
>>>> be
>>>> > acceptable for libc++ in this regard.
>>>> >
>>>> > I want to ask what's "best", before starting with the implementation.
>>>> The
>>>> > only real alternative is writing all this (ie catgets with supporting
>>>> iconv
>>>> > code) from scratch, which I don't see myself doing in the near to
>>>> distant
>>>> > future. Another way is to completely replace libc++ parts that
>>>> use/rely on
>>>> > these API's, but this would make libc++ a lot more assymetric across
>>>> > platforms. In theory, these support files could help other platforms
>>>> where
>>>> > these API's are missing.
>>>> >
>>>> > Please let me know what you think. Thanks!
>>>>
>>>> MSVC doesn't provide a non-trivial implementation of
>>>> std::messages::do_open etc., so I don't see why it matters if libc++
>>>> does not provide one on Windows.
>>>>
>>>
>>> Hmm, glancing through libstdc++'s headers, it doesn't seem to do much
>>> more :(. Then the question becomes (if the authors/copyright holders of the
>>> support code in question do not want to have it used in win32 libc++),
>>> should I just mimic current behavior of the other "Big 2" (MSVC/GCC) in this
>>> regard? What would I be missing out on then? Is it really only n3290's
>>> 22.4.7?
>>>
>>
>> Implementing stubs was easy, so I just went ahead with it. I can always
>> improve on it later if I want. Attached is another incremental patch.
>>
>> Notes:
>>  - inclue/__bit_reference: Win32 <yvals.h> has a line: "#define _C2 1"
>> which obviously messes up the templates in this file. I added n extra
>> underscore to work around this, and did the same to _C1 for consistency.
>>  - include/locale: include my support header for vasprintf, make the
>> std::message functions stubs for Windows, mirroring other C++ Standard
>> Library implementations on Windows.
>>  - include/support/win32/support.h: add two missing functions and
>> implement them naively in function of not-missing functions.
>>  - src/locale.cpp: exclude langinfo.h inclusion for Windows. More work is
>> needed here later to provide an alternative.
>>  - src/support/win32/suppport.cpp: implementation.
>>
>> Please comment on this if something looks weird or wrong.
>>
>> Clang failed me (http://llvm.org/bugs/show_bug.cgi?id=10989) so, I'll
>> need to resort to GCC and its unhelpful error messages in the meantime.
>>
>> Ruben
>>
>
> This time with patch. Note that the 32% is a rough estimate, mostly meant
> for my own encouragement.
>
> PS: thanks for the quick patching for the assembler error.
>

I have reached 100%, that is, linking Clang object files fails horribly, and
GCC can't inline var_args functions, but every source file builds with GCC
and Clang apart from these non-libc++ related issues.

The patch explained:
 - include/__bit_reference: Win32 <yvals.h> has a line: "#define _C2 1"
which obviously messes up the templates in this file. I added an extra
underscore to work around this, and did the same to _C1 for consistency.
 - include/locale: include my support header for vasprintf, make the
std::message functions stubs for Windows, mirroring other C++ Standard
Library implementations on Windows.
 - include/support/win32/support.h: add two missing functions, and define a
swprintf for MINGX32. Technically, VS2003 also has a bad declaration, but as
it doesn't support a lot more that isn't really relevant for libc++.
 - include/suppport/win32/locale.h:
   - add missing defines for prefixed versions of functions.
   - Redo newlocale to accept a third parameter. Functionality should be ok,
as the third parameter is always 0 in libc++.
   - Implement is(w)blank naively, which are missing from msvcrt.
   - define LC_ALL_MASK
   - remove the nl_types enum definition, as it is now unused.
 - src/locale.cpp: exclude langinfo.h inclusion for Windows, instead
including <locale.h>. msvcrt does not provide a C99 compliant lconv
implementation, so for Windows, some int_* prefixed struct members are not
present. I used the non-prefixed ones instead. Technically incorrect, but
should work OK in most cases.
 - src/string.cpp: include Win32 support header.
 - src/thread.cpp: omit inclusion of sys/sysctl.h on Windows.
 - src/support/win32/suppport.cpp:
   - fix vasprintf.
   - implement the two new functions naively in function of not-missing
functions. These are slow and wasteful, but should work correctly at least.

I think the changes to core libc++ are minor, considering everything. Please
comment if you think otherwise. On the other hand, please commit when you're
OK with it.

This leads to the next problem(s):
 - libc++ dll needs exports a la
__cdeclspec(dllexport)/__cdeclspec(dllimport). See LLVM/Clang DLL
discussions. Maybe using the same idea is possible.
 - GCC (4.6) cannot inline vararg functions, thus is cannot fully compile
libc++. On the other hand, Clang fails linking the dll due to a bunch of
undefined references, among which cxxabi stuff, and C std library stuff.
I'll investigate.
 - MSVC has trouble parsing (I think) "inline namespace":
[  4%] Building CXX object lib/CMakeFiles/cxx.dir/__/src/algorithm.cpp.obj
algorithm.cpp
M:\Development\Source\libc++\include\__config(14) : warning C4068: unknown
pragma
M:\Development\Source\libc++\include\cstddef(46) : warning C4068: unknown
pragma
M:\Development\Source\libc++\include\cstddef(50) : error C2143: syntax error
: missing ';' before 'using'
M:\Development\Source\libc++\include\cstddef(50) : error C4430: missing type
specifier - int assumed. Note: C++ does not support default-int
M:\Development\Source\libc++\include\cstddef(93) : error C2143: syntax error
: missing ';' before 'namespace'
M:\Development\Source\libc++\include\cstddef(93) : error C4430: missing type
specifier - int assumed. Note: C++ does not support default-int

Ruben
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110923/8aa0e2f3/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: windows.patch
Type: application/octet-stream
Size: 15059 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110923/8aa0e2f3/attachment.obj>