iox-#2052 Refactor copy_and_move_impl

iceoryx_dust/container/include/iox/detail/fixed_position_container.inl

@@ -18,10 +18,8 @@
 #ifndef IOX_DUST_CONTAINER_DETAIL_FIXED_POSITION_CONTAINER_INL
 #define IOX_DUST_CONTAINER_DETAIL_FIXED_POSITION_CONTAINER_INL
 
-#include "iox/fixed_position_container.hpp"
-#if __cplusplus < 201703L
 #include "iox/detail/fixed_position_container_helper.hpp"
-#endif
+#include "iox/fixed_position_container.hpp"
 
 namespace iox
 {
@@ -58,23 +56,13 @@ inline FixedPositionContainer<T, CAPACITY>::~FixedPositionContainer() noexcept
 template <typename T, uint64_t CAPACITY>
 inline FixedPositionContainer<T, CAPACITY>::FixedPositionContainer(const FixedPositionContainer& rhs) noexcept
 {
-    for (IndexType i = 0; i < CAPACITY; ++i)
-    {
-        m_status[i] = SlotStatus::FREE;
-    }
-
-    *this = rhs;
+    copy_and_move_impl<detail::MoveAndCopyOperations::CopyConstructor>(rhs);
 }
 
 template <typename T, uint64_t CAPACITY>
 inline FixedPositionContainer<T, CAPACITY>::FixedPositionContainer(FixedPositionContainer&& rhs) noexcept
 {
-    for (IndexType i = 0; i < CAPACITY; ++i)
-    {
-        m_status[i] = SlotStatus::FREE;
-    }
-
-    *this = std::move(rhs);
+    copy_and_move_impl<detail::MoveAndCopyOperations::MoveConstructor>(std::move(rhs));
 }
 
 template <typename T, uint64_t CAPACITY>
@@ -83,7 +71,7 @@ FixedPositionContainer<T, CAPACITY>::operator=(const FixedPositionContainer& rhs
 {
     if (this != &rhs)
     {
-        copy_and_move_impl(rhs);
+        copy_and_move_impl<detail::MoveAndCopyOperations::CopyAssignment>(rhs);
     }
     return *this;
 }
@@ -94,73 +82,67 @@ FixedPositionContainer<T, CAPACITY>::operator=(FixedPositionContainer&& rhs) noe
 {
     if (this != &rhs)
     {
-        copy_and_move_impl(std::move(rhs));
-
-        // clear rhs
-        rhs.clear();
+        copy_and_move_impl<detail::MoveAndCopyOperations::MoveAssignment>(std::move(rhs));
     }
     return *this;
 }
 
 template <typename T, uint64_t CAPACITY>
-template <typename RhsType>
+template <detail::MoveAndCopyOperations Opt, typename RhsType>
 inline void FixedPositionContainer<T, CAPACITY>::copy_and_move_impl(RhsType&& rhs) noexcept
 {
-    // we already make sure rhs is always passed by std::move() for move case, therefore
-    // the result of "decltype(rhs)" is same as "decltype(std::forward<RhsType>(rhs))"
-    static_assert(std::is_rvalue_reference<decltype(rhs)>::value
-                      || (std::is_lvalue_reference<decltype(rhs)>::value
-                          && std::is_const<std::remove_reference_t<decltype(rhs)>>::value),
-                  "RhsType must be const lvalue reference or rvalue reference");
+    // alias helper struct
+    using Helper = detail::MoveAndCopyHelper<Opt>;
+
+    constexpr bool is_ctor = Helper::is_ctor();
+    constexpr bool is_move = Helper::is_move();
 
-    constexpr bool is_move = std::is_rvalue_reference<decltype(rhs)>::value;
+    // status array is not yet initialized for constructor creation
+    if constexpr (is_ctor)
+    {
+        for (IndexType i = 0; i < CAPACITY; ++i)
+        {
+            m_status[i] = SlotStatus::FREE;
+        }
+    }
 
     IndexType i{Index::FIRST};
     auto rhs_it = (std::forward<RhsType>(rhs)).begin();
 
-    // transfer src data to destination
     for (; rhs_it.to_index() != Index::INVALID; ++i, ++rhs_it)
     {
         if (m_status[i] == SlotStatus::USED)
         {
-#if __cplusplus >= 201703L
+            // When the slot is in the 'USED' state, it is safe to proceed with either construction (ctor) or assignment
+            // operation. Therefore, creation can be carried out according to the option specified by Opt.
             if constexpr (is_move)
             {
-                m_data[i] = std::move(*rhs_it);
+                Helper::transfer(m_data[i], std::move(*rhs_it));
             }
             else
             {
-                m_data[i] = *rhs_it;
+                Helper::transfer(m_data[i], *rhs_it);
             }
-#else
-            // We introduce a helper struct primarily due to the test case
-            // e1cc7c9f-c1b5-4047-811b-004302af5c00. It demands compile-time if-else branching
-            // for classes that are either non-copyable or non-moveable.
-            // Note: With C++17's 'if constexpr', the need for these helper structs can be eliminated.
-            detail::AssignmentHelper<is_move>::assign(m_data[i], detail::MoveHelper<is_move>::move_or_copy(rhs_it));
-#endif
         }
-        // use ctor to avoid UB for non-initialized free slots
         else
         {
-#if __cplusplus >= 201703L
+            // When the slot is in the 'FREE' state, it is unsafe to proceed with assignment operation.
+            // Therefore, we need to force helper to use ctor create to make sure that the 'FREE' slots get initialized.
             if constexpr (is_move)
             {
-                new (&m_data[i]) T(std::move(*rhs_it));
+                Helper::ctor_create(m_data[i], std::move(*rhs_it));
             }
             else
             {
-                new (&m_data[i]) T(*rhs_it);
+                Helper::ctor_create(m_data[i], *rhs_it);
             }
-#else
-            // Same as above
-            detail::CtorHelper<is_move>::construct(m_data[i], detail::MoveHelper<is_move>::move_or_copy(rhs_it));
-#endif
         }
+
         m_status[i] = SlotStatus::USED;
         m_next[i] = static_cast<IndexType>(i + 1U);
     }
 
+    // reset rest
     for (; i < CAPACITY; ++i)
     {
         if (m_status[i] == SlotStatus::USED)
@@ -174,6 +156,7 @@ inline void FixedPositionContainer<T, CAPACITY>::copy_and_move_impl(RhsType&& rh
         m_next[i] = next;
     }
 
+    // correct m_next
     m_next[Index::LAST] = Index::INVALID;
     if (!rhs.empty())
     {
@@ -183,6 +166,12 @@ inline void FixedPositionContainer<T, CAPACITY>::copy_and_move_impl(RhsType&& rh
     m_begin_free = static_cast<IndexType>(rhs.m_size);
     m_begin_used = rhs.empty() ? Index::INVALID : Index::FIRST;
     m_size = rhs.m_size;
+
+    // reset rhs if is_move is true
+    if constexpr (is_move)
+    {
+        rhs.clear();
+    }
 }
 
 template <typename T, uint64_t CAPACITY>

iceoryx_dust/container/include/iox/detail/fixed_position_container_helper.hpp

@@ -17,81 +17,105 @@
 #ifndef IOX_DUST_CONTAINER_DETAIL_FIXED_POSITION_CONTAINER_HELPER_HPP
 #define IOX_DUST_CONTAINER_DETAIL_FIXED_POSITION_CONTAINER_HELPER_HPP
 
+#include <functional>
 #include <utility>
 
 namespace iox
 {
 namespace detail
 {
-template <bool IsMove>
-struct AssignmentHelper;
 
-template <>
-struct AssignmentHelper<true>
+enum class MoveAndCopyOperations
 {
-    template <typename T>
-    static void assign(T& dest, T&& src)
-    {
-        dest = std::forward<T>(src);
-    }
+    CopyConstructor,
+    CopyAssignment,
+    MoveConstructor,
+    MoveAssignment,
 };
 
-template <>
-struct AssignmentHelper<false>
+/// @brief MoveAndCopyHelper is a template structure used to create or assign objects based on the provided
+/// operation type (Opt).
+/// @tparam Opt The operation type that determines how objects are created or assigned.
+template <MoveAndCopyOperations Opt>
+struct MoveAndCopyHelper
 {
-    template <typename T>
-    static void assign(T& dest, const T& src)
+    /// @brief Creates or assigns an object to 'dest' based on the specail operation type.
+    /// @tparam T The type of the object to be created or assigned.
+    /// @tparam V The type of the source object, kept as a universal reference to preserve its lvalue or rvalue nature.
+    /// @param[out] dest The destination object where the new object is created or to which the source object is
+    /// assigned.
+    /// @param[in] src The source object, either for copy or move operations.
+    template <typename T, typename V>
+    static inline void transfer(T& dest, V&& src) noexcept
     {
-        dest = src;
+        if constexpr (is_ctor())
+        {
+            ctor_create(dest, std::forward<V>(src));
+        }
+        else
+        {
+            assignment_create(dest, std::forward<V>(src));
+        }
     }
-};
-
-template <bool IsMove>
-struct CtorHelper;
 
-template <>
-struct CtorHelper<true>
-{
-    template <typename T>
-    static void construct(T& dest, T&& src)
+    /// @brief Force to use constructor to create an object at the destination.
+    /// @tparam T The type of the object to be constructed.
+    /// @tparam V The type of the source object, used for move or copy construction.
+    /// @param[out] dest The destination object where the new object is constructed.
+    /// @param[in] src The source object, either for move or copy construction.
+    template <typename T, typename V>
+    static inline void ctor_create(T& dest, V&& src) noexcept
     {
-        new (&dest) T(std::forward<T>(src));
+        if constexpr (is_move())
+        {
+            static_assert(std::is_rvalue_reference_v<decltype(src)>, "src should be rvalue reference");
+            static_assert(std::is_convertible_v<V, T>, "src type is not convertible to dest type");
+            new (&dest) T(std::forward<V>(src));
+        }
+        else
+        {
+            static_assert(std::is_lvalue_reference_v<decltype(src)>, "src should be lvalue reference");
+            static_assert(std::is_const_v<std::remove_reference_t<decltype(src)>>, "src should has 'const' modifier");
+            static_assert(std::is_convertible_v<V, T>, "src type is not convertible to dest type");
+            new (&dest) T(src);
+        }
     }
-};
 
-template <>
-struct CtorHelper<false>
-{
-    template <typename T>
-    static void construct(T& dest, const T& src)
+    /// @brief Force to use assignment to assign an object to the destination.
+    /// @tparam T The type of the destination object.
+    /// @tparam V The type of the source object, used for move or copy assignment.
+    /// @param dest The destination object where the source object is assigned.
+    /// @param src The source object, either for move or copy assignment.
+    template <typename T, typename V>
+    static inline void assignment_create(T& dest, V&& src) noexcept
     {
-        new (&dest) T(src);
+        if constexpr (is_move())
+        {
+            static_assert(std::is_rvalue_reference_v<decltype(src)>, "src should be rvalue reference");
+            dest = std::forward<V>(src);
+        }
+        else
+        {
+            static_assert(std::is_lvalue_reference_v<decltype(src)>, "src should be lvalue reference");
+            static_assert(std::is_const_v<std::remove_reference_t<decltype(src)>>, "src should has 'const' modifier");
+            dest = src;
+        }
     }
-};
-
-template <bool IsMove>
-struct MoveHelper;
 
-template <>
-struct MoveHelper<true>
-{
-    template <typename Iterator>
-    static auto move_or_copy(Iterator& it) -> decltype(std::move(*it))
+    /// @brief Checks if the current special operation is a constructor call.
+    /// @return True if the operation is a copy or move constructor, false otherwise.
+    static constexpr bool is_ctor() noexcept
     {
-        return std::move(*it);
+        return Opt == MoveAndCopyOperations::CopyConstructor || Opt == MoveAndCopyOperations::MoveConstructor;
     }
-};
 
-template <>
-struct MoveHelper<false>
-{
-    template <typename Iterator>
-    static auto move_or_copy(Iterator& it) -> decltype(*it)
+    /// @brief Checks if the current special operation is a move operation.
+    /// @return True if the operation is a move constructor or move assignment, false otherwise.
+    static constexpr bool is_move() noexcept
     {
-        return *it;
+        return Opt == MoveAndCopyOperations::MoveAssignment || Opt == MoveAndCopyOperations::MoveConstructor;
     }
 };
-
+} // namespace detail
+} // namespace iox
 #endif
-}
-}

iceoryx_dust/container/include/iox/fixed_position_container.hpp

@@ -19,6 +19,7 @@
 
 #include "iceoryx_hoofs/cxx/requires.hpp"
 #include "iox/algorithm.hpp"
+#include "iox/detail/fixed_position_container_helper.hpp"
 #include "iox/uninitialized_array.hpp"
 
 #include <cstdint>
@@ -315,7 +316,7 @@ class FixedPositionContainer final
     };
 
   private:
-    template <typename RhsType>
+    template <detail::MoveAndCopyOperations Opt, typename RhsType>
     void copy_and_move_impl(RhsType&& rhs) noexcept;
 
   private: