[libcxx-commits] [libcxx] [libc++] Refactor the creation of the global and classic locales (PR #72581)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Nov 17 13:20:10 PST 2023


https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/72581

>From a99e9a32dc6b187ad07b89eaa62f06263e764074 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 16 Nov 2023 17:26:55 -0500
Subject: [PATCH 1/2] [libc++] Refactor the creation of the global and classic
 locales

The creation of the global and the classic locales was pretty
twisty. This patch refactors how this is done to reduce the
amount of indirections and prepare the terrain for a future
where GCC implements the no_destroy attribute.
---
 libcxx/include/__locale |  3 +++
 libcxx/src/locale.cpp   | 53 ++++++++++++++++++-----------------------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/libcxx/include/__locale b/libcxx/include/__locale
index b1502dd71edadf6..0b0b94c99393b61 100644
--- a/libcxx/include/__locale
+++ b/libcxx/include/__locale
@@ -127,6 +127,9 @@ private:
     class __imp;
     __imp* __locale_;
 
+    template <class> friend struct no_destroy;
+    _LIBCPP_HIDE_FROM_ABI explicit locale(__imp* __loc) : __locale_(__loc) {}
+
     void __install_ctor(const locale&, facet*, long);
     static locale& __global();
     bool has_facet(id&) const;
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index c37e091dcc4671b..6b1e5a6e4b494c4 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <__utility/unreachable.h>
 #include <algorithm>
 #include <clocale>
 #include <codecvt>
@@ -15,9 +14,11 @@
 #include <cstdlib>
 #include <cstring>
 #include <locale>
+#include <new>
 #include <string>
 #include <type_traits>
 #include <typeinfo>
+#include <utility>
 #include <vector>
 
 #ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
@@ -154,8 +155,6 @@ class _LIBCPP_HIDDEN locale::__imp
         {return static_cast<size_t>(id) < facets_.size() && facets_[static_cast<size_t>(id)];}
     const locale::facet* use_facet(long id) const;
 
-    static const locale& make_classic();
-    static       locale& make_global();
 private:
     void install(facet* f, long id);
     template <class F> void install(F* f) {install(f, f->id.__get());}
@@ -538,37 +537,31 @@ locale::__imp::use_facet(long id) const
 
 // locale
 
-const locale&
-locale::__imp::make_classic()
-{
-    // only one thread can get in here and it only gets in once
-    alignas(locale) static std::byte buf[sizeof(locale)];
-    locale* c = reinterpret_cast<locale*>(&buf);
-    c->__locale_ = &make<__imp>(1u);
-    return *c;
-}
+// This class basically implements __attribute__((no_destroy)), which isn't supported
+// by GCC as of writing this.
+template <class T>
+struct no_destroy {
+    template <class... Args>
+    explicit no_destroy(Args&&... args) {
+        T* obj = reinterpret_cast<T*>(&buf);
+        new (obj) T(std::forward<Args>(args)...);
+    }
 
-const locale&
-locale::classic()
-{
-    static const locale& c = __imp::make_classic();
-    return c;
-}
+    T& get() { return *reinterpret_cast<T*>(&buf); }
+    T const& get() const { return *reinterpret_cast<T const*>(&buf); }
 
-locale&
-locale::__imp::make_global()
-{
-    // only one thread can get in here and it only gets in once
-    alignas(locale) static std::byte buf[sizeof(locale)];
-    auto *obj = ::new (&buf) locale(locale::classic());
-    return *obj;
+  private:
+    alignas(T) byte buf[sizeof(T)];
+};
+
+const locale& locale::classic() {
+    static const no_destroy<locale> c(&make<__imp>(1u));
+    return c.get();
 }
 
-locale&
-locale::__global()
-{
-    static locale& g = __imp::make_global();
-    return g;
+locale& locale::__global() {
+    static no_destroy<locale> g(locale::classic());
+    return g.get();
 }
 
 locale::locale() noexcept

>From 4a7617658502ebd429f266bf62b34474c7bfec03 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Fri, 17 Nov 2023 16:19:55 -0500
Subject: [PATCH 2/2] Fix ambiguous constructor and add test to catch that
 specifically

---
 libcxx/include/__locale                       |  3 ++-
 libcxx/src/locale.cpp                         |  2 +-
 .../locale/locale.cons/char_pointer.pass.cpp  | 22 +++++++++++--------
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/libcxx/include/__locale b/libcxx/include/__locale
index 0b0b94c99393b61..c237b65b0cf3c3f 100644
--- a/libcxx/include/__locale
+++ b/libcxx/include/__locale
@@ -128,7 +128,8 @@ private:
     __imp* __locale_;
 
     template <class> friend struct no_destroy;
-    _LIBCPP_HIDE_FROM_ABI explicit locale(__imp* __loc) : __locale_(__loc) {}
+    struct __private_tag { };
+    _LIBCPP_HIDE_FROM_ABI explicit locale(__private_tag, __imp* __loc) : __locale_(__loc) {}
 
     void __install_ctor(const locale&, facet*, long);
     static locale& __global();
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index 6b1e5a6e4b494c4..7a411039b5d60ab 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -555,7 +555,7 @@ struct no_destroy {
 };
 
 const locale& locale::classic() {
-    static const no_destroy<locale> c(&make<__imp>(1u));
+    static const no_destroy<locale> c(__private_tag{}, &make<__imp>(1u));
     return c.get();
 }
 
diff --git a/libcxx/test/std/localization/locales/locale/locale.cons/char_pointer.pass.cpp b/libcxx/test/std/localization/locales/locale/locale.cons/char_pointer.pass.cpp
index 99131dd245fbdac..bd0693a542848b3 100644
--- a/libcxx/test/std/localization/locales/locale/locale.cons/char_pointer.pass.cpp
+++ b/libcxx/test/std/localization/locales/locale/locale.cons/char_pointer.pass.cpp
@@ -81,21 +81,25 @@ int main(int, char**)
         assert(!(loc == loc3));
         assert(loc != loc3);
 #ifndef TEST_HAS_NO_EXCEPTIONS
-        try
-        {
+        try {
             std::locale((const char*)0);
             assert(false);
+        } catch (std::runtime_error&) {
+            // pass
         }
-        catch (std::runtime_error&)
-        {
+
+        try {
+            std::locale(nullptr);
+            assert(false);
+        } catch (std::runtime_error&) {
+            // pass
         }
-        try
-        {
+
+        try {
             std::locale("spazbot");
             assert(false);
-        }
-        catch (std::runtime_error&)
-        {
+        } catch (std::runtime_error&) {
+            // pass
         }
 #endif
         std::locale ok("");



More information about the libcxx-commits mailing list