Make etl::variant trivially copyable (#1574)

include/etl/private/variant_variadic.h

@@ -555,113 +555,94 @@ namespace etl
       }
     };
 
-    //***************************************************************************
+    //*******************************************
-    /// variant_base specialisation for trivially destructible types.
+    /// Trait: are all types trivially copyable?
-    /// Uses variadic_union storage. Destructor is trivial (defaulted), making
+    //*******************************************
-    /// the variant a literal type eligible for constexpr / ROM placement.
-    /// No operation function pointer is needed since destroy/copy/move are
-    /// all handled without indirection for trivially destructible types.
-    //***************************************************************************
     template <typename... TTypes>
-    struct variant_base<true, TTypes...>
+    struct are_all_trivially_copyable : etl::conjunction<etl::is_trivially_copyable<TTypes>...>
+    {
+    };
+
+    //***************************************************************************
+    /// Storage and copy/move operations used by the trivially destructible
+    /// variant_base specialisation.
+    /// The general case defines the copy and move operations in terms of the
+    /// operations of the currently active alternative.
+    //***************************************************************************
+    template <bool IsAllTriviallyCopyable, typename... TTypes>
+    struct variant_trivially_destructible_base
     {
       variadic_union<TTypes...> data;
       size_t                    type_id;
 
-      constexpr variant_base() noexcept
+      constexpr variant_trivially_destructible_base() noexcept
         : data()
         , type_id(variant_npos)
       {
       }
 
-      constexpr variant_base(size_t id) noexcept
+      constexpr variant_trivially_destructible_base(size_t id) noexcept
         : data()
         , type_id(id)
       {
       }
 
       template <size_t Index, typename T>
-      constexpr variant_base(etl::in_place_index_t<Index>, T&& value,
+      constexpr variant_trivially_destructible_base(etl::in_place_index_t<Index>, T&& value,
-                             size_t id) noexcept(etl::is_nothrow_constructible<etl::nth_type_t<Index, TTypes...>, T>::value)
+                                                    size_t id) noexcept(etl::is_nothrow_constructible<etl::nth_type_t<Index, TTypes...>, T>::value)
         : data(etl::in_place_index_t<Index>{}, etl::forward<T>(value))
         , type_id(id)
       {
       }
 
-      variant_base(const variant_base& other) noexcept(etl::conjunction<etl::is_nothrow_copy_constructible<TTypes>...>::value)
+      variant_trivially_destructible_base(const variant_trivially_destructible_base& other) noexcept(
+        etl::conjunction<etl::is_nothrow_copy_constructible<TTypes>...>::value)
         : data()
         , type_id(other.type_id)
       {
-        copy_construct_from(other, is_all_trivially_copyable{});
+        copy_construct_from(other);
       }
 
-      variant_base(variant_base&& other) noexcept(etl::conjunction<etl::is_nothrow_move_constructible<TTypes>...>::value)
+      variant_trivially_destructible_base(variant_trivially_destructible_base&& other) noexcept(
+        etl::conjunction<etl::is_nothrow_move_constructible<TTypes>...>::value)
         : data()
         , type_id(other.type_id)
       {
-        move_construct_from(other, is_all_trivially_copyable{});
+        move_construct_from(other);
       }
 
-      variant_base& operator=(const variant_base& other) noexcept(etl::conjunction<etl::is_nothrow_copy_constructible<TTypes>...>::value
+      variant_trivially_destructible_base&
-                                                                  && etl::conjunction<etl::is_nothrow_copy_assignable<TTypes>...>::value)
+        operator=(const variant_trivially_destructible_base& other) noexcept(etl::conjunction<etl::is_nothrow_copy_constructible<TTypes>...>::value
+                                                                             && etl::conjunction<etl::is_nothrow_copy_assignable<TTypes>...>::value)
       {
         if (this != &other)
         {
-          copy_assign_from(other, is_all_trivially_copyable{});
+          copy_assign_from(other);
         }
         return *this;
       }
 
-      variant_base& operator=(variant_base&& other) noexcept(etl::conjunction<etl::is_nothrow_move_constructible<TTypes>...>::value
+      variant_trivially_destructible_base&
-                                                             && etl::conjunction<etl::is_nothrow_move_assignable<TTypes>...>::value)
+        operator=(variant_trivially_destructible_base&& other) noexcept(etl::conjunction<etl::is_nothrow_move_constructible<TTypes>...>::value
+                                                                        && etl::conjunction<etl::is_nothrow_move_assignable<TTypes>...>::value)
       {
         if (this != &other)
         {
-          move_assign_from(other, is_all_trivially_copyable{});
+          move_assign_from(other);
         }
         return *this;
       }
 
-      ~variant_base() = default;
+      ~variant_trivially_destructible_base() = default;
 
     private:
 
-      // All alternatives trivially copyable => a raw-byte copy correctly
-      // reproduces both copy and move construction.
-      typedef etl::integral_constant<bool, etl::conjunction<etl::is_trivially_copyable<TTypes>...>::value> is_all_trivially_copyable;
-
       //*******************************************
-      // Trivially copyable fast path: raw-byte copy.
+      // Dispatch to the active alternative's own copy/move operations.
+      // The active union member shares its address with the union, so the
+      // raw-pointer dispatch places the new object in the correct storage.
       //*******************************************
-      void copy_construct_from(const variant_base& other, etl::true_type) noexcept
+      void copy_construct_from(const variant_trivially_destructible_base& other)
-      {
-        memcpy(static_cast<void*>(&data), static_cast<const void*>(&other.data), sizeof(data));
-      }
-
-      void move_construct_from(const variant_base& other, etl::true_type) noexcept
-      {
-        memcpy(static_cast<void*>(&data), static_cast<const void*>(&other.data), sizeof(data));
-      }
-
-      void copy_assign_from(const variant_base& other, etl::true_type) noexcept
-      {
-        type_id = other.type_id;
-        memcpy(static_cast<void*>(&data), static_cast<const void*>(&other.data), sizeof(data));
-      }
-
-      void move_assign_from(const variant_base& other, etl::true_type) noexcept
-      {
-        type_id = other.type_id;
-        memcpy(static_cast<void*>(&data), static_cast<const void*>(&other.data), sizeof(data));
-      }
-
-      //*******************************************
-      // Non-trivial path: dispatch to the active alternative's own
-      // copy/move constructor. The active union member shares its address
-      // with the union, so the raw-pointer dispatch places the new object
-      // in the correct storage.
-      //*******************************************
-      void copy_construct_from(const variant_base& other, etl::false_type)
       {
         if (other.type_id != variant_npos)
         {
@@ -669,7 +650,7 @@ namespace etl
         }
       }
 
-      void move_construct_from(variant_base& other, etl::false_type)
+      void move_construct_from(variant_trivially_destructible_base& other)
       {
         if (other.type_id != variant_npos)
         {
@@ -677,7 +658,7 @@ namespace etl
         }
       }
 
-      void copy_assign_from(const variant_base& other, etl::false_type)
+      void copy_assign_from(const variant_trivially_destructible_base& other)
       {
         if ((type_id != variant_npos) && (type_id == other.type_id))
         {
@@ -698,7 +679,7 @@ namespace etl
         }
       }
 
-      void move_assign_from(variant_base& other, etl::false_type)
+      void move_assign_from(variant_trivially_destructible_base& other)
       {
         if ((type_id != variant_npos) && (type_id == other.type_id))
         {
@@ -720,6 +701,76 @@ namespace etl
       }
     };
 
+    //***************************************************************************
+    /// Specialisation for when all of the types are trivially copyable.
+    /// All of the copy and move operations are defaulted, so that they are
+    /// trivial and the variant itself is trivially copyable. See P0602R4.
+    //***************************************************************************
+    template <typename... TTypes>
+    struct variant_trivially_destructible_base<true, TTypes...>
+    {
+      variadic_union<TTypes...> data;
+      size_t                    type_id;
+
+      constexpr variant_trivially_destructible_base() noexcept
+        : data()
+        , type_id(variant_npos)
+      {
+      }
+
+      constexpr variant_trivially_destructible_base(size_t id) noexcept
+        : data()
+        , type_id(id)
+      {
+      }
+
+      template <size_t Index, typename T>
+      constexpr variant_trivially_destructible_base(etl::in_place_index_t<Index>, T&& value,
+                                                    size_t id) noexcept(etl::is_nothrow_constructible<etl::nth_type_t<Index, TTypes...>, T>::value)
+        : data(etl::in_place_index_t<Index>{}, etl::forward<T>(value))
+        , type_id(id)
+      {
+      }
+
+      variant_trivially_destructible_base(const variant_trivially_destructible_base&)            = default;
+      variant_trivially_destructible_base(variant_trivially_destructible_base&&)                 = default;
+      variant_trivially_destructible_base& operator=(const variant_trivially_destructible_base&) = default;
+      variant_trivially_destructible_base& operator=(variant_trivially_destructible_base&&)      = default;
+      ~variant_trivially_destructible_base()                                                     = default;
+    };
+
+    //***************************************************************************
+    /// variant_base specialisation for trivially destructible types.
+    /// Uses variadic_union storage. Destructor is trivial (defaulted), making
+    /// the variant a literal type eligible for constexpr / ROM placement.
+    /// No operation function pointer is needed since destroy/copy/move are
+    /// all handled without indirection for trivially destructible types.
+    /// The copy and move operations are inherited so that they are trivial
+    /// when every alternative is trivially copyable.
+    //***************************************************************************
+    template <typename... TTypes>
+    struct variant_base<true, TTypes...> : public variant_trivially_destructible_base<are_all_trivially_copyable<TTypes...>::value, TTypes...>
+    {
+      typedef variant_trivially_destructible_base<are_all_trivially_copyable<TTypes...>::value, TTypes...> base_t;
+
+      constexpr variant_base() noexcept
+        : base_t()
+      {
+      }
+
+      constexpr variant_base(size_t id) noexcept
+        : base_t(id)
+      {
+      }
+
+      template <size_t Index, typename T>
+      constexpr variant_base(etl::in_place_index_t<Index>, T&& value,
+                             size_t id) noexcept(etl::is_nothrow_constructible<etl::nth_type_t<Index, TTypes...>, T>::value)
+        : base_t(etl::in_place_index_t<Index>{}, etl::forward<T>(value), id)
+      {
+      }
+    };
+
     //*******************************************
     /// Trait: are all types copy constructible?
     //*******************************************

test/test_variant_variadic.cpp

@@ -1324,6 +1324,35 @@ namespace
       CHECK(etl::get<TrivialAssignTracker>(variant_1).move_assigned);
     }
 
