Skip to content

Commit

Permalink
Merge pull request eclipse-iceoryx#1607 from ApexAI/iox-#1394-fix-axi…
Browse files Browse the repository at this point in the history 
…vion-violation-for-optional_hpp_inl

Iox eclipse-iceoryx#1394 fix axivion violation for optional hpp inl
  • Loading branch information
@FerdinandSpitzschnueffler
FerdinandSpitzschnueffler authored Nov 1, 2022
2 parents e1e55dd + 34fb486 commit d4ea6f4
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 81 deletions.
86 changes: 61 additions & 25 deletions iceoryx_hoofs/include/iceoryx_hoofs/cxx/optional.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,17 @@ namespace cxx
struct nullopt_t
{
};
constexpr nullopt_t nullopt = nullopt_t();
// AXIVION Next Construct AutosarC++19_03-M17.0.2 : nullopt is defined within iox::cxx namespace which prevents easy
// misuse
constexpr nullopt_t nullopt{nullopt_t()};

/// @brief helper struct which is used to call the in-place-construction constructor
struct in_place_t
{
};

// AXIVION Next Construct AutosarC++19_03-M17.0.2 : in_place is defined within iox::cxx namespace which prevents easy
// misuse
constexpr in_place_t in_place{};

/// @brief Optional implementation from the C++17 standard with C++11. The
Expand All @@ -65,7 +70,7 @@ constexpr in_place_t in_place{};
/// }
/// @endcode
template <typename T>
class optional : public FunctionalInterface<optional<T>, T, void>
class optional final : public FunctionalInterface<optional<T>, T, void>
{
public:
using type = T;
Expand All @@ -79,7 +84,7 @@ class optional : public FunctionalInterface<optional<T>, T, void>
/// optional via .value() or the arrow operator the application
/// terminates.
// NOLINTNEXTLINE(hicpp-explicit-conversions) for justification see doxygen
optional(const nullopt_t& noValue) noexcept;
optional(const nullopt_t) noexcept;

/// @brief Creates an optional by forwarding value to the constructor of
/// T. This optional has a value.
Expand Down Expand Up @@ -128,26 +133,6 @@ class optional : public FunctionalInterface<optional<T>, T, void>
/// @return reference to the current optional
optional& operator=(optional&& rhs) noexcept;

/// @brief If the optionals have values it compares these values by using
/// their comparison operator.
/// @param[in] rhs value to which this optional should be compared to
/// @return true if the contained values are equal, otherwise false
constexpr bool operator==(const optional<T>& rhs) const noexcept;

/// @brief Comparison with nullopt_t for easier unset optional comparison
/// @return true if the optional is unset, otherwise false
constexpr bool operator==(const nullopt_t& rhs) const noexcept;

/// @brief If the optionals have values it compares these values by using
/// their comparison operator.
/// @param[in] rhs value to which this optional should be compared to
/// @return true if the contained values are not equal, otherwise false
constexpr bool operator!=(const optional<T>& rhs) const noexcept;

/// @brief comparison with nullopt_t for easier unset optional comparison
/// @return true if the optional is set, otherwise false
constexpr bool operator!=(const nullopt_t& rhs) const noexcept;

/// @brief Direct assignment of the underlying value. If the optional has no
/// value then a new T is constructed by forwarding the assignment to
/// T's constructor.
Expand All @@ -156,7 +141,7 @@ class optional : public FunctionalInterface<optional<T>, T, void>
/// @return reference to the current optional
template <typename U = T>
// NOLINTNEXTLINE(cppcoreguidelines-c-copy-assignment-signature) return type is optional&
typename std::enable_if<!std::is_same<U, optional<T>&>::value, optional>::type& operator=(U&& value) noexcept;
typename std::enable_if<!std::is_same<U, optional<T>&>::value, optional>::type& operator=(U&& newValue) noexcept;

/// @brief Returns a pointer to the underlying value. If the optional has no
/// value the application terminates. You need to verify that the
Expand All @@ -182,6 +167,9 @@ class optional : public FunctionalInterface<optional<T>, T, void>
/// @return reference of type T to the underlying type
T& operator*() noexcept;

// AXIVION Next Construct AutosarC++19_03-A13.5.3: Implemented to be as close to the STL interface as possible.
// In combination with the keyword explicit accidental casts can be excluded and the usage is well known in the
// C++ community.
/// @brief Will return true if the optional contains a value, otherwise false.
/// @return true if optional contains a value, otherwise false
constexpr explicit operator bool() const noexcept;
Expand Down Expand Up @@ -242,7 +230,10 @@ class optional : public FunctionalInterface<optional<T>, T, void>
// initHandle(&handle);
// }
bool m_hasValue{false};
// safe access is guaranteed since the array is wrapped inside the optional

private:
// 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)
alignas(T) byte_t m_data[sizeof(T)];

Expand All @@ -252,13 +243,58 @@ class optional : public FunctionalInterface<optional<T>, T, void>
void destruct_value() noexcept;
};

