Libc++ Windows fixes

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.

-Eli

2011/9/22 Eli Friedman <eli.friedman@gmail.com>

2011/9/22 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/22 Eli Friedman <eli.friedman@gmail.com>

2011/9/22 Howard Hinnant <hhinnant@apple.com>

Patch committed revision 140328.

Thanks!

Now that I’ve got your attention, on to more… involved… matters :slight_smile:

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

2011/9/22 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/22 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/22 Eli Friedman <eli.friedman@gmail.com>

2011/9/22 Howard Hinnant <hhinnant@apple.com>

Patch committed revision 140328.

Thanks!

Now that I’ve got your attention, on to more… involved… matters :slight_smile:

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.

32percent.patch (8.61 KB)

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

2011/9/22 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/22 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/22 Eli Friedman <eli.friedman@gmail.com>

2011/9/22 Howard Hinnant <hhinnant@apple.com>

Patch committed revision 140328.

Thanks!

Now that I’ve got your attention, on to more… involved… matters :slight_smile:

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

windows.patch (14.7 KB)

   - Redo newlocale to accept a third parameter. Functionality should be ok, as the third parameter is always 0 in libc++.

Don't expect this to remain the same in the future. My brief profiling of libc++ showed that it's calling newlocale() far more than it should - 8 times to construct a single std::locale object. I'd expect that it should be calling newlocale() once here, and maybe duplocale() a few times. Calling newlocale() with a NULL third parameter is the most expensive of all of the locale-related functions that libc++ makes, so reducing this is something I plan on investigating.

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

Not sure about the C library stuff. I seem to recall something weird on Windows there, but I'm not sure what. The ABI stuff is going to be a problem on Windows. In the long term, you probably want to be using Microsoft's RTTI and exception ABIs - the latter especially - but neither is supported by clang or gcc yet. You may be able to use libcxxrt or GNU libc++sup, but then you're using code that won't interoperate with other C++ libraries on Windows nicely...

David

Committed revision 140384.

Howard

Optimizations are certainly welcome. Just a heads up though. There's currently an optimization in there I'd like to keep. The C locale, constructed implicitly as the original global locale, or obtained via locale::classic(), allocates no memory and doesn't call newlocale().

#include <locale>
#include <iostream>

std::size_t memory = 0;
std::size_t alloc = 0;

void* malloc(std::size_t) {return NULL;}
void free(void*) {}

void* operator new(std::size_t s) throw(std::bad_alloc)
{
    memory += s;
    ++alloc;
    return malloc(s);
}

void operator delete(void* p) throw()
{
    --alloc;
    free(p);
}

void memuse()
{
    std::cout << "memory = " << memory << '\n';
    std::cout << "alloc = " << alloc << '\n';
}

int main()
{
    std::locale loc;
    loc = std::locale::classic();
    memuse();
}

memory = 0
alloc = 0

But I agree that the *named* locale facility could use optimization, even when the name is "C". But this is a very tricky area. Please post non-trivial code for review prior to committing.

Howard

Absolutely. The global "C" locale, and a named "C" locale are different in xlocale, since the locale_t contains some state used by the various multibyte functions.

However...

Does the classic locale refer to the "C" locale, or the current global locale? If it's the former, then it DOES need to be allocating memory (although in libc, not in libc++). If it's the latter, then you have to be very careful with it because using the global locale from multiple threads is not safe (on FreeBSD or Darwin) if you call any of the mbs* functions. You need to call duplocale() on the global locale if this is what you want, and have an instance of it for each thread.

David

22.3.1.5 locale static members [locale.statics]:

static const locale& classic();

4 The "C" locale.

5 Returns: A locale that implements the classic "C" locale semantics, equivalent to the value locale("C").

6 Remarks: This locale, its facets, and their member functions, do not change with time.

Howard

2011/9/23 Howard Hinnant <hhinnant@apple.com>

Committed revision 140384.

Howard

Thanks.

Can anything be done about this GCC incompatibility: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10980#c7 ? It is preventing me from linking a functional libc++ (as Clang is not capable of producing correct object files to be linked together), and of course, testing the beast :). To fix it, this basically needs the functions in question to not be declared as “always inline”, because GCC produces an error in this case. If Clang does not inline in this case, it should really also produce an error instead of silently ignoring the attribute.

Ruben

Seems like the easiest thing to do would be to rewrite these without using va_list. Only one or two arguments need to be supported. Just make these always-inline templates.

