[libcxx-commits] [PATCH] D59572: Fix and speedup __libcpp_locale_guard on Windows

[Tom Anderson via Phabricator via libcxx-commits]

thomasanderson updated this revision to Diff 192336.
thomasanderson added a comment.

Add comment suggested by smeenai@


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

https://reviews.llvm.org/D59572

Files:
  include/__locale


Index: include/__locale
===================================================================
--- include/__locale
+++ include/__locale
@@ -19,6 +19,7 @@
 #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>
@@ -63,28 +64,41 @@
 #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);
+      }
+      _configthreadlocale(__status);
+    }
+    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
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59572.192336.patch
Type: text/x-patch
Size: 2969 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190326/d118d7b5/attachment.bin>