// AXIVION Next Construct AutosarC++19_03-M17.0.3 : make_optional is defined within iox::cxx which prevents easy misuse
/// @brief Creates an optional which contains a value by forwarding the arguments
/// to the constructor of T.
/// @param[in] args arguments which will be perfectly forwarded to the constructor
/// of T
/// @return optional which contains T constructed with args
template <typename OptionalBaseType, typename... Targs>
optional<OptionalBaseType> make_optional(Targs&&... args) noexcept;

/// @brief Compare two optionals for equality.
/// @param[in] lhs cxx::optional
/// @param[in] rhs optional to which lhs should be compared to
/// @return true if the contained values are equal or both have no value, otherwise false
template <typename T>
bool operator==(const optional<T>& lhs, const optional<T>& rhs) noexcept;

/// @brief Compare two optionals for inequality.
/// @param[in] lhs cxx::optional
/// @param[in] rhs optional to which lhs should be compared to
/// @return true if the contained values are not equal, otherwise false
template <typename T>
bool operator!=(const optional<T>& lhs, const optional<T>& rhs) noexcept;

// AXIVION DISABLE STYLE AutosarC++19_03-A13.5.5: Comparison with nullopt_t is required
/// @brief Comparison for equality with nullopt_t for easier unset optional comparison
/// @param[in] lhs empty optional, cxx::nullopt_t
/// @param[in] rhs optional to which lhs should be compared to
/// @return true if the rhs is not set, otherwise false
template <typename T>
bool operator==(const nullopt_t, const optional<T>& rhs) noexcept;

/// @brief Comparison for equality with nullopt_t for easier unset optional comparison
/// @param[in] lhs optional which should be compared to cxx::nullopt_t
/// @param[in] rhs empty optional
/// @return true if the lhs is not set, otherwise false
template <typename T>
bool operator==(const optional<T>& lhs, const nullopt_t) noexcept;

/// @brief Comparison for inequality with nullopt_t for easier unset optional comparison
/// @param[in] lhs empty optional, cxx::nullopt_t
/// @param[in] rhs optional to which lhs should be compared to
/// @return true if the optional is set, otherwise false
template <typename T>
bool operator!=(const nullopt_t, const optional<T>& rhs) noexcept;

/// @brief Comparison for inequality with nullopt_t for easier unset optional comparison
/// @param[in] lhs optional which should be compared to cxx::nullopt_t
/// @param[in] rhs empty optional
/// @return true if the optional is set, otherwise false
template <typename T>
bool operator!=(const optional<T>& lhs, const nullopt_t) noexcept;
// AXIVION ENABLE STYLE AutosarC++19_03-A13.5.5
} // namespace cxx
} // namespace iox

Expand Down
118 changes: 74 additions & 44 deletions iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/optional.inl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ namespace iox
{
namespace cxx
{
// m_data is not initialized since this is a constructor for an optional with no value; an access of the value would
// lead to the application's termination
// AXIVION DISABLE STYLE AutosarC++19_03-A12.6.1 : m_data is not initialized here, since this is a
// constructor for an optional with no value; an access of the value would lead to the
// application's termination
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init, hicpp-member-init)
template <typename T>
inline optional<T>::optional(const nullopt_t& noVal IOX_MAYBE_UNUSED) noexcept
inline optional<T>::optional(const nullopt_t) noexcept
{
}

Expand All @@ -46,6 +47,8 @@ inline optional<T>::optional() noexcept
template <typename T>
inline optional<T>::optional(T&& value) noexcept
{
// AXIVION Next Construct AutosarC++19_03-A18.9.2 : Perfect forwarding is intended here and
// std::forward is the idiomatic way for this
construct_value(std::forward<T>(value));
}

Expand Down Expand Up @@ -89,7 +92,10 @@ inline optional<T>::optional(in_place_t, Targs&&... args) noexcept
{
construct_value(std::forward<Targs>(args)...);
}
// AXIVION ENABLE STYLE AutosarC++19_03-A12.6.1

// AXIVION Next Construct AutosarC++19_03-A13.3.1 : False positive. Overloading excluded via std::enable_if in
// typename std::enable_if<!std::is_same<U, optional&>::value, optional>::type& operator=(U&& value) noexcept.
template <typename T>
inline optional<T>& optional<T>::operator=(const optional& rhs) noexcept
{
Expand All @@ -107,10 +113,16 @@ inline optional<T>& optional<T>::operator=(const optional& rhs) noexcept
{
construct_value(rhs.value());
}
else
{
// do nothing since this and rhs contain no values
}
}
return *this;
}

