Skip to content

Commit

Permalink
iox-eclipse-iceoryx#1614 Use struct for array alignment
Browse files Browse the repository at this point in the history 
Refactoring in UninitializedArray, variant and optional. Change gcc
version to 8 for ubuntu build.

Signed-off-by: Marika Lehmann <marika.lehmann@apex.ai>
  • Loading branch information
@FerdinandSpitzschnueffler
FerdinandSpitzschnueffler committed Nov 4, 2022
1 parent e988034 commit c31b000
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
- uses: actions/checkout@v3
- uses: egor-tensin/setup-gcc@v1
with:
version: 9
version: 8
platform: x64
- uses: jwlawson/actions-setup-cmake@v1.12
with:
Expand Down
26 changes: 16 additions & 10 deletions iceoryx_hoofs/container/include/iox/uninitialized_array.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,15 @@ namespace iox
template <typename ElementType, uint64_t Capacity>
struct ZeroedBuffer
{
struct alignas(ElementType) element_t
{
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
// NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
cxx::byte_t data[sizeof(ElementType)];
};
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
// NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
using element_t = cxx::byte_t[sizeof(ElementType)];
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
alignas(ElementType) element_t value[Capacity]{};
// NOLINTEND(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
// NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
element_t value[Capacity]{};
};

/// @brief struct used as policy parameter in UninitializedArray to wrap an uninitialized array
Expand All @@ -44,12 +47,15 @@ struct ZeroedBuffer
template <typename ElementType, uint64_t Capacity>
struct NonZeroedBuffer
{
struct alignas(ElementType) element_t
{
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
// NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
cxx::byte_t data[sizeof(ElementType)];
};
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
// NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
using element_t = cxx::byte_t[sizeof(ElementType)];
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : required by low level UninitializedArray building block and encapsulated in abstraction
alignas(ElementType) element_t value[Capacity];
// NOLINTEND(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
// NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays, hicpp-avoid-c-arrays)
element_t value[Capacity];
};

/// @brief Wrapper class for a C-style array of type ElementType and size Capacity. Per default it is uninitialized but
Expand Down
10 changes: 9 additions & 1 deletion iceoryx_hoofs/include/iceoryx_hoofs/cxx/optional.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "iceoryx_hoofs/cxx/functional_interface.hpp"
#include "iceoryx_hoofs/cxx/requires.hpp"
#include "iceoryx_hoofs/iceoryx_hoofs_types.hpp"

#include <new> // needed for placement new in the construct_value member function
#include <utility>
Expand Down Expand Up @@ -229,7 +230,14 @@ class optional final : public FunctionalInterface<optional<T>, T, void>
// initHandle(&handle);
// }
bool m_hasValue{false};
typename std::aligned_storage<sizeof(T), alignof(T)>::type m_data;
struct alignas(T) element_t
{
// AXIVION Next Construct AutosarC++19_03-A18.1.1 : safe access is guaranteed since the array
// is wrapped inside the optional
// NOLINTNEXTLINE(hicpp-avoid-c-arrays, cppcoreguidelines-avoid-c-arrays)
byte_t data[sizeof(T)];
};
element_t m_data;

private:
template <typename... Targs>
Expand Down
13 changes: 9 additions & 4 deletions iceoryx_hoofs/include/iceoryx_hoofs/cxx/variant.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class variant
/// @brief contains the size of the largest element
static constexpr uint64_t TYPE_SIZE = algorithm::maxVal(sizeof(Types)...);

static constexpr uint64_t TYPE_ALIGNMENT = algorithm::maxVal(alignof(Types)...);

public:
/// @brief the default constructor constructs a variant which does not contain
/// an element and returns INVALID_VARIANT_INDEX when .index() is called
Expand Down Expand Up @@ -265,10 +267,13 @@ class variant
constexpr uint64_t index() const noexcept;

private:
// NOLINTJUSTIFICATION safe access is guaranteed since the c-array is wrapped inside the variant class
// NOLINTBEGIN(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)
alignas(algorithm::maxVal(alignof(Types)...)) internal::byte_t m_storage[TYPE_SIZE]{0U};
// NOLINTEND(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)
struct alignas(TYPE_ALIGNMENT) storage_t
{
// NOLINTJUSTIFICATION safe access is guaranteed since the c-array is wrapped inside the variant class
// NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays)
internal::byte_t data[TYPE_SIZE];
};
storage_t m_storage{};
uint64_t m_type_index{INVALID_VARIANT_INDEX};

