iox-eclipse-iceoryx#1104 Remove nullability from cxx::function and in…

…tegrate it in hoofs

Signed-off-by: Christian Eltzschig <[email protected]>

iceoryx_hoofs/include/iceoryx_hoofs/cxx/function_ref.hpp

@@ -48,8 +48,6 @@ using has_same_decayed_type = typename std::
 ///         * Stateful lambda support
 ///         * C++11/14 support
 ///
-/// @attention Invoking an empty function_ref can lead to a program termination!
-///
 /// @code
 ///         // Usage as function parameter
 ///         void fuu(cxx::function_ref<void()> callback)
@@ -102,7 +100,6 @@ class function_ref<ReturnType(ArgTypes...)> final
     /// @brief Calls the provided callable
     /// @param[in] Arguments are forwarded to the underlying function pointer
     /// @return Returns the data type of the underlying function pointer
-    /// @attention Invoking an empty function_ref can lead to a program termination!
     ReturnType operator()(ArgTypes... args) const noexcept;
 
     /// @brief Swaps the contents of two function_ref's

iceoryx_hoofs/include/iceoryx_hoofs/cxx/scope_guard.hpp

@@ -73,7 +73,7 @@ class ScopeGuardWithVariableCapacity final
     void destroy() noexcept;
 
   private:
-    function<void(), CleanupCapacity> m_cleanupFunction;
+    cxx::optional<function<void(), CleanupCapacity>> m_cleanupFunction;
 };
 
 // This alias can be removed with C++17 and class template argument deduction

iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/scope_guard.inl

@@ -60,7 +60,7 @@ inline ScopeGuardWithVariableCapacity<CleanupCapacity>& ScopeGuardWithVariableCa
     {
         destroy();
         m_cleanupFunction = rhs.m_cleanupFunction;
-        rhs.m_cleanupFunction = function<void(), CleanupCapacity>();
+        rhs.m_cleanupFunction.reset();
     }
     return *this;
 }
@@ -70,8 +70,7 @@ inline void ScopeGuardWithVariableCapacity<CleanupCapacity>::destroy() noexcept
 {
     if (m_cleanupFunction)
     {
-        m_cleanupFunction();
-        m_cleanupFunction = function<void(), CleanupCapacity>();
+        m_cleanupFunction.value()();
     }
 }
 

iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/storable_function.hpp

@@ -54,8 +54,6 @@ class storable_function<StorageType, signature<ReturnType, Args...>>
   public:
     using signature_t = signature<ReturnType, Args...>;
 
-    storable_function() noexcept = default;
-
     /// @brief construct from functor (including lambdas)
     ///
     /// @note  Will not compile for StorageType = static_storage if the functor cannot be stored.
@@ -120,12 +118,7 @@ class storable_function<StorageType, signature<ReturnType, Args...>>
     ///          Not specifying move or using a default implementation is fine.
     ///          This is also the case for std::function (for the gcc implementation at least).
     ///
-    ReturnType operator()(Args... args) const;
-
-
-    /// @brief indicates whether a function is currently stored
-    /// @return true if a function is stored, false otherwise
-    explicit operator bool() const noexcept;
+    ReturnType operator()(Args... args) const noexcept;
 
     /// @brief swap this with another storable function
     /// @param f the function to swap this with
@@ -189,8 +182,6 @@ class storable_function<StorageType, signature<ReturnType, Args...>>
                                                  void>::type>
     void storeFunctor(const Functor& functor) noexcept;
 
-    bool empty() const noexcept;
-
     // we need these templates to preserve the actual CallableType for the underlying call
     template <typename CallableType>
     static void copy(const storable_function& src, storable_function& dest) noexcept;

iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/storable_function.inl

@@ -18,6 +18,7 @@
 #define IOX_HOOFS_STORABLE_FUNCTION_INL
 
 #include "iceoryx_hoofs/cxx/helplets.hpp"
