feat: Owning delegate (#1674)

This PR changes `Acts::Delegate` so it can **optionally** own it's payload (i.e. isn't copyable and destroys it correctly). The default remains to not own it. The memory footprint of a non-owning stays as before, the owning delegate gets an extra pointer to a deleter function. I added an alias `OwningDelegate`.

Core/include/Acts/Utilities/Delegate.hpp

@@ -12,49 +12,69 @@
 
 #include <cassert>
 #include <functional>
+#include <memory>
+#include <type_traits>
 
 namespace Acts {
 
-template <typename>
+/// Ownership enum for @c Delegate
+enum class DelegateType { Owning, NonOwning };
+
+// Specialization needed for defaulting ownership and for R(Args...) syntax
+template <typename, DelegateType = DelegateType::NonOwning>
 class Delegate;
 
-/// Delegate type that allows type erasure of a callable without allocation and
-/// with a single level of indirection.
-/// This type can support:
+/// Delegate type that allows type erasure of a callable without allocation
+/// and with a single level of indirection. This type can support:
 /// - a free function pointer
 /// - a pointer to a member function alongside an instance pointer
-/// @note @c Delegate does not assume ownership of the instance.
-///          You need to ensure that the lifetime of the callable
-///          instance is longer than that of the @c Delegate.
+/// @note @c Delegate by default does not assume ownership of the instance.
+///          In that case You need to ensure that the lifetime of the callable
+///          instance is longer than that of the @c Delegate. If you set @c O
+///          to @c DelegateType::Owning, it will assume ownership.
 /// @note Currently @c Delegate only supports callables that are ``const``
 /// @tparam R Return type of the function signature
+/// @tparam O Ownership type of the delegate: Owning or NonOwning
 /// @tparam Args Types of the arguments of the function signatures
 ///
-template <typename R, typename... Args>
-class Delegate<R(Args...)> {
+template <typename R, DelegateType O, typename... Args>
+class Delegate<R(Args...), O> {
+  static constexpr DelegateType kOwnership = O;
+
   /// Alias of the return type
   using return_type = R;
   /// Alias to the function pointer type this class will store
   using function_type = return_type (*)(const void *, Args...);
 
   using function_ptr_type = return_type (*)(Args...);
 
+  using deleter_type = void (*)(const void *);
+
   template <typename T, typename C>
   using isSignatureCompatible =
       decltype(std::declval<T &>() = std::declval<C>());
 
+  using OwningDelegate = Delegate<R(Args...), DelegateType::Owning>;
+  using NonOwningDelegate = Delegate<R(Args...), DelegateType::NonOwning>;
   template <typename T>
   using isNoFunPtr = std::enable_if_t<
-      not std::is_convertible_v<std::decay_t<T>, function_type>>;
+      not std::is_convertible_v<std::decay_t<T>, function_type> and
+      not std::is_same_v<std::decay_t<T>, OwningDelegate> and
+      not std::is_same_v<std::decay_t<T>, NonOwningDelegate>>;
 
  public:
   Delegate() = default;
 
+  Delegate(Delegate &&) = default;
+  Delegate &operator=(Delegate &&) = default;
+  Delegate(const Delegate &) = default;
+  Delegate &operator=(const Delegate &) = default;
+
   /// Constructor with an explicit runtime callable
   /// @param callable The runtime value of the callable
   /// @note The function signature requires the first argument of the callable is `const void*`.
-  ///       i.e. if the signature of the delegate is `void(int)`, the callable's
-  ///       signature has to be `void(const void*, int)`.
+  ///       i.e. if the signature of the delegate is `void(int)`, the
+  ///       callable's signature has to be `void(const void*, int)`.
   Delegate(function_type callable) { connect(callable); }
 
   /// Constructor with a possibly stateful function object.
@@ -75,8 +95,8 @@ class Delegate<R(Args...)> {
   /// Assignment operator with an explicit runtime callable
   /// @param callable The runtime value of the callable
   /// @note The function signature requires the first argument of the callable is `const void*`.
-  ///       i.e. if the signature of the delegate is `void(int)`, the callable's
-  ///       signature has to be `void(const void*, int)`.
+  ///       i.e. if the signature of the delegate is `void(int)`, the
+  ///       callable's signature has to be `void(const void*, int)`.
   void operator=(function_type callable) { connect(callable); }
 
   /// Assignment operator with possibly stateful function object.
@@ -99,7 +119,7 @@ class Delegate<R(Args...)> {
   /// @tparam Callable The compile-time free function pointer
   template <auto Callable>
   void connect() {
-    m_payload = nullptr;
+    m_payload.payload = nullptr;
 
     static_assert(
         Concepts::is_detected<isSignatureCompatible, function_ptr_type,
@@ -122,18 +142,20 @@ class Delegate<R(Args...)> {
     connect<&Callable::operator(), Callable>(&callable);
   }
 
-  /// Connection with rvalue reference is deleted, should catch assignment from
-  /// temporary objects and thus invalid pointers
+  /// Connection with rvalue reference is deleted, should catch assignment
+  /// from temporary objects and thus invalid pointers
   template <typename Callable, typename = isNoFunPtr<Callable>>
   void connect(Callable &&) = delete;
 
   /// Connect anything that is assignable to the function pointer
   /// @param callable The runtime value of the callable
   /// @note The function signature requires the first argument of the callable is `const void*`.
-  ///       i.e. if the signature of the delegate is `void(int)`, the callable's
-  ///       signature has to be `void(const void*, int)`.
+  ///       i.e. if the signature of the delegate is `void(int)`, the
+  ///       callable's signature has to be `void(const void*, int)`.
   void connect(function_type callable) {
-    m_payload = nullptr;
+    if constexpr (kOwnership == DelegateType::NonOwning) {
+      m_payload.payload = nullptr;
+    }
     m_function = callable;
   }
 
@@ -143,7 +165,8 @@ class Delegate<R(Args...)> {
   /// @param instance The instance on which the member function pointer should be called on
   /// @note @c Delegate does not assume owner ship over @p instance. You need to ensure
   ///       it's lifetime is longer than that of @c Delegate.
-  template <auto Callable, typename Type>
+  template <auto Callable, typename Type, DelegateType T = kOwnership,
+            typename = std::enable_if_t<T == DelegateType::NonOwning>>
   void connect(const Type *instance) {
     using member_ptr_type = return_type (Type::*)(Args...) const;
 
@@ -152,7 +175,39 @@ class Delegate<R(Args...)> {
                   "Callable given does not correspond exactly to required call "
                   "signature");
 
-    m_payload = instance;
+    m_payload.payload = instance;
+
+    m_function = [](const void *payload, Args... args) -> return_type {
+      assert(payload != nullptr && "Payload is required, but not set");
+      const auto *concretePayload = static_cast<const Type *>(payload);
+      return std::invoke(Callable, concretePayload,
+                         std::forward<Args>(args)...);
+    };
+  }
+
+  /// Connect a member function to be called on an instance
+  /// @tparam Callable The compile-time member function pointer
+  /// @tparam Type The type of the instance the member function should be called on
+  /// @param instance The instance on which the member function pointer should be called on
+  /// @note @c Delegate assumes owner ship over @p instance.
+  template <auto Callable, typename Type>
+  void connect(std::unique_ptr<const Type> instance) {
+    using member_ptr_type = return_type (Type::*)(Args...) const;
+    static_assert(Concepts::is_detected<isSignatureCompatible, member_ptr_type,
+                                        decltype(Callable)>::value,
+                  "Callable given does not correspond exactly to required call "
+                  "signature");
+
+    if constexpr (kOwnership == DelegateType::Owning) {
+      m_payload.payload = std::unique_ptr<const void, deleter_type>(
+          instance.release(), [](const void *payload) {
+            const auto *concretePayload = static_cast<const Type *>(payload);
+            delete concretePayload;
+          });
+    } else {
+      m_payload.payload = instance.release();
+    }
+
     m_function = [](const void *payload, Args... args) -> return_type {
       assert(payload != nullptr && "Payload is required, but not set");
       const auto *concretePayload = static_cast<const Type *>(payload);
@@ -166,7 +221,8 @@ class Delegate<R(Args...)> {
   /// @return Return value of the contained function
   return_type operator()(Args... args) const {
     assert(connected() && "Delegate is not connected");
-    return std::invoke(m_function, m_payload, std::forward<Args>(args)...);
+    return std::invoke(m_function, m_payload.ptr(),
+                       std::forward<Args>(args)...);
   }
 
   /// Return whether this delegate is currently connected
@@ -179,14 +235,51 @@ class Delegate<R(Args...)> {
 
   /// Disconnect this delegate, meaning it cannot be called anymore
   void disconnect() {
-    m_payload = nullptr;
+    m_payload.clear();
     m_function = nullptr;
   }
 
  private:
-  /// Stores the instance pointer
-  const void *m_payload{nullptr};
+  // Deleter that does not do anything
+  static void noopDeleter(const void *) {}
+
+  /// @cond
+
+  // Payload object without a deleter
+  struct NonOwningPayload {
+    void clear() { payload = nullptr; }
+
+    const void *ptr() const { return payload; }
+
+    const void *payload{nullptr};
+  };
+
+  // Payload object with a deleter
+  struct OwningPayload {
+    void clear() { payload.reset(); }
+
+    const void *ptr() const { return payload.get(); }
+
+    std::unique_ptr<const void, deleter_type> payload{nullptr, &noopDeleter};
+  };
+
+  /// Stores the instance pointer and maybe a deleter
+  std::conditional_t<kOwnership == DelegateType::NonOwning, NonOwningPayload,
+                     OwningPayload>
+      m_payload;
+
+  /// @endcond
+
   /// Stores the function pointer wrapping the compile time function pointer given in @c connect().
   function_type m_function{nullptr};
 };
+
+template <typename>
+class OwningDelegate;
+
+/// Alias for an owning delegate
+template <typename R, typename... Args>
+class OwningDelegate<R(Args...)>
+    : public Delegate<R(Args...), DelegateType::Owning> {};
+
 }  // namespace Acts

Tests/UnitTests/Core/Utilities/DelegateTests.cpp

@@ -12,6 +12,7 @@
 #include "Acts/Tests/CommonHelpers/FloatComparisons.hpp"
 #include "Acts/Utilities/Delegate.hpp"
 
+#include <memory>
 #include <numeric>
 #include <optional>
 #include <random>
@@ -199,4 +200,136 @@ BOOST_AUTO_TEST_CASE(StatefullLambdas) {
   // d.connect([&](int a){ v.push_back(a); return v.size(); });
 }
 
+struct CheckDestructor {
+  CheckDestructor(bool* _out) : destructorCalled{_out} {}
+
+  bool* destructorCalled;
+
+  int func() const { return 4; }
+
+  ~CheckDestructor() { (*destructorCalled) = true; }
+};
+
+int owningTest() {
+  return 8;
+}
+
+int owningTest2(const void*) {
+  return 8;
+}
+
+BOOST_AUTO_TEST_CASE(OwningDelegateTest) {
+  {
+    auto s = std::make_unique<const SignatureTest>();
+    Delegate<void(int&, int)> d;
+    d.connect<&SignatureTest::modify>(std::move(s));
+
+    int v = 0;
+    d(v, 42);
+    BOOST_CHECK_EQUAL(v, 42);
+  }
+
+  {
+    bool destructorCalled = false;
+    auto s = std::make_unique<const CheckDestructor>(&destructorCalled);
+    {
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      Delegate<int(), DelegateType::NonOwning> d;
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      d.connect<&CheckDestructor::func>(s.get());
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      Delegate<int(), DelegateType::NonOwning> dCopy{d};
+      BOOST_CHECK_EQUAL(d(), 4);
+      BOOST_CHECK_EQUAL(dCopy(), 4);
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+    }
+    // destructor not called after non-owning delegate goes out of scope
+    BOOST_CHECK_EQUAL(destructorCalled, false);
+
+    {
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      Delegate<int(), DelegateType::Owning> d;
+      // This doesn't compile: owning delegate is not copyable
+      // Delegate<int(), DelegateType::Owning> dCopy = d;
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      // This doesn't compile: owning delegate cannot accept raw pointer
+      // instance
+      // d.connect<&CheckDestructor::func>(s.get());
+      d.connect<&CheckDestructor::func>(std::move(s));
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      BOOST_CHECK_EQUAL(d(), 4);
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+    }
+    // destructor called after owning delegate goes out of scope
+    BOOST_CHECK_EQUAL(destructorCalled, true);
+
+    destructorCalled = false;
+    s = std::make_unique<const CheckDestructor>(&destructorCalled);
+    {
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      OwningDelegate<int()> d;
+      // This doesn't compile: owning delegate is not copyable
+      // OwningDelegate<int()> dCopy = d;
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      d.connect<&CheckDestructor::func>(std::move(s));
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      BOOST_CHECK_EQUAL(d(), 4);
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+    }
+    // destructor called after owning delegate goes out of scope
+    BOOST_CHECK_EQUAL(destructorCalled, true);
+  }
+
+  {
+    bool destructorCalled = false;
+    auto s = std::make_unique<const CheckDestructor>(&destructorCalled);
+    {
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      Delegate<int(), DelegateType::NonOwning> d;
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      d.connect<&CheckDestructor::func>(s.get());
+      Delegate<int(), DelegateType::NonOwning> dCopy{d};
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      BOOST_CHECK_EQUAL(d(), 4);
+      BOOST_CHECK_EQUAL(dCopy(), 4);
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      d.disconnect();
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+    }
+
+    {
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      Delegate<int(), DelegateType::Owning> d;
+      // This doesn't compile: owning delegate is not copyable
+      // Delegate<int(), DelegateType::Owning> dCopy = d;
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      // This doesn't compile: owning delegate cannot accept raw pointer
+      // instance
+      // d.connect<&CheckDestructor::func>(s.get());
+      d.connect<&CheckDestructor::func>(std::move(s));
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      BOOST_CHECK_EQUAL(d(), 4);
+      BOOST_CHECK_EQUAL(destructorCalled, false);
+      d.disconnect();
+      BOOST_CHECK_EQUAL(destructorCalled, true);
+    }
+    // destructor called after owning delegate goes out of scope
+    BOOST_CHECK_EQUAL(destructorCalled, true);
+  }
+
+  {
+    OwningDelegate<int()> d;
+    d.connect<&owningTest>();
+    BOOST_CHECK_EQUAL(d(), 8);
+
+    d.disconnect();
+    d.connect<&owningTest>();
+    BOOST_CHECK_EQUAL(d(), 8);
+
+    d.disconnect();
+    d.connect(owningTest2);
+    BOOST_CHECK_EQUAL(d(), 8);
+  }
+}
+
 BOOST_AUTO_TEST_SUITE_END()