[cfe-dev] [PATCH] Libc++ Windows fixes (Attention all libc++ ports!!!)

[David Chisnall]

On 25 Sep 2011, at 22:30, Howard Hinnant wrote:

> 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