+#include "iceoryx_hoofs/cxx/requires.hpp"
 #include "iceoryx_hoofs/internal/cxx/storable_function.hpp"
 
 namespace iox
@@ -33,18 +34,17 @@ storable_function<S, signature<ReturnType, Args...>>::storable_function(const Fu
 
 template <typename S, typename ReturnType, typename... Args>
 storable_function<S, signature<ReturnType, Args...>>::storable_function(ReturnType (*function)(Args...)) noexcept
+    : /// @NOLINTJUSTIFICATION we use type erasure in combination with compile time template arguments to restore
+      ///                      the correct type whenever the callable is used
+      /// @NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+    m_callable(reinterpret_cast<void*>(function))
+    , m_invoker(invokeFreeFunction)
 {
-    if (function)
-    {
-        m_invoker = invokeFreeFunction;
-        /// @NOLINTJUSTIFICATION we use type erasure in combination with compile time template arguments to restore
-        ///                      the correct type whenever the callable is used
-        /// @NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
-        m_callable = reinterpret_cast<void*>(function);
-        m_operations.copyFunction = copyFreeFunction;
-        m_operations.moveFunction = moveFreeFunction;
-        // destroy is not needed for free functions
-    }
+    cxx::Expects(function);
+
+    m_operations.copyFunction = copyFreeFunction;
+    m_operations.moveFunction = moveFreeFunction;
+    // destroy is not needed for free functions
 }
 
 template <typename S, typename ReturnType, typename... Args>
@@ -122,19 +122,11 @@ storable_function<S, signature<ReturnType, Args...>>::~storable_function() noexc
 }
 
 template <typename S, typename ReturnType, typename... Args>
-ReturnType storable_function<S, signature<ReturnType, Args...>>::operator()(Args... args) const
+ReturnType storable_function<S, signature<ReturnType, Args...>>::operator()(Args... args) const noexcept
 {
-    cxx::Expects(!empty());
     return m_invoker(m_callable, std::forward<Args>(args)...);
 }
 
-
-template <typename S, typename ReturnType, typename... Args>
-storable_function<S, signature<ReturnType, Args...>>::operator bool() const noexcept
-{
-    return !empty();
-}
-
 template <typename S, typename ReturnType, typename... Args>
 void storable_function<S, signature<ReturnType, Args...>>::swap(storable_function& f) noexcept
 {
@@ -149,12 +141,6 @@ void swap(storable_function<S, T>& f, storable_function<S, T>& g) noexcept
     f.swap(g);
 }
 
-template <typename S, typename ReturnType, typename... Args>
-bool storable_function<S, signature<ReturnType, Args...>>::empty() const noexcept
-{
-    return m_invoker == nullptr;
-}
-
 template <typename S, typename ReturnType, typename... Args>
 template <typename Functor, typename>
 void storable_function<S, signature<ReturnType, Args...>>::storeFunctor(const Functor& functor) noexcept
@@ -181,22 +167,13 @@ void storable_function<S, signature<ReturnType, Args...>>::storeFunctor(const Fu
         std::cerr << "storable_function: no memory to store functor\n";
         // this cannot happen in the static_storage case
     }
