From 9e1b727745a993f526747e7025f3685b325c1b6b Mon Sep 17 00:00:00 2001
From: Pavel Kirienko <pavel.kirienko@gmail.com>
Date: Thu, 22 Feb 2024 13:09:05 +0200
Subject: [PATCH] Advance the variant; we need to move the destruction logic
 away from the arena

---
 include/cetl/pf17/variant.hpp | 129 +++++++++++++++++++++++++++++-----
 1 file changed, 113 insertions(+), 16 deletions(-)

diff --git a/include/cetl/pf17/variant.hpp b/include/cetl/pf17/variant.hpp
index df4cf233..480bc4e9 100644
--- a/include/cetl/pf17/variant.hpp
+++ b/include/cetl/pf17/variant.hpp
@@ -10,10 +10,12 @@
 
 #include <cetl/_helpers.hpp>
 #include <cetl/pf17/utility.hpp>
+#include <cetl/pf17/type_traits.hpp>
 #include <cetl/pf17/attribute.hpp>
 
 #include <tuple>
 #include <limits>
+#include <cassert>
 #include <algorithm>
 #include <exception>  // We need this even if exceptions are disabled for std::terminate.
 #include <type_traits>
@@ -25,6 +27,18 @@ namespace pf17
 /// Implementation of \ref std::variant_npos.
 constexpr std::size_t variant_npos = std::numeric_limits<std::size_t>::max();
 
+/// Implementation of \ref std::monostate.
+struct monostate
+{};
+// clang-format off
+inline constexpr bool operator==(const monostate, const monostate) noexcept { return true;  }
+inline constexpr bool operator!=(const monostate, const monostate) noexcept { return false; }
+inline constexpr bool operator< (const monostate, const monostate) noexcept { return false; }
+inline constexpr bool operator> (const monostate, const monostate) noexcept { return false; }
+inline constexpr bool operator<=(const monostate, const monostate) noexcept { return true;  }
+inline constexpr bool operator>=(const monostate, const monostate) noexcept { return true;  }
+// clang-format on
+
 namespace detail
 {
 namespace var
@@ -32,6 +46,26 @@ namespace var
 template <std::size_t N, typename... Ts>
 using nth_type = std::tuple_element_t<N, std::tuple<Ts...>>;
 
+/// An internal helper used to keep the list of the variant types and query their properties.
+template <typename... Ts>
+struct types final
+{
+    template <template <typename> class F>
+    static constexpr bool all_satisfy = conjunction_v<F<Ts>...>;
+
+    static constexpr bool trivially_destructible       = all_satisfy<std::is_trivially_destructible>;
+    static constexpr bool trivially_copyable           = all_satisfy<std::is_trivially_copyable>;
+    static constexpr bool trivially_copy_assignable    = all_satisfy<std::is_trivially_copy_assignable>;
+    static constexpr bool trivially_move_assignable    = all_satisfy<std::is_trivially_move_assignable>;
+    static constexpr bool trivially_copy_constructible = all_satisfy<std::is_trivially_copy_constructible>;
+    static constexpr bool trivially_move_constructible = all_satisfy<std::is_trivially_move_constructible>;
+
+    types()  = delete;
+    ~types() = delete;
+};
+static_assert(types<int, float>::trivially_destructible, "");
+static_assert(!types<int, types<>>::trivially_destructible, "");
+
 /// index_of<> fails with a missing type error if the type is not found in the sequence.
 /// If there is more than one matching type, the index of the first occurrence is selected.
 template <typename T, typename... Ts>
@@ -48,7 +82,6 @@ struct index_of_impl<T, U, Ts...>
 };
 template <typename T, typename... Ts>
 constexpr std::size_t index_of = index_of_impl<T, Ts...>::value;
-
 static_assert(0 == index_of<int, int>, "");
 static_assert(0 == index_of<int, int, double, char>, "");
 static_assert(1 == index_of<double, int, double, char>, "");
@@ -56,33 +89,39 @@ static_assert(2 == index_of<char, int, double, char>, "");
 
 /// This type manages the storage arena for the variant and keeps track of the currently active type.
 /// It expressly ignores copy/move/destruction/assignment; this must be managed externally.
