Fix and speedup __libcpp_locale_guard on Windows

[thomasanderson]

Summary

The old implementation assumed the POSIX setlocale() API where the old
locale is returned. On Windows, the _new_ locale is returned. This meant
that __libcpp_locale_guard wasn't resetting the locale on destruction.

The new implementation fixes the above issue and takes advantage of
setlocale(LC_ALL) to reduce the number of calls, and also avoids setting
the locale at all if it's not necessary.

[thomasanderson]

+ @smeenai please review

@EricWF or @ldionne or @mclow.lists please review and give formal approval

[thomasanderson]

Handle memory alloc failures

[EricWF]

What happens here when the user has different locales for different categories?

That said, this feels like a POSIX-ism we're leaking into our Windows implementation. So I'm open to doing whatever is more Windows-like.

[thomasanderson]

In D59572#1440417, @EricWF wrote:

What happens here when the user has different locales for different categories?

Then __setlocale() on line 71 will return a string like "LC_CTYPE=C;LC_MONETARY=en-US;...". This string will then be fed back into __setlocale() on line 85 to restore the categories to what they were.

That said, this feels like a POSIX-ism we're leaking into our Windows implementation. So I'm open to doing whatever is more Windows-like.

[thomasanderson]

In D59572#1440417, @EricWF wrote:

That said, this feels like a POSIX-ism we're leaking into our Windows implementation. So I'm open to doing whatever is more Windows-like.

Ideally we shouldn't have to use __libcpp_locale_guard at all. I think its only use right now is for internally converting ints (and other numbers) to strings by setting the locale to "C", calling some variant of sprintf("%d"), and restoring the locale. This seems like a 100 lb solution to a 1 lb problem. setlocale() and the printf() family of functions are slow. I think it would be much faster and not very difficult to just implement this in libc++.

[EricWF]

[smeenai]

include/__locale

85

Have you confirmed that setlocale does the right thing when given the semicolon-separated list of locale settings for the different categories? The docs talk about returning such a string for the setlocale(LC_ALL, nullptr) case, but they don't mention what happens if you try to use such a string to set the locale. https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/setlocale-wsetlocale?view=vs-2017

[thomasanderson]

include/__locale

85

Yes, I've verified that it does have the correct behavior

[smeenai]

include/__locale

85

Awesome, LGTM. Might be worth adding a comment to that effect, since the documentation isn't explicit about it.

[thomasanderson]

Add comment suggested by smeenai@

[mclow.lists]

include/__locale

96

I really doubt that this is going to work for anyone who builds with exceptions disabled.

On the other hand, maybe that's "not a thing" on windows and/or mingw.

[thomasanderson]

include/__locale

96

Looks like __throw_bad_alloc() calls std::abort() if building without exceptions. (Also there's similar unguarded calls to __throw_bad_alloc() in eg. include/locale)

[thomasanderson]

@mclow.lists does this lg to land?

[mclow.lists]

include/__locale

96

This is completely my bad. I was wrong here.

I read this as throw bad_alloc, which will not work with exceptions disabled.
__throw_bad_alloc, on the other hand, will work fine.

[thomasanderson]

include/__locale

96

Ok cool, I'll land this as-is in that case

include/__locale

 #include <string>
 #include <memory>
 #include <utility>
 #include <mutex>
 #include <cstdint>
 #include <cctype>
 #include <locale.h>
 #if defined(_LIBCPP_MSVCRT_LIKE)
+# include <cstring>
 # include <support/win32/locale_win32.h>
 #elif defined(_AIX)
 # include <support/ibm/xlocale.h>
 #elif defined(__ANDROID__)
 # include <support/android/locale_bionic.h>
 #elif defined(__sun__)
 # include <xlocale.h>
 # include <support/solaris/xlocale.h>
struct __libcpp_locale_guard {
   locale_t __old_loc_;
 private:
   __libcpp_locale_guard(__libcpp_locale_guard const&);
   __libcpp_locale_guard& operator=(__libcpp_locale_guard const&);
 };
 #elif defined(_LIBCPP_MSVCRT_LIKE)
 struct __libcpp_locale_guard {
     __libcpp_locale_guard(locale_t __l) :
-        __status(_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)),
-        __locale_collate(setlocale(LC_COLLATE, __l.__get_locale())),
-        __locale_ctype(setlocale(LC_CTYPE, __l.__get_locale())),
-        __locale_monetary(setlocale(LC_MONETARY, __l.__get_locale())),
-        __locale_numeric(setlocale(LC_NUMERIC, __l.__get_locale())),
-        __locale_time(setlocale(LC_TIME, __l.__get_locale()))
-        // LC_MESSAGES is not supported on Windows.
-    {}
+        __status(_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)) {
+      // Setting the locale can be expensive even when the locale given is
+      // already the current locale, so do an explicit check to see if the
+      // current locale is already the one we want.
+      const char* __lc = __setlocale(nullptr);
+      // If every category is the same, the locale string will simply be the
+      // locale name, otherwise it will be a semicolon-separated string listing
+      // each category.  In the second case, we know at least one category won't
+      // be what we want, so we only have to check the first case.
+      if (strcmp(__l.__get_locale(), __lc) != 0) {
+        __locale_all = _strdup(__lc);
+        if (__locale_all == nullptr)
+          __throw_bad_alloc();
+        __setlocale(__l.__get_locale());
+      }
+    }
     ~__libcpp_locale_guard() {
-        setlocale(LC_COLLATE, __locale_collate);
-        setlocale(LC_CTYPE, __locale_ctype);
-        setlocale(LC_MONETARY, __locale_monetary);
-        setlocale(LC_NUMERIC, __locale_numeric);
-        setlocale(LC_TIME, __locale_time);
-        _configthreadlocale(__status);
+      // The CRT documentation doesn't explicitly say, but setlocale() does the
+      // right thing when given a semicolon-separated list of locale settings
+      // for the different categories in the same format as returned by
+      // setlocale(LC_ALL, nullptr).
+      if (__locale_all != nullptr) {
+        __setlocale(__locale_all);
+        free(__locale_all);
+      }
     }
+    static const char* __setlocale(const char* __locale) {
+      const char* __new_locale = setlocale(LC_ALL, __locale);
+      if (__new_locale == nullptr)
+        __throw_bad_alloc();
+      return __new_locale;
+    }
     int __status;
-    char* __locale_collate;
-    char* __locale_ctype;
-    char* __locale_monetary;
-    char* __locale_numeric;
-    char* __locale_time;
+    char* __locale_all = nullptr;
 };
 #endif
 
 
 class _LIBCPP_TYPE_VIS locale;
 
 template <class _Facet>
 _LIBCPP_INLINE_VISIBILITY