-
-    cxx::Ensures(!empty());
 }
 
 template <typename S, typename ReturnType, typename... Args>
 template <typename CallableType>
 void storable_function<S, signature<ReturnType, Args...>>::copy(const storable_function& src,
                                                                 storable_function& dest) noexcept
 {
-    if (src.empty())
-    {
-        // nothing to do, destroy and setting m_invoker to nullptr are performed before the type specific copy
-        // operation
-        return;
-    }
-
     auto ptr = dest.m_storage.template allocate<CallableType>();
 
     if (ptr)
@@ -214,22 +191,13 @@ void storable_function<S, signature<ReturnType, Args...>>::copy(const storable_f
         // no memory avilable in source, assignment could not be performed
         // (this cannot happen in the static_storage case)
     }
-
-    cxx::Ensures(!dest.empty());
 }
 
 template <typename S, typename ReturnType, typename... Args>
 template <typename CallableType>
 void storable_function<S, signature<ReturnType, Args...>>::move(storable_function& src,
                                                                 storable_function& dest) noexcept
 {
-    if (src.empty())
-    {
-        // nothing to do, destroy and setting m_invoker to nullptr are performed before the type specific move
-        // operation
-        return;
-    }
-
     auto ptr = dest.m_storage.template allocate<CallableType>();
     if (ptr)
     {
@@ -249,8 +217,6 @@ void storable_function<S, signature<ReturnType, Args...>>::move(storable_functio
         // (this cannot happen in the static_storage case)
         std::cerr << "storable_function: no memory to store moved object at destination\n";
     }
-
-    cxx::Ensures(!dest.empty());
 }
 
 template <typename S, typename ReturnType, typename... Args>

iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/unique_ptr.inl

@@ -39,16 +39,18 @@ inline unique_ptr<T>& unique_ptr<T>::operator=(unique_ptr&& rhs) noexcept
     {
         destroy();
         m_ptr = rhs.m_ptr;
-        m_deleter = std::move(rhs.m_deleter);
+        m_deleter = rhs.m_deleter;
         rhs.m_ptr = nullptr;
     }
     return *this;
 }
 
 template <typename T>
 inline unique_ptr<T>::unique_ptr(unique_ptr&& rhs) noexcept
+    : m_ptr{rhs.m_ptr}
+    , m_deleter{rhs.m_deleter}
 {
-    *this = std::move(rhs);
+    rhs.m_ptr = nullptr;
 }
 
 template <typename T>
@@ -105,7 +107,7 @@ inline void unique_ptr<T>::reset(T* const ptr) noexcept
 template <typename T>
 inline void unique_ptr<T>::destroy() noexcept
 {
-    if (m_ptr && m_deleter)
+    if (m_ptr)
     {
         m_deleter(m_ptr);
     }

iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/thread.hpp

@@ -39,7 +39,6 @@ ThreadName_t getThreadName(iox_pthread_t thread) noexcept;
 
 enum class ThreadError
 {
-    EMPTY_CALLABLE,
     INSUFFICIENT_MEMORY,
     INSUFFICIENT_PERMISSIONS,
     INSUFFICIENT_RESOURCES,
@@ -78,7 +77,7 @@ class Thread
     friend class cxx::optional<Thread>;
 
   private:
-    Thread() noexcept = default;
+    Thread(const ThreadName_t& name, const callable_t& callable) noexcept;
 
     static ThreadError errnoToEnum(const int errnoValue) noexcept;
 

iceoryx_hoofs/source/posix_wrapper/thread.cpp

@@ -55,16 +55,7 @@ ThreadName_t getThreadName(iox_pthread_t thread) noexcept
 cxx::expected<ThreadError> ThreadBuilder::create(cxx::optional<Thread>& uninitializedThread,
                                                  const Thread::callable_t& callable) noexcept
 {
-    if (!callable)
-    {
-        LogError() << "The thread cannot be created with an empty callable.";
-        return cxx::error<ThreadError>(ThreadError::EMPTY_CALLABLE);
-    }
-
-    uninitializedThread.emplace();
-    uninitializedThread->m_callable = callable;
-
-    uninitializedThread->m_threadName = m_name;
+    uninitializedThread.emplace(m_name, callable);
 
     const iox_pthread_attr_t* threadAttributes = nullptr;
 
@@ -83,6 +74,14 @@ cxx::expected<ThreadError> ThreadBuilder::create(cxx::optional<Thread>& uninitia
     return cxx::success<>();
 }
 
+Thread::Thread(const ThreadName_t& name, const callable_t& callable) noexcept
+    : m_threadHandle{}
+    , m_callable{callable}
+    , m_isThreadConstructed{false}
+    , m_threadName{name}
+{
+}
+
 Thread::~Thread() noexcept
 {
     if (m_isThreadConstructed)