private:
Expand Down
28 changes: 15 additions & 13 deletions iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/variant.inl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ inline constexpr variant<Types...>::variant(const variant& rhs) noexcept
{
if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::copyConstructor(m_type_index, rhs.m_storage, m_storage);
internal::call_at_index<0, Types...>::copyConstructor(m_type_index, &rhs.m_storage.data[0], &m_storage.data[0]);
}
}

Expand Down Expand Up @@ -75,14 +75,15 @@ inline constexpr variant<Types...>& variant<Types...>::operator=(const variant&

if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::copyConstructor(m_type_index, rhs.m_storage, m_storage);
internal::call_at_index<0, Types...>::copyConstructor(
m_type_index, &rhs.m_storage.data[0], &m_storage.data[0]);
}
}
else
{
if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::copy(m_type_index, rhs.m_storage, m_storage);
internal::call_at_index<0, Types...>::copy(m_type_index, &rhs.m_storage.data[0], &m_storage.data[0]);
}
}
}
Expand All @@ -95,7 +96,7 @@ inline constexpr variant<Types...>::variant(variant&& rhs) noexcept
{
if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::moveConstructor(m_type_index, rhs.m_storage, m_storage);
internal::call_at_index<0, Types...>::moveConstructor(m_type_index, &rhs.m_storage.data[0], &m_storage.data[0]);
}
}

Expand All @@ -110,14 +111,15 @@ inline constexpr variant<Types...>& variant<Types...>::operator=(variant&& rhs)
m_type_index = std::move(rhs.m_type_index);
if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::moveConstructor(m_type_index, rhs.m_storage, m_storage);
internal::call_at_index<0, Types...>::moveConstructor(
m_type_index, &rhs.m_storage.data[0], &m_storage.data[0]);
}
}
else
{
if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::move(m_type_index, rhs.m_storage, m_storage);
internal::call_at_index<0, Types...>::move(m_type_index, &rhs.m_storage.data[0], &m_storage.data[0]);
}
}
}
Expand All @@ -135,7 +137,7 @@ inline void variant<Types...>::call_element_destructor() noexcept
{
if (m_type_index != INVALID_VARIANT_INDEX)
{
internal::call_at_index<0, Types...>::destructor(m_type_index, m_storage);
internal::call_at_index<0, Types...>::destructor(m_type_index, &m_storage.data[0]);
}
}

Expand All @@ -153,7 +155,7 @@ variant<Types...>::operator=(T&& rhs) noexcept

if (!has_bad_variant_element_access<T>())
{
auto storage = static_cast<T*>(static_cast<void*>(m_storage));
auto storage = static_cast<T*>(static_cast<void*>(&m_storage));
*storage = (std::forward<T>(rhs));
}
else
Expand All @@ -175,7 +177,7 @@ inline bool variant<Types...>::emplace_at_index(CTorArguments&&... args) noexcep
using T = typename internal::get_type_at_index<0, TypeIndex, Types...>::type;

call_element_destructor();
new (m_storage) T(std::forward<CTorArguments>(args)...);
new (&m_storage.data) T(std::forward<CTorArguments>(args)...);
m_type_index = TypeIndex;

return true;
Expand All @@ -198,7 +200,7 @@ inline bool variant<Types...>::emplace(CTorArguments&&... args) noexcept
call_element_destructor();
}

new (m_storage) T(std::forward<CTorArguments>(args)...);
new (&m_storage.data) T(std::forward<CTorArguments>(args)...);
m_type_index = internal::get_index_of_type<0, T, Types...>::index;

return true;
Expand All @@ -215,7 +217,7 @@ inline typename internal::get_type_at_index<0, TypeIndex, Types...>::type* varia

using T = typename internal::get_type_at_index<0, TypeIndex, Types...>::type;

return static_cast<T*>(static_cast<void*>(m_storage));
return static_cast<T*>(static_cast<void*>(&m_storage));
}

template <typename... Types>
Expand All @@ -239,7 +241,7 @@ inline const T* variant<Types...>::get() const noexcept
}
// AXIVION Next Construct AutosarC++19_03-A5.2.3 : avoid code duplication
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
return static_cast<const T*>(static_cast<const void*>(m_storage));
return static_cast<const T*>(static_cast<const void*>(&m_storage));
}

template <typename... Types>
Expand Down Expand Up @@ -309,7 +311,7 @@ inline constexpr bool operator==(const variant<Types...>& lhs, const variant<Typ
{
return false;
}
return internal::call_at_index<0, Types...>::equality(lhs.index(), lhs.m_storage, rhs.m_storage);
return internal::call_at_index<0, Types...>::equality(lhs.index(), &lhs.m_storage.data[0], &rhs.m_storage.data[0]);
}

template <typename... Types>
Expand Down

0 comments on commit c31b000

Please sign in to comment.