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