[libc++] Remove the need for uselocale()

[ldionne]

Instead of requiring uselocale() as part of the base locale API, define __locale_guard in the few places that need it directly, without making __locale_guard part of the base API.

In practice, most mainstream platforms never used __locale_guard, so they also didn't need to define uselocale(), and after this patch they actually don't define it anymore.

[llvmbot]

@llvm/pr-subscribers-github-workflow

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

Instead of requiring uselocale() as part of the base locale API, define __locale_guard in the few places that need it directly, without making __locale_guard part of the base API.

In practice, most mainstream platforms never used __locale_guard, so they also didn't need to define uselocale(), and after this patch they actually don't define it anymore.

---

Full diff: https://github.com/llvm/llvm-project/pull/120158.diff

10 Files Affected:

• (modified) libcxx/include/CMakeLists.txt (-1)
• (modified) libcxx/include/__locale_dir/locale_base_api.h (-6)
• (modified) libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h (+14-2)
• (removed) libcxx/include/__locale_dir/locale_guard.h (-78)
• (modified) libcxx/include/__locale_dir/support/bsd_like.h (-2)
• (modified) libcxx/include/__locale_dir/support/windows.h (+38-4)
• (modified) libcxx/include/__support/xlocale/__nop_locale_mgmt.h (-2)
• (modified) libcxx/include/module.modulemap (-2)
• (modified) libcxx/src/iostream.cpp (+1-5)
• (modified) libcxx/src/support/win32/locale_win32.cpp (+14-15)

diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index ba392e5883ff39..b5bbfbe7d3a233 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -503,7 +503,6 @@ set(files
   __locale_dir/locale_base_api/ibm.h
   __locale_dir/locale_base_api/musl.h
   __locale_dir/locale_base_api/openbsd.h
-  __locale_dir/locale_guard.h
   __locale_dir/pad_and_output.h
   __locale_dir/support/apple.h
   __locale_dir/support/bsd_like.h
diff --git a/libcxx/include/__locale_dir/locale_base_api.h b/libcxx/include/__locale_dir/locale_base_api.h
index 7e3c205c04a780..c8097beb9052df 100644
--- a/libcxx/include/__locale_dir/locale_base_api.h
+++ b/libcxx/include/__locale_dir/locale_base_api.h
@@ -23,14 +23,12 @@
 // Variadic functions may be implemented as templates with a parameter pack instead
 // of C-style variadic functions.
 //
-// TODO: I think __uselocale() is not necessary if we refactor a bit.
 // TODO: __localeconv shouldn't take a reference, but the Windows implementation doesn't allow copying __locale_t
 //
 // Locale management
 // -----------------
 // namespace __locale {
 //  using __locale_t = implementation-defined;
-//  __locale_t  __uselocale(__locale_t);
 //  __locale_t  __newlocale(int, const char*, __locale_t);
 //  void        __freelocale(__locale_t);
 //  lconv*      __localeconv(__locale_t&);
@@ -139,10 +137,6 @@ namespace __locale {
 //
 using __locale_t = locale_t;
 
-#  ifndef _LIBCPP_MSVCRT_LIKE
-inline _LIBCPP_HIDE_FROM_ABI __locale_t __uselocale(__locale_t __loc) { return uselocale(__loc); }
-#  endif
-
 inline _LIBCPP_HIDE_FROM_ABI __locale_t __newlocale(int __category_mask, const char* __name, __locale_t __loc) {
   return newlocale(__category_mask, __name, __loc);
 }
diff --git a/libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h b/libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h
index d514bdfaace903..b62a1b737e97f9 100644
--- a/libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h
+++ b/libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h
@@ -14,8 +14,6 @@
 #define _LIBCPP___LOCALE_DIR_LOCALE_BASE_API_BSD_LOCALE_FALLBACKS_H
 
 #include <locale.h>
-
-#include <__locale_dir/locale_guard.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -30,6 +28,20 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+struct __locale_guard {
+  _LIBCPP_HIDE_FROM_ABI __locale_guard(locale_t& __loc) : __old_loc_(::uselocale(__loc)) {}
+
+  _LIBCPP_HIDE_FROM_ABI ~__locale_guard() {
+    if (__old_loc_)
+      ::uselocale(__old_loc_);
+  }
+
+  locale_t __old_loc_;
+
+  __locale_guard(__locale_guard const&)            = delete;
+  __locale_guard& operator=(__locale_guard const&) = delete;
+};
+
 inline _LIBCPP_HIDE_FROM_ABI decltype(MB_CUR_MAX) __libcpp_mb_cur_max_l(locale_t __l) {
   __locale_guard __current(__l);
   return MB_CUR_MAX;
diff --git a/libcxx/include/__locale_dir/locale_guard.h b/libcxx/include/__locale_dir/locale_guard.h
deleted file mode 100644
index 93d38334f31323..00000000000000
--- a/libcxx/include/__locale_dir/locale_guard.h
+++ /dev/null
@@ -1,78 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef _LIBCPP___LOCALE_DIR_LOCALE_GUARD_H
-#define _LIBCPP___LOCALE_DIR_LOCALE_GUARD_H
-
-#include <__config>
-#include <__locale> // for locale_t
-#include <clocale>
-
-#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-#  pragma GCC system_header
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-#if defined(_LIBCPP_MSVCRT_LIKE)
-struct __locale_guard {
-  __locale_guard(__locale::__locale_t __l) : __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 (std::strcmp(__l.__get_locale(), __lc) != 0) {
-      __locale_all = _strdup(__lc);
-      if (__locale_all == nullptr)
-        __throw_bad_alloc();
-      __setlocale(__l.__get_locale());
-    }
-  }
-  ~__locale_guard() {
-    // 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_all = nullptr;
-};
-#else
-struct __locale_guard {
-  _LIBCPP_HIDE_FROM_ABI __locale_guard(locale_t& __loc) : __old_loc_(uselocale(__loc)) {}
-
-  _LIBCPP_HIDE_FROM_ABI ~__locale_guard() {
-    if (__old_loc_)
-      uselocale(__old_loc_);
-  }
-
-  locale_t __old_loc_;
-
-  __locale_guard(__locale_guard const&)            = delete;
-  __locale_guard& operator=(__locale_guard const&) = delete;
-};
-#endif
-
-_LIBCPP_END_NAMESPACE_STD
-
-#endif // _LIBCPP___LOCALE_DIR_LOCALE_GUARD_H
diff --git a/libcxx/include/__locale_dir/support/bsd_like.h b/libcxx/include/__locale_dir/support/bsd_like.h
index cce6de64673b0e..da31aeaf3c58e3 100644
--- a/libcxx/include/__locale_dir/support/bsd_like.h
+++ b/libcxx/include/__locale_dir/support/bsd_like.h
@@ -38,8 +38,6 @@ namespace __locale {
 //
 using __locale_t = ::locale_t;
 
-inline _LIBCPP_HIDE_FROM_ABI __locale_t __uselocale(__locale_t __loc) { return ::uselocale(__loc); }
-
 inline _LIBCPP_HIDE_FROM_ABI __locale_t __newlocale(int __category_mask, const char* __locale, __locale_t __base) {
   return ::newlocale(__category_mask, __locale, __base);
 }
diff --git a/libcxx/include/__locale_dir/support/windows.h b/libcxx/include/__locale_dir/support/windows.h
index fd6aab8de9ac8d..4ba7ccfeb585a2 100644
--- a/libcxx/include/__locale_dir/support/windows.h
+++ b/libcxx/include/__locale_dir/support/windows.h
@@ -39,6 +39,44 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 namespace __locale {
 
+struct __locale_guard {
+  _LIBCPP_HIDE_FROM_ABI __locale_guard(locale_t __l) : __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 (std::strcmp(__l.__get_locale(), __lc) != 0) {
+      __locale_all = _strdup(__lc);
+      if (__locale_all == nullptr)
+        __throw_bad_alloc();
+      __setlocale(__l.__get_locale());
+    }
+  }
+  _LIBCPP_HIDE_FROM_ABI ~__locale_guard() {
+    // 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);
+  }
+  _LIBCPP_HIDE_FROM_ABI 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_all = nullptr;
+};
+
 class __lconv_storage {
 public:
   __lconv_storage(const lconv* __lc_input) {
@@ -149,10 +187,6 @@ class __locale_t {
   __lconv_storage* __lc_ = nullptr;
 };
 
-// __uselocale can't be implemented on Windows because Windows allows partial modification
-// of thread-local locale and so _get_current_locale() returns a copy while __uselocale does
-// not create any copies. We can still implement RAII even without __uselocale though.
-__locale_t __uselocale(__locale_t) = delete;
 _LIBCPP_EXPORTED_FROM_ABI __locale_t __newlocale(int __mask, const char* __locale, __locale_t __base);
 inline _LIBCPP_HIDE_FROM_ABI void __freelocale(__locale_t __loc) { ::_free_locale(__loc); }
 _LIBCPP_EXPORTED_FROM_ABI lconv* __localeconv(__locale_t& __loc);
diff --git a/libcxx/include/__support/xlocale/__nop_locale_mgmt.h b/libcxx/include/__support/xlocale/__nop_locale_mgmt.h
index 5aaf3eaa6441d9..eabe169cc4bc8c 100644
--- a/libcxx/include/__support/xlocale/__nop_locale_mgmt.h
+++ b/libcxx/include/__support/xlocale/__nop_locale_mgmt.h
@@ -21,8 +21,6 @@ inline _LIBCPP_HIDE_FROM_ABI void freelocale(locale_t) {}
 
 inline _LIBCPP_HIDE_FROM_ABI locale_t newlocale(int, const char*, locale_t) { return nullptr; }
 
-inline _LIBCPP_HIDE_FROM_ABI locale_t uselocale(locale_t) { return nullptr; }
-
 #define LC_COLLATE_MASK (1 << LC_COLLATE)
 #define LC_CTYPE_MASK (1 << LC_CTYPE)
 #define LC_MESSAGES_MASK (1 << LC_MESSAGES)
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index ba45af273f4842..bd815a66ea6be0 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1466,8 +1466,6 @@ module std [system] {
 
   module locale {
     header "locale"
-
-    module locale_guard   { header "__locale_dir/locale_guard.h" }
     module pad_and_output { header "__locale_dir/pad_and_output.h" }
 
     module support {
diff --git a/libcxx/src/iostream.cpp b/libcxx/src/iostream.cpp
index ee33f4bae6d891..6db02d56037947 100644
--- a/libcxx/src/iostream.cpp
+++ b/libcxx/src/iostream.cpp
@@ -11,10 +11,6 @@
 #include <new>
 #include <string>
 
-#ifdef _LIBCPP_MSVCRT_LIKE
-#  include <__locale_dir/locale_guard.h>
-#endif
-
 #define _str(s) #s
 #define str(s) _str(s)
 #define _LIBCPP_ABI_NAMESPACE_STR str(_LIBCPP_ABI_NAMESPACE)
@@ -109,7 +105,7 @@ static void force_locale_initialization() {
   static bool once = []() {
     auto loc = __locale::__newlocale(LC_ALL_MASK, "C", 0);
     {
-      __locale_guard g(loc); // forces initialization of locale TLS
+      __locale::__locale_guard g(loc); // forces initialization of locale TLS
       ((void)g);
     }
     __locale::__freelocale(loc);
diff --git a/libcxx/src/support/win32/locale_win32.cpp b/libcxx/src/support/win32/locale_win32.cpp
index 0a537d6997ca23..c5d7af41c3e7b1 100644
--- a/libcxx/src/support/win32/locale_win32.cpp
+++ b/libcxx/src/support/win32/locale_win32.cpp
@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <__locale_dir/locale_guard.h>
 #include <__locale_dir/support/windows.h>
 #include <clocale> // std::localeconv() & friends
 #include <cstdarg> // va_start & friends
@@ -28,7 +27,7 @@ __locale_t __newlocale(int /*mask*/, const char* locale, __locale_t /*base*/) {
 }
 
 lconv* __localeconv(__locale_t& loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   lconv* lc = std::localeconv();
   if (!lc)
     return lc;
@@ -40,12 +39,12 @@ lconv* __localeconv(__locale_t& loc) {
 //
 #if !defined(_LIBCPP_MSVCRT)
 float __strtof(const char* nptr, char** endptr, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::strtof(nptr, endptr);
 }
 
 long double __strtold(const char* nptr, char** endptr, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::strtold(nptr, endptr);
 }
 #endif
@@ -55,7 +54,7 @@ long double __strtold(const char* nptr, char** endptr, __locale_t loc) {
 //
 #if defined(__MINGW32__) && __MSVCRT_VERSION__ < 0x0800
 size_t __strftime(char* ret, size_t n, const char* format, const struct tm* tm, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::strftime(ret, n, format, tm);
 }
 #endif
@@ -67,18 +66,18 @@ decltype(MB_CUR_MAX) __mb_len_max(__locale_t __l) {
 #if defined(_LIBCPP_MSVCRT)
   return ::___mb_cur_max_l_func(__l);
 #else
-  std::__locale_guard __current(__l);
+  __locale_guard __current(__l);
   return MB_CUR_MAX;
 #endif
 }
 
 wint_t __btowc(int c, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::btowc(c);
 }
 
 int __wctob(wint_t c, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::wctob(c);
 }
 
@@ -88,12 +87,12 @@ size_t __wcsnrtombs(char* __restrict dst,
                     size_t len,
                     mbstate_t* __restrict ps,
                     __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return ::wcsnrtombs(dst, src, nwc, len, ps);
 }
 
 size_t __wcrtomb(char* __restrict s, wchar_t wc, mbstate_t* __restrict ps, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::wcrtomb(s, wc, ps);
 }
 
@@ -103,24 +102,24 @@ size_t __mbsnrtowcs(wchar_t* __restrict dst,
                     size_t len,
                     mbstate_t* __restrict ps,
                     __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return ::mbsnrtowcs(dst, src, nms, len, ps);
 }
 
 size_t
 __mbrtowc(wchar_t* __restrict pwc, const char* __restrict s, size_t n, mbstate_t* __restrict ps, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::mbrtowc(pwc, s, n, ps);
 }
 
 size_t __mbrlen(const char* __restrict s, size_t n, mbstate_t* __restrict ps, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::mbrlen(s, n, ps);
 }
 
 size_t __mbsrtowcs(
     wchar_t* __restrict dst, const char** __restrict src, size_t len, mbstate_t* __restrict ps, __locale_t loc) {
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   return std::mbsrtowcs(dst, src, len, ps);
 }
 
@@ -132,7 +131,7 @@ int __snprintf(char* ret, size_t n, __locale_t loc, const char* format, ...) {
   int result = ::__stdio_common_vsprintf(
       _CRT_INTERNAL_LOCAL_PRINTF_OPTIONS | _CRT_INTERNAL_PRINTF_STANDARD_SNPRINTF_BEHAVIOR, ret, n, format, loc, ap);
 #else
-  std::__locale_guard __current(loc);
+  __locale_guard __current(loc);
   _LIBCPP_DIAGNOSTIC_PUSH
   _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wformat-nonliteral")
   int result = std::vsnprintf(ret, n, format, ap);

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 254ba78495100d9f20c4fa9802395f11c6d3cef1 41ba06c6bf032fde3b408d65b2f772ad497b58d3 --extensions h,cpp -- libcxx/include/__locale_dir/locale_base_api.h libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h libcxx/include/__locale_dir/support/bsd_like.h libcxx/include/__locale_dir/support/windows.h libcxx/include/__support/xlocale/__nop_locale_mgmt.h libcxx/src/iostream.cpp libcxx/src/support/win32/locale_win32.cpp

View the diff from clang-format here.

diff --git a/libcxx/include/__locale_dir/support/windows.h b/libcxx/include/__locale_dir/support/windows.h
index 81fbd74af9..03d05a410f 100644
--- a/libcxx/include/__locale_dir/support/windows.h
+++ b/libcxx/include/__locale_dir/support/windows.h
@@ -285,8 +285,7 @@ _LIBCPP_DIAGNOSTIC_POP
 #undef _LIBCPP_VARIADIC_ATTRIBUTE_FORMAT
 
 struct __locale_guard {
-  _LIBCPP_HIDE_FROM_ABI __locale_guard(__locale_t __l)
-      : __status(_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)) {
+  _LIBCPP_HIDE_FROM_ABI __locale_guard(__locale_t __l) : __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.