// AXIVION Next Construct AutosarC++19_03-A13.3.1 : False positive. Overloading excluded via std::enable_if in typename
// std::enable_if<!std::is_same<U, optional&>::value, optional>::type& operator=(U&& value) noexcept.
template <typename T>
inline optional<T>& optional<T>::operator=(optional&& rhs) noexcept
{
Expand All @@ -128,6 +140,10 @@ inline optional<T>& optional<T>::operator=(optional&& rhs) noexcept
{
construct_value(std::move(rhs.value()));
}
else
{
// do nothing since this and rhs contain no values
}
if (rhs.m_hasValue)
{
rhs.destruct_value();
Expand All @@ -153,51 +169,15 @@ optional<T>::operator=(U&& newValue) noexcept
{
if (m_hasValue)
{
/// @todo iox-#1694 broken msvc compiler, see:
/// https://developercommunity.visualstudio.com/content/problem/858688/stdforward-none-of-these-2-overloads-could-convert.html
/// remove this as soon as it is fixed;
#ifdef _WIN32
value() = newValue;
#else
value() = std::forward<T>(newValue);
#endif
}
else
{
/// @todo iox-#1694 again broken msvc compiler
#ifdef _WIN32
construct_value(newValue);
#else
construct_value(std::forward<T>(newValue));
#endif
}
return *this;
}

template <typename T>
constexpr inline bool optional<T>::operator==(const optional<T>& rhs) const noexcept
{
return (!m_hasValue && !rhs.m_hasValue) || ((m_hasValue && rhs.m_hasValue) && (value() == rhs.value()));
}

template <typename T>
constexpr inline bool optional<T>::operator==(const nullopt_t& rhs IOX_MAYBE_UNUSED) const noexcept
{
return !m_hasValue;
}

template <typename T>
constexpr inline bool optional<T>::operator!=(const optional<T>& rhs) const noexcept
{
return !(*this == rhs);
}

template <typename T>
constexpr inline bool optional<T>::operator!=(const nullopt_t& rhs) const noexcept
{
return !(*this == rhs);
}

template <typename T>
inline const T* optional<T>::operator->() const noexcept
{
Expand Down Expand Up @@ -260,35 +240,45 @@ template <typename T>
inline T& optional<T>::value() & noexcept
{
Expects(has_value());
// AXIVION Next Construct AutosarC++19_03-M5.2.8 : The optional has the type T defined
// during compile time and the type is unchangeable during the lifetime of the object.
// All accesses to the underlying data is done via the same static type and therefore the
// casts are always valid
return *static_cast<T*>(static_cast<void*>(&m_data));
}

template <typename T>
inline const T& optional<T>::value() const& noexcept
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) const_cast to avoid code duplication
// AXIVION Next Construct AutosarC++19_03-A5.2.3 : Avoid code duplication
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
return const_cast<optional<T>*>(this)->value();
}

template <typename T>
inline T&& optional<T>::value() && noexcept
{
Expects(has_value());
// AXIVION Next Construct AutosarC++19_03-M5.2.8 : The optional has the type T defined
// during compile time and the type is unchangeable during the lifetime of the object.
// All accesses to the underlying data is done via the same static type and therefore the
// casts are always valid
return std::move(*static_cast<T*>(static_cast<void*>(&m_data)));
}

template <typename T>
inline const T&& optional<T>::value() const&& noexcept
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) const_cast to avoid code duplication
// AXIVION Next Construct AutosarC++19_03-A5.2.3 : Avoid code duplication
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
return std::move(*const_cast<optional<T>*>(this)->value());
}

