diff --git a/iceoryx_dust/container/include/iox/detail/fixed_position_container.inl b/iceoryx_dust/container/include/iox/detail/fixed_position_container.inl index 613ea1aca6..8be6a9b4e5 100644 --- a/iceoryx_dust/container/include/iox/detail/fixed_position_container.inl +++ b/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::~FixedPositionContainer() noexcept template inline FixedPositionContainer::FixedPositionContainer(const FixedPositionContainer& rhs) noexcept { - for (IndexType i = 0; i < CAPACITY; ++i) - { - m_status[i] = SlotStatus::FREE; - } - - *this = rhs; + copy_and_move_impl(rhs); } template inline FixedPositionContainer::FixedPositionContainer(FixedPositionContainer&& rhs) noexcept { - for (IndexType i = 0; i < CAPACITY; ++i) - { - m_status[i] = SlotStatus::FREE; - } - - *this = std::move(rhs); + copy_and_move_impl(std::move(rhs)); } template @@ -83,7 +71,7 @@ FixedPositionContainer::operator=(const FixedPositionContainer& rhs { if (this != &rhs) { - copy_and_move_impl(rhs); + copy_and_move_impl(rhs); } return *this; } @@ -94,73 +82,67 @@ FixedPositionContainer::operator=(FixedPositionContainer&& rhs) noe { if (this != &rhs) { - copy_and_move_impl(std::move(rhs)); - - // clear rhs - rhs.clear(); + copy_and_move_impl(std::move(rhs)); } return *this; } template -template +template inline void FixedPositionContainer::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(rhs))" - static_assert(std::is_rvalue_reference::value - || (std::is_lvalue_reference::value - && std::is_const>::value), - "RhsType must be const lvalue reference or rvalue reference"); + // alias helper struct + using Helper = detail::MoveAndCopyHelper; + + constexpr bool is_ctor = Helper::is_ctor(); + constexpr bool is_move = Helper::is_move(); - constexpr bool is_move = std::is_rvalue_reference::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(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::assign(m_data[i], detail::MoveHelper::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::construct(m_data[i], detail::MoveHelper::move_or_copy(rhs_it)); -#endif } + m_status[i] = SlotStatus::USED; m_next[i] = static_cast(i + 1U); } + // reset rest for (; i < CAPACITY; ++i) { if (m_status[i] == SlotStatus::USED) @@ -174,6 +156,7 @@ inline void FixedPositionContainer::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::copy_and_move_impl(RhsType&& rh m_begin_free = static_cast(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 diff --git a/iceoryx_dust/container/include/iox/detail/fixed_position_container_helper.hpp b/iceoryx_dust/container/include/iox/detail/fixed_position_container_helper.hpp index ad1c12cb08..76ff0de14a 100644 --- a/iceoryx_dust/container/include/iox/detail/fixed_position_container_helper.hpp +++ b/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 #include namespace iox { namespace detail { -template -struct AssignmentHelper; -template <> -struct AssignmentHelper +enum class MoveAndCopyOperations { - template - static void assign(T& dest, T&& src) - { - dest = std::forward(src); - } + CopyConstructor, + CopyAssignment, + MoveConstructor, + MoveAssignment, }; -template <> -struct AssignmentHelper +/// @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 +struct MoveAndCopyHelper { - template - 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 + static inline void transfer(T& dest, V&& src) noexcept { - dest = src; + if constexpr (is_ctor()) + { + ctor_create(dest, std::forward(src)); + } + else + { + assignment_create(dest, std::forward(src)); + } } -}; - -template -struct CtorHelper; -template <> -struct CtorHelper -{ - template - 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 + static inline void ctor_create(T& dest, V&& src) noexcept { - new (&dest) T(std::forward(src)); + if constexpr (is_move()) + { + static_assert(std::is_rvalue_reference_v, "src should be rvalue reference"); + static_assert(std::is_convertible_v, "src type is not convertible to dest type"); + new (&dest) T(std::forward(src)); + } + else + { + static_assert(std::is_lvalue_reference_v, "src should be lvalue reference"); + static_assert(std::is_const_v>, "src should has 'const' modifier"); + static_assert(std::is_convertible_v, "src type is not convertible to dest type"); + new (&dest) T(src); + } } -}; -template <> -struct CtorHelper -{ - template - 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 + 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, "src should be rvalue reference"); + dest = std::forward(src); + } + else + { + static_assert(std::is_lvalue_reference_v, "src should be lvalue reference"); + static_assert(std::is_const_v>, "src should has 'const' modifier"); + dest = src; + } } -}; - -template -struct MoveHelper; -template <> -struct MoveHelper -{ - template - 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 -{ - template - 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 -} -} diff --git a/iceoryx_dust/container/include/iox/fixed_position_container.hpp b/iceoryx_dust/container/include/iox/fixed_position_container.hpp index f32759f55a..733d0f3599 100644 --- a/iceoryx_dust/container/include/iox/fixed_position_container.hpp +++ b/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 @@ -315,7 +316,7 @@ class FixedPositionContainer final }; private: - template + template void copy_and_move_impl(RhsType&& rhs) noexcept; private: