[libcxx-commits] [PATCH] D59525: Speed up certain locale functions on Windows

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai added inline comments.


================
Comment at: include/__locale:67
         __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.
+        __locale_all(setlocale(LC_ALL, __l.__get_locale()))
     {}
----------------
I'm not really familiar with locales, but if I'm reading [MSDN](https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/setlocale-wsetlocale?view=vs-2017) correctly, wouldn't you need a call with a NULL `locale` argument to get the value of all categories? I'm not sure how you'd then use that to restore the categories though.

Also, "Global or thread local storage is used for the string returned by setlocale. Later calls to setlocale overwrite the string, which invalidates string pointers returned by earlier calls." Shouldn't we be copying the return value instead of just storing a pointer? (This is already potentially a problem with the existing code, unless I'm misunderstanding things.)


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D59525