From fe4db13587bfdc1b0b0bdb857c7dde4534c9e862 Mon Sep 17 00:00:00 2001 From: Roland Reichwein Date: Fri, 28 Aug 2026 12:49:01 +0200 Subject: [PATCH] Make etl::variant trivially copyable (#1574) --- include/etl/private/variant_variadic.h | 171 ++++++++++++++++--------- test/test_variant_variadic.cpp | 29 +++++ 2 files changed, 140 insertions(+), 60 deletions(-) diff --git a/include/etl/private/variant_variadic.h b/include/etl/private/variant_variadic.h index b2b8771e..5f6444b2 100644 --- a/include/etl/private/variant_variadic.h +++ b/include/etl/private/variant_variadic.h @@ -555,113 +555,94 @@ namespace etl } }; - //*************************************************************************** - /// 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. - //*************************************************************************** + //******************************************* + /// Trait: are all types trivially copyable? + //******************************************* template - struct variant_base + struct are_all_trivially_copyable : etl::conjunction...> + { + }; + + //*************************************************************************** + /// 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 + struct variant_trivially_destructible_base { variadic_union 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 - constexpr variant_base(etl::in_place_index_t, T&& value, - size_t id) noexcept(etl::is_nothrow_constructible, T>::value) + constexpr variant_trivially_destructible_base(etl::in_place_index_t, T&& value, + size_t id) noexcept(etl::is_nothrow_constructible, T>::value) : data(etl::in_place_index_t{}, etl::forward(value)) , type_id(id) { } - variant_base(const variant_base& other) noexcept(etl::conjunction...>::value) + variant_trivially_destructible_base(const variant_trivially_destructible_base& other) noexcept( + etl::conjunction...>::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...>::value) + variant_trivially_destructible_base(variant_trivially_destructible_base&& other) noexcept( + etl::conjunction...>::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...>::value - && etl::conjunction...>::value) + variant_trivially_destructible_base& + operator=(const variant_trivially_destructible_base& other) noexcept(etl::conjunction...>::value + && etl::conjunction...>::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...>::value - && etl::conjunction...>::value) + variant_trivially_destructible_base& + operator=(variant_trivially_destructible_base&& other) noexcept(etl::conjunction...>::value + && etl::conjunction...>::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...>::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 - { - memcpy(static_cast(&data), static_cast(&other.data), sizeof(data)); - } - - void move_construct_from(const variant_base& other, etl::true_type) noexcept - { - memcpy(static_cast(&data), static_cast(&other.data), sizeof(data)); - } - - void copy_assign_from(const variant_base& other, etl::true_type) noexcept - { - type_id = other.type_id; - memcpy(static_cast(&data), static_cast(&other.data), sizeof(data)); - } - - void move_assign_from(const variant_base& other, etl::true_type) noexcept - { - type_id = other.type_id; - memcpy(static_cast(&data), static_cast(&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) + void copy_construct_from(const variant_trivially_destructible_base& other) { 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 + struct variant_trivially_destructible_base + { + variadic_union 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 + constexpr variant_trivially_destructible_base(etl::in_place_index_t, T&& value, + size_t id) noexcept(etl::is_nothrow_constructible, T>::value) + : data(etl::in_place_index_t{}, etl::forward(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 + struct variant_base : public variant_trivially_destructible_base::value, TTypes...> + { + typedef variant_trivially_destructible_base::value, TTypes...> base_t; + + constexpr variant_base() noexcept + : base_t() + { + } + + constexpr variant_base(size_t id) noexcept + : base_t(id) + { + } + + template + constexpr variant_base(etl::in_place_index_t, T&& value, + size_t id) noexcept(etl::is_nothrow_constructible, T>::value) + : base_t(etl::in_place_index_t{}, etl::forward(value), id) + { + } + }; + //******************************************* /// Trait: are all types copy constructible? //******************************************* diff --git a/test/test_variant_variadic.cpp b/test/test_variant_variadic.cpp index 0680f79a..8e81a606 100644 --- a/test/test_variant_variadic.cpp +++ b/test/test_variant_variadic.cpp @@ -1324,6 +1324,35 @@ namespace CHECK(etl::get(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 >::value)); + CHECK((std::is_trivially_copy_constructible >::value)); + CHECK((std::is_trivially_move_constructible >::value)); + CHECK((std::is_trivially_copy_assignable >::value)); + CHECK((std::is_trivially_move_assignable >::value)); + + // An alternative that is not trivially copyable prevents propagation. + CHECK((!std::is_trivially_copyable >::value)); + + // The variant still copies and moves correctly. + typedef etl::variant 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(variant_2)); + + trivial_variant variant_3; + variant_3 = variant_2; + CHECK_EQUAL(2U, variant_3.index()); + CHECK_EQUAL(1.5, etl::get(variant_3)); + } + //************************************************************************* // Issue #1512: even though the copy operations are disabled at compile // time, a move-only variant must still move construct / move assign