template <typename T>
template <typename... Targs>
inline void optional<T>::construct_value(Targs&&... args) noexcept
{
new (static_cast<T*>(static_cast<void*>(&m_data))) T(std::forward<Targs>(args)...);
new (&m_data) T(std::forward<Targs>(args)...);
m_hasValue = true;
}

Expand All @@ -298,14 +288,54 @@ inline void optional<T>::destruct_value() noexcept
value().~T();
m_hasValue = false;
}

// AXIVION Next Construct AutosarC++19_03-M17.0.3 : make_optional is defined within iox::cxx which prevents easy misuse
template <typename OptionalBaseType, typename... Targs>
inline optional<OptionalBaseType> make_optional(Targs&&... args) noexcept
{
optional<OptionalBaseType> returnValue = nullopt_t();
optional<OptionalBaseType> returnValue{nullopt_t()};
returnValue.emplace(std::forward<Targs>(args)...);
return returnValue;
}

template <typename T>
bool operator==(const optional<T>& lhs, const optional<T>& rhs) noexcept
{
const auto bothNull = !lhs.has_value() && !rhs.has_value();
const auto bothValuesEqual = (lhs.has_value() && rhs.has_value()) && (*lhs == *rhs);
return bothNull || bothValuesEqual;
}

template <typename T>
bool operator!=(const optional<T>& lhs, const optional<T>& rhs) noexcept
{
return !(lhs == rhs);
}

// AXIVION DISABLE STYLE AutosarC++19_03-A13.5.5: Comparison with nullopt_t is required
template <typename T>
bool operator==(const optional<T>& lhs, const nullopt_t) noexcept
{
return !lhs.has_value();
}

template <typename T>
bool operator==(const nullopt_t, const optional<T>& rhs) noexcept
{
return !rhs.has_value();
}

template <typename T>
bool operator!=(const optional<T>& lhs, const nullopt_t) noexcept
{
return lhs.has_value();
}

template <typename T>
bool operator!=(const nullopt_t, const optional<T>& rhs) noexcept
{
return rhs.has_value();
}
// AXIVION ENABLE STYLE AutosarC++19_03-A13.5.5
} // namespace cxx
} // namespace iox

Expand Down
8 changes: 4 additions & 4 deletions iceoryx_hoofs/test/mocktests/test_error_handler_mock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ TEST(ErrorHandlerMock_test, CallingErrorHandlerWithErrorOfKnownModuleAndDefaultL
iox::errorHandler(KnownError::TEST__FOOBAR);

ASSERT_TRUE(detectedError.has_value());
EXPECT_THAT(detectedError, Eq(KnownError::TEST__FOOBAR));
EXPECT_THAT(detectedError.value(), Eq(KnownError::TEST__FOOBAR));
ASSERT_TRUE(detectedLevel.has_value());
EXPECT_THAT(detectedLevel, Eq(iox::ErrorLevel::FATAL));
EXPECT_THAT(detectedLevel.value(), Eq(iox::ErrorLevel::FATAL));
}

TEST(ErrorHandlerMock_test, CallingErrorHandlerWithErrorOfKnownModuleAndNonDefaultLevelIsCaught)
Expand All @@ -99,9 +99,9 @@ TEST(ErrorHandlerMock_test, CallingErrorHandlerWithErrorOfKnownModuleAndNonDefau
iox::errorHandler(KnownError::TEST__FOOBAR, iox::ErrorLevel::MODERATE);

ASSERT_TRUE(detectedError.has_value());
EXPECT_THAT(detectedError, Eq(KnownError::TEST__FOOBAR));
EXPECT_THAT(detectedError.value(), Eq(KnownError::TEST__FOOBAR));
ASSERT_TRUE(detectedLevel.has_value());
EXPECT_THAT(detectedLevel, Eq(iox::ErrorLevel::MODERATE));
EXPECT_THAT(detectedLevel.value(), Eq(iox::ErrorLevel::MODERATE));
}

TEST(ErrorHandlerMock_test, CallingErrorHandlerWithErrorOfUnknownModuleCallsGTestFail)
Expand Down
Loading

0 comments on commit d4ea6f4

Please sign in to comment.