Skip to content

Commit

Permalink
Merge pull request #2105 from Dennis40816/iox-2052-refactor-copy_and_…
Browse files Browse the repository at this point in the history 
…move_impl

iox-#2052 Refactor copy_and_move_impl
  • Loading branch information
@elBoberido
elBoberido authored Nov 22, 2023
2 parents c07fa9f + 7bd507e commit 5872051
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 99 deletions.
83 changes: 36 additions & 47 deletions iceoryx_dust/container/include/iox/detail/fixed_position_container.inl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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>
Expand All @@ -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;
}
Expand All @@ -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)
Expand All @@ -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())
{
Expand All @@ -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>
Expand Down
126 changes: 75 additions & 51 deletions iceoryx_dust/container/include/iox/detail/fixed_position_container_helper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
3 changes: 2 additions & 1 deletion iceoryx_dust/container/include/iox/fixed_position_container.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 5872051

Please sign in to comment.