[libc++] Avoid <memory> include in locale_win32.h

[smeenai]

Summary

When _LIBCPP_NO_EXCEPTIONS is defined, we end up with compile errors
when targeting MSVCRT:

• Code includes <new>
• <new> includes <cstdlib> in order to get abort
• <cstdlib> includes <stdlib.h>, _before_ the using ::abort
• <stdlib.h> includes locale_win32.h
• locale_win32.h includes <memory>
• <memory> includes <stdexcept>
• <stdexcept> includes <cstdlib for abort, but that inclusion gets (correctly) ignored because of header guards
• <stdexcept> references _VSTD::abort, which isn't declared

The easiest solution is to make locale_win32.h not include <memory>,
by removing the use of unique_ptr and manually restoring the locale
instead.

[smeenai]

I'm aware that my replacement code isn't entirely equivalent, since it won't restore the locale in case MB_CUR_MAX throws. However, I'm quite certain the MB_CUR_MAX implementation won't throw. I can make my own RAII wrapper if desired, however.

Alternatively, MSDN documents an ___mb_cur_max_l_func, which is available Visual Studio 2013 onwards and does exactly what we want. It does warn that it's an internal CRT function and recommends against its use, hence my current implementation avoiding it. However, if we're okay using internal CRT functions, it would make for a cleaner implementation, and avoid the locale restoration issues altogether.

[EricWF]

LGTM. I'm OK with things that are "technically" regressions in Windows support if it helps us get it working today.

[smeenai]

Cool. Any thoughts on this implementation vs. ___mb_cur_max_l_func? In other words, do we have a general policy on using internal CRT functionality? FWIW, support/win32/support.h was using xlocinfo.h before my recent cleanup, which is also an internal header.

[bcraig]

This seems related:
https://reviews.llvm.org/D20596

In that (stale) review, I switch away from unique_ptr in general for locale related operations.

[smeenai]

@bcraig thanks for pointing me to that diff; there's a lot of nice cleanup going on there. Were you planning on updating and following up on it?

I also realized I forgot to adjust locale_win32.cpp for this diff. Will re-upload with that fixed.

[bcraig]

In D24374#538521, @smeenai wrote:

@bcraig thanks for pointing me to that diff; there's a lot of nice cleanup going on there. Were you planning on updating and following up on it?

I also realized I forgot to adjust locale_win32.cpp for this diff. Will re-upload with that fixed.

I'm still in favor of that change, but I don't expect to have the time, resources, or permission to submit or test that change in the near future. I'm about to change jobs, and the new position doesn't involve working on the llvm projects.

[smeenai]

Correcting support_win32.cpp

[smeenai]

In D24374#538534, @bcraig wrote:

In D24374#538521, @smeenai wrote:

@bcraig thanks for pointing me to that diff; there's a lot of nice cleanup going on there. Were you planning on updating and following up on it?

I also realized I forgot to adjust locale_win32.cpp for this diff. Will re-upload with that fixed.

I'm still in favor of that change, but I don't expect to have the time, resources, or permission to submit or test that change in the near future. I'm about to change jobs, and the new position doesn't involve working on the llvm projects.

Ah, that's unfortunate.

include/support/win32/locale_win32.h

 #define _LIBCPP_SUPPORT_WIN32_LOCALE_WIN32_H
 
 // ctype mask table defined in msvcrt.dll
 extern "C" unsigned short  __declspec(dllimport) _ctype[];
 
 #include "support/win32/support.h"
 #include "support/win32/locale_mgmt_win32.h"
 #include <stdio.h>
-#include <memory>
 
 lconv *localeconv_l( locale_t loc );
 size_t mbrlen_l( const char *__restrict s, size_t n,
                  mbstate_t *__restrict ps, locale_t loc);
 size_t mbsrtowcs_l( wchar_t *__restrict dst, const char **__restrict src,
                     size_t len, mbstate_t *__restrict ps, locale_t loc );
 size_t wcrtomb_l( char *__restrict s, wchar_t wc, mbstate_t *__restrict ps,
                   locale_t loc);
 size_t mbrtowc_l( wchar_t *__restrict pwc, const char *__restrict s,
                   size_t n, mbstate_t *__restrict ps, locale_t loc);
 size_t mbsnrtowcs_l( wchar_t *__restrict dst, const char **__restrict src,
                      size_t nms, size_t len, mbstate_t *__restrict ps, locale_t loc);
 size_t wcsnrtombs_l( char *__restrict dst, const wchar_t **__restrict src,
                      size_t nwc, size_t len, mbstate_t *__restrict ps, locale_t loc);
 wint_t btowc_l( int c, locale_t loc );
 int wctob_l( wint_t c, locale_t loc );
-typedef _VSTD::remove_pointer<locale_t>::type __locale_struct;
-typedef _VSTD::unique_ptr<__locale_struct, decltype(&uselocale)> __locale_raii;
 inline _LIBCPP_ALWAYS_INLINE
 decltype(MB_CUR_MAX) MB_CUR_MAX_L( locale_t __l )
 {
-  __locale_raii __current( uselocale(__l), uselocale );
-  return MB_CUR_MAX;
+  return ___mb_cur_max_l_func(__l);
 }
 
 // the *_l functions are prefixed on Windows, only available for msvcr80+, VS2005+
 #define mbtowc_l _mbtowc_l
 #define strtoll_l _strtoi64_l
 #define strtoull_l _strtoui64_l
 // FIXME: current msvcrt does not know about long double
 #define strtold_l _strtod_l

src/support/win32/locale_win32.cpp

 // -*- C++ -*-
 //===-------------------- support/win32/locale_win32.cpp ------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include <locale>
 #include <cstdarg> // va_start, va_end
+#include <memory>
+#include <type_traits>
+
+typedef _VSTD::remove_pointer<locale_t>::type __locale_struct;
+typedef _VSTD::unique_ptr<__locale_struct, decltype(&uselocale)> __locale_raii;
 
 // FIXME: base currently unused. Needs manual work to construct the new locale
 locale_t newlocale( int mask, const char * locale, locale_t /*base*/ )
 {
     return _create_locale( mask, locale );
 }
 locale_t uselocale( locale_t newloc )
 {