+/// We support the valueless semantics even if __cpp_exceptions is not defined for the sake of ABI compatibility
+/// because there may be another translation unit that uses exceptions.
 template <typename... Ts>
 class arena final
 {
 public:
-    template <std::size_t N, typename... Args>
-    explicit arena(const in_place_index_t<N>, Args&&... args)
+    template <std::size_t N, typename T = nth_type<N, Ts...>, typename... Args>
+    explicit arena(const in_place_index_t<N>, Args&&... args)  // NOLINT(*-pro-type-member-init)
+        : m_index(N)
+        , m_dtor([](void* const data) { reinterpret_cast<T*>(data)->~T(); })
     {
-        construct<N>(std::forward<Args>(args)...);
+        new (m_data) T(std::forward<Args>(args)...);
     }
     template <typename T, typename... Args>
     explicit arena(const in_place_type_t<T>, Args&&... args)
         : arena(in_place_index<index_of<T, Ts...>>, std::forward<Args>(args)...)
     {
     }
-    arena()  // #0 must be default-constructible, otherwise this type is not default-constructible either.
+    arena()  // Type #0 must be default-constructible, otherwise this type is not default-constructible either.
         : arena(in_place_index<0>)
     {
     }
 
-    /// Constructs the new value in the arena; the caller is responsible for destroying the old value beforehand.
-    template <std::size_t N, typename... Args>
+    /// Constructs a new value in the arena. The old value will be destroyed beforehand by calling destroy().
+    /// If the constructor throws, the arena will be left valueless.
+    template <std::size_t N, typename T = nth_type<N, Ts...>, typename... Args>
     auto& construct(Args&&... args)
     {
-        using T = nth_type<N, Ts...>;
-        m_index = N;
-        m_dtor  = [](void* const data) { reinterpret_cast<T*>(data)->~T(); };
-        return *new (m_data) T(std::forward<Args>(args)...);
+        destroy();                                                    // First, make it valueless...
+        auto* const p = new (m_data) T(std::forward<Args>(args)...);  // If constructor throws, stay valueless.
+        m_index       = N;                                            // If constructor succeeds, become non-valueless.
+        m_dtor        = [](void* const data) { reinterpret_cast<T*>(data)->~T(); };
+        return *p;
     }
     template <typename T, typename... Args>
     auto& construct(Args&&... args)
@@ -90,12 +129,18 @@ class arena final
         return construct<index_of<T, Ts...>>(std::forward<Args>(args)...);
     }
 
-    /// Destroys the value and makes this arena valueless.
+    /// Destroys the value and makes this arena valueless. Does nothing if the arena is already valueless.
+    /// If all Ts are trivially destructible, there is no need to invoke this function upon destruction of the arena.
     void destroy() noexcept
     {
-        m_dtor(m_data);
-        m_dtor  = nullptr;
-        m_index = variant_npos;
+        if (!is_valueless())
+        {
+            m_index = variant_npos;
+            assert(m_dtor != nullptr);
+            m_dtor(m_data);
+            m_dtor = nullptr;
+        }
+        assert(is_valueless());
     }
 
     /// Returns variant_npos if valueless.
@@ -104,6 +149,11 @@ class arena final
         return m_index;
     }
 
+    CETL_NODISCARD bool is_valueless() const noexcept
+    {
+        return m_index == variant_npos;
+    }
+
     // clang-format off
     template <typename T>       T&  as()      &  { return *reinterpret_cast<T*>(m_data); }
     template <typename T> const T&  as() const&  { return *reinterpret_cast<const T*>(m_data); }
@@ -118,7 +168,8 @@ class arena final
 
 private:
     alignas(std::max({alignof(Ts)...})) unsigned char m_data[std::max({sizeof(Ts)...})];
-    void (*m_dtor)(void*);  // We could have used visitation instead but this approach does not require runtime search.
+    // We could have used visitation instead but the closure approach does not require runtime search.
+    void        (*m_dtor)(void*);
     std::size_t m_index;
 };
 static_assert(std::is_trivially_copyable<arena<int, double>>::value, "");
@@ -128,6 +179,52 @@ static_assert(std::is_trivially_copy_assignable<arena<int, double>>::value, "");
 static_assert(std::is_trivially_move_assignable<arena<int, double>>::value, "");
 static_assert(std::is_trivially_destructible<arena<int, double>>::value, "");
 
+/// DESTRUCTION POLICY
+template <typename Seq, bool = Seq::trivially_destructible>
+struct base_destruction;
+/// Trivially destructible case.
+template <typename... Ts>
+struct base_destruction<types<Ts...>, true>
+{
+    arena<Ts...> m_arena;
+};
+/// Non-trivially destructible case.
+template <typename... Ts>
+struct base_destruction<types<Ts...>, false>
+{
+    base_destruction()                                   = default;
+    base_destruction(const base_destruction&)            = default;
+    base_destruction(base_destruction&&)                 = default;
+    base_destruction& operator=(const base_destruction&) = default;
+    base_destruction& operator=(base_destruction&&)      = default;
+    ~base_destruction() noexcept
+    {
+        m_arena.destroy();
+    }
+    arena<Ts...> m_arena;
+};
+
+/// COPY CONSTRUCTION POLICY
+template <typename Seq, bool = Seq::trivially_copy_constructible>
+struct base_copy_construction;
+/// Trivially copy constructible case.
+template <typename... Ts>
+struct base_copy_construction<types<Ts...>, true> : base_destruction<types<Ts...>>
+{
+    // Copies constructed by merely copying the bytes of the source.
+};
+/// Non-trivially copy constructible case.
+template <typename... Ts>
+struct base_copy_construction<types<Ts...>, false> : base_destruction<types<Ts...>>
+{
+    base_copy_construction()                                         = default;
+    base_copy_construction(const base_copy_construction&)            = default;
+    base_copy_construction(base_copy_construction&&)                 = default;
+    base_copy_construction& operator=(const base_copy_construction&) = default;
+    base_copy_construction& operator=(base_copy_construction&&)      = default;
+    ~base_copy_construction() noexcept                               = default;
+};
+
 }  // namespace var
 }  // namespace detail