+    //*************************************************************************
+    // Issue #1567: the variant propagates trivial copyability from its
+    // alternatives, as required by P0602R4.
+    TEST(test_trivially_copyable)
+    {
+      // Fundamental types are trivially copyable in every ETL configuration.
+      CHECK((std::is_trivially_copyable<etl::variant<char, int, double> >::value));
+      CHECK((std::is_trivially_copy_constructible<etl::variant<char, int, double> >::value));
+      CHECK((std::is_trivially_move_constructible<etl::variant<char, int, double> >::value));
+      CHECK((std::is_trivially_copy_assignable<etl::variant<char, int, double> >::value));
+      CHECK((std::is_trivially_move_assignable<etl::variant<char, int, double> >::value));
+
+      // An alternative that is not trivially copyable prevents propagation.
+      CHECK((!std::is_trivially_copyable<etl::variant<int, AssignTracker> >::value));
+
+      // The variant still copies and moves correctly.
+      typedef etl::variant<char, int, double> trivial_variant;
+
+      trivial_variant variant_1(1.5);
+      trivial_variant variant_2(variant_1);
+      CHECK_EQUAL(2U, variant_2.index());
+      CHECK_EQUAL(1.5, etl::get<double>(variant_2));
+
+      trivial_variant variant_3;
+      variant_3 = variant_2;
+      CHECK_EQUAL(2U, variant_3.index());
+      CHECK_EQUAL(1.5, etl::get<double>(variant_3));
+    }
+
     //*************************************************************************
     // Issue #1512: even though the copy operations are disabled at compile
     // time, a move-only variant must still move construct / move assign