[libcxx-commits] [PATCH] D69505: [libcxx] [Windows] Store the lconv struct returned from localeconv in locale_t

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D69505#1726163 <https://reviews.llvm.org/D69505#1726163>, @rnk wrote:

> Exciting. =/ So, successive calls to localeconv_l would invalidate the result from the last call.


It's even worse than that. If you call `localeconv_l` with a locale other than the currently set thread specific locale, `__libcpp_locale_guard` will temporarily set the one we want to inspect, then receive the `lconv *` return pointer, and then reset the thread specific locale to what it was before the function was called. When the thread specific locale is reset on return, the `lconv *` pointer we're returning is invalidated.

So currently, `localeconv_l` never works, unless it's used on the currently set locale.

> I could live with that if it was possible to make it private to libcxx.

Any suggestions on how to achieve that? Everything in `include/locale`, `include/__locale` and `include/support/win32/locale_win32.h` ends up included by callers. This current patch changes the ABI of the `locale_t` class on windows, but is that part of the API/ABI that users of libcxx are expected to use, or is it just a helper that accidentally ends up visible? (The `locale_t` class ends up defined without any enclosing namespace whatsoever, at the moment.)

One way could be to add some wrapper around the `localeconv_l` calls in src/locale.cpp. Ideally a wrapper that still returns `lconv *` for compatibility with everything else, but which can be made to allocate temp storage on the stack for windows. But I struggle to see a neat way of doing it.

One way that does come to mind is something preprocessor based, like this:

  #ifdef _WIN32
  #define GET_LCONV_PTR(var, loc) \
    lconv_storage lcs##var; \
    { \
      __libcpp_locale_guard __current(loc); \
      lcs.store(localeconv()); \
    } \
    lconv *var = lcs##var.getPtr()
  #else
  #define GET_LCONV_PTR(var, loc) \
    lconv *var = localeconv_l(loc)
  #endif


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69505/new/

https://reviews.llvm.org/D69505