Howard

2011/9/23 Howard Hinnant <hhinnant@apple.com>

windows.patch (7.54 KB)

2011/9/25 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/23 Howard Hinnant <hhinnant@apple.com>

Can anything be done about this GCC incompatibility: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10980#c7 ? It is preventing me from linking a functional libc++ (as Clang is not capable of producing correct object files to be linked together), and of course, testing the beast :). To fix it, this basically needs the functions in question to not be declared as “always inline”, because GCC produces an error in this case. If Clang does not inline in this case, it should really also produce an error instead of silently ignoring the attribute.

Seems like the easiest thing to do would be to rewrite these without using va_list. Only one or two arguments need to be supported. Just make these always-inline templates.

Thanks for the valuable idea. Attached patch implements these for non-clang compiles (#ifdef clang). I also used the available *_l function variants for those that are available on Windows (MSVC++ runtime 8.0 and later). I also fixed up the win32 support header stuff to work with the full header things required.
I added a mingw bit to the lib/buildit script, that for now quite hackishly links to libsupc++ and allows multiple definitions due to a missing alternative. This will at least allow some testing to happen on the rest of the libc++ code, that isn’t compiled into the library itself.
I envision a full LLVM alternative to libgcc and libsupc++, but that is still a looooong way off.

Comments and especially commits are very welcome!

Apologies, I missed a return statement in the previous patch. Attached is an updated one.

windows.patch (7.55 KB)

2011/9/25 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/25 Ruben Van Boxem <vanboxem.ruben@gmail.com>

2011/9/23 Howard Hinnant <hhinnant@apple.com>

Can anything be done about this GCC incompatibility: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10980#c7 ? It is preventing me from linking a functional libc++ (as Clang is not capable of producing correct object files to be linked together), and of course, testing the beast :). To fix it, this basically needs the functions in question to not be declared as “always inline”, because GCC produces an error in this case. If Clang does not inline in this case, it should really also produce an error instead of silently ignoring the attribute.

Seems like the easiest thing to do would be to rewrite these without using va_list. Only one or two arguments need to be supported. Just make these always-inline templates.

Thanks for the valuable idea. Attached patch implements these for non-clang compiles (#ifdef clang). I also used the available *_l function variants for those that are available on Windows (MSVC++ runtime 8.0 and later). I also fixed up the win32 support header stuff to work with the full header things required.
I added a mingw bit to the lib/buildit script, that for now quite hackishly links to libsupc++ and allows multiple definitions due to a missing alternative. This will at least allow some testing to happen on the rest of the libc++ code, that isn’t compiled into the library itself.
I envision a full LLVM alternative to libgcc and libsupc++, but that is still a looooong way off.

Comments and especially commits are very welcome!

Apologies, I missed a return statement in the previous patch. Attached is an updated one.

Ruben

PS: the only thing holding back a test run is an undefined reference in mingw-w64’s winpthreads library.

Since this undefined reference was due to a missing extern “C” in winpthreads, nothing is stopping me and anyone alse from running the test suite for Windows! Except for a small attached patch, that also allows GCC to be used (with appropriate environment variables), because GCC on Windows does not produce a.out, but a.exe (Clang always uses a.out) as a default executable name. Feel free to change the “a.out” name to “test” or something else for every other platform.

There is still an unanswered (by Howard or anyone else that feels like he can make high-impact decisions): How will the Windows DLL exports be marked in libc++? The simplest would be something like this:
#if defined _LIBCPP_DLL
#ifdef _BUILDING_LIBCPP
#define _LIBCPP_API __declspec(dllexport)
#else
#define _LIBCPP_API __declspec(dllimport)
#endif
#else
#define _LIBCPP_API
#endif

and every symbol/class marked with these macros. The problem with this is that _LIBCPP_DLL needs to be defined when linking user programs to libc++. The alternative is creating/maintaining a .def file listing all the exports. This also allows for better ABI backwards-compatibility and is done by libstdc++. The problem with this approach is 1) getting a list of all these symbols, 2) maintaining this list when ABI changes, and 3) MSVC and MinGW/GCC/Clang mangle C++ in different ways. A third and the least refined way is using --export-all-symbols when linking the libc++ DLL. I do not know if this works though (testing it now). All this nonsense has also been resolved recently in LLVM/Clang, and perhaps libc++ can copy their approach, if such a user-library approach makes sense for a runtime library like libc++.

Ruben

testitwindows.patch (765 Bytes)

See:

http://gcc.gnu.org/wiki/Visibility

I'd like to try and use the above approach. libc++ has different spellings for the macros than indicated in the above link:

_LIBCPP_HIDDEN / DLL_LOCAL
_LIBCPP_VISIBLE / DLL_PUBLIC

Also relevant is:

_LIBCPP_EXCEPTION_ABI
_LIBCPP_ALWAYS_INLINE
_LIBCPP_INLINE_VISIBILITY

The last two are nothing but mistaken duplicates of each other. On my to-do list is to turn these last two macros into one.

Howard

I think it is getting time for a refactoring in <locale>.

What appears to be happening is that functions like asprintf_l are getting implemented for each platform, which I think is a good thing. But it appears to be happening in an increasingly convoluted way. For example we have:

#ifdef _LIBCPP_STABLE_APPLE_ABI
        __n = asprintf_l(&__bb, 0, "%.0Lf", __units);
#else
        __n = __asprintf_l(&__bb, __cloc(), "%.0Lf", __units);
#endif

The first branch is only taken by __APPLE__. The second branch is taken by everyone else and the definition of __asprintf_l is earlier in <locale>. This definition sometimes calls vasprintf_l, vasprintf, asprintf, and even (ironically) asprintf_l! And it isn't easy (at least to me), to see what is happening on each platform.

Suggestion:

#ifdef __APPLE__
# define _LIBCPP_GET_C_LOCALE 0
#else
# define _LIBCPP_GET_C_LOCALE __cloc()
#endif

...

     __n = asprintf_l(&__bb, _LIBCPP_GET_C_LOCALE, "%.0Lf", __units);

And then everyone go off and implement asprintf_l, preferably not in <locale>. E.g. (and correct me if I'm wrong), __FreeBSD__ has done this in libc. _WIN32 is doing it in src/support/win32/support.cpp. I'd like to see this block of code:

// OSX has nice foo_l() functions that let you turn off use of the global
// locale. Linux, not so much. The following functions avoid the locale when
// that's possible and otherwise do the wrong thing. FIXME.
#ifndef _LIBCPP_STABLE_APPLE_ABI

just disappear, or at the very least become greatly reduced.

Thoughts?

Howard

I think it is getting time for a refactoring in <locale>.

What appears to be happening is that functions like asprintf_l are getting implemented for each platform, which I think is a good thing. But it appears to be happening in an increasingly convoluted way. For example we have:

#ifdef _LIBCPP_STABLE_APPLE_ABI
     __n = asprintf_l(&__bb, 0, "%.0Lf", __units);
#else
     __n = __asprintf_l(&__bb, __cloc(), "%.0Lf", __units);
#endif

That struck me as a mess when I was trying to follow the code.

The first branch is only taken by __APPLE__.

It should also be taken by FreeBSD, since we now have a complete xlocale implementation. I'm not sure if it is, because the mess of #ifdefs confused me (and I've no idea what stable Apple ABI is supposed to mean).

The second branch is taken by everyone else and the definition of __asprintf_l is earlier in <locale>. This definition sometimes calls vasprintf_l, vasprintf, asprintf, and even (ironically) asprintf_l!

I don't really understand why this happens. If you have vasprintf_l(), the odds are that you have asprintf_l() too...

And it isn't easy (at least to me), to see what is happening on each platform.

Completely agree.

Suggestion:

#ifdef __APPLE__
# define _LIBCPP_GET_C_LOCALE 0
#else
# define _LIBCPP_GET_C_LOCALE __cloc()
#endif

I'm not sure what this is for. 0 is not the C locale on Apple, it is the current thread's locale. From the xlocale(3) man page on Darwin:

For each of these rou-
tines, if a NULL locale_t is given, the current locale is used.

If we should be using the C locale, then we should not be using 0 on OS X. If we should be using the current locale, we should not be using __cloc() anywhere else...

...

  __n = asprintf_l(&__bb, _LIBCPP_GET_C_LOCALE, "%.0Lf", __units);

Yes please! I already fixed one copy-and-paste bug where the OS X code path was correct but the version in the #else clause was not (and, if we set the proper printf attributes on these printf-analogues, would have been rejected by the compiler.

And then everyone go off and implement asprintf_l, preferably not in <locale>. E.g. (and correct me if I'm wrong), __FreeBSD__ has done this in libc.

Yup, FreeBSD (pending code review) has this in libc, just like Darwin.

_WIN32 is doing it in src/support/win32/support.cpp. I'd like to see this block of code:

// OSX has nice foo_l() functions that let you turn off use of the global
// locale. Linux, not so much. The following functions avoid the locale when
// that's possible and otherwise do the wrong thing. FIXME.
#ifndef _LIBCPP_STABLE_APPLE_ABI

just disappear, or at the very least become greatly reduced.

Thoughts?

Sounds good, modulo the queries I've already raised,

David

On the same man page:

CONVENIENCE FUNCTIONS
     ... If a NULL locale_t is passed, the C locale will be used.

Howard

2011/9/25 Howard Hinnant <hhinnant@apple.com>

2011/9/23 Howard Hinnant <hhinnant@apple.com>

Can anything be done about this GCC incompatibility: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10980#c7 ? It is preventing me from linking a functional libc++ (as Clang is not capable of producing correct object files to be linked together), and of course, testing the beast :). To fix it, this basically needs the functions in question to not be declared as “always inline”, because GCC produces an error in this case. If Clang does not inline in this case, it should really also produce an error instead of silently ignoring the attribute.

Seems like the easiest thing to do would be to rewrite these without using va_list. Only one or two arguments need to be supported. Just make these always-inline templates.

Thanks for the valuable idea. Attached patch implements these for non-clang compiles (#ifdef clang). I also used the available *_l function variants for those that are available on Windows (MSVC++ runtime 8.0 and later). I also fixed up the win32 support header stuff to work with the full header things required.
I added a mingw bit to the lib/buildit script, that for now quite hackishly links to libsupc++ and allows multiple definitions due to a missing alternative. This will at least allow some testing to happen on the rest of the libc++ code, that isn’t compiled into the library itself.
I envision a full LLVM alternative to libgcc and libsupc++, but that is still a looooong way off.

Comments and especially commits are very welcome!

Ruben

PS: the only thing holding back a test run is an undefined reference in mingw-w64’s winpthreads library.
<windows.patch>

I think it is getting time for a refactoring in .

What appears to be happening is that functions like asprintf_l are getting implemented for each platform, which I think is a good thing. But it appears to be happening in an increasingly convoluted way. For example we have:

#ifdef _LIBCPP_STABLE_APPLE_ABI
__n = asprintf_l(&__bb, 0, “%.0Lf”, __units);
#else
__n = __asprintf_l(&__bb, __cloc(), “%.0Lf”, __units);
#endif

The first branch is only taken by APPLE. The second branch is taken by everyone else and the definition of __asprintf_l is earlier in . This definition sometimes calls vasprintf_l, vasprintf, asprintf, and even (ironically) asprintf_l! And it isn’t easy (at least to me), to see what is happening on each platform.

Suggestion:

#ifdef APPLE

define _LIBCPP_GET_C_LOCALE 0

#else

define _LIBCPP_GET_C_LOCALE __cloc()

#endif

__n = asprintf_l(&__bb, _LIBCPP_GET_C_LOCALE, “%.0Lf”, __units);

And then everyone go off and implement asprintf_l, preferably not in . E.g. (and correct me if I’m wrong), FreeBSD has done this in libc. _WIN32 is doing it in src/support/win32/support.cpp. I’d like to see this block of code:

// OSX has nice foo_l() functions that let you turn off use of the global
// locale. Linux, not so much. The following functions avoid the locale when
// that’s possible and otherwise do the wrong thing. FIXME.
#ifndef _LIBCPP_STABLE_APPLE_ABI

just disappear, or at the very least become greatly reduced.

If the __foo_l functions are just the wrappers that they seem to be, I can certainly live with removing this layer of indirection and me implementing the missing functions that are now wrongfully bypassed. I think expecting all xlocale functions to be present is fair for future implementations/ports. This would mean some additions to the support/win32/locale.h header only, which would concentrate any real problems there, which is a good thing. All platform-related fixme’s and their future fixes (at least the win32 ones) would go in one centralized place, away from the public libc++ headers and core libc++. Other ports can do the same.

In that block of code, there is also a construct
__locale_raii __current(uselocale(__l), uselocale);
int __res = sprintf(__s, __format, __args…);

Does this do the right thing; a very wasteful implementation of the correct behavior, or is this just an attempt to not make things worse. This would be important for me implementing missing *_l functions. If the above worked like I hope it does, that becomes very easy, albeit unperformant.

Thanks!

Ruben