Skip to content

Commit

Permalink
feat: Allow delegate to hold non-void instance (#1675)
Browse files Browse the repository at this point in the history 
In case the holder type is not void, the delegate exposes the instance
as a pointer so you can downcast it if desired.
  • Loading branch information
@paulgessinger
paulgessinger authored Nov 17, 2022
1 parent 7da4880 commit 22ee1e9
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 26 deletions.
52 changes: 32 additions & 20 deletions Core/include/Acts/Utilities/Delegate.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace Acts {
enum class DelegateType { Owning, NonOwning };

// Specialization needed for defaulting ownership and for R(Args...) syntax
template <typename, DelegateType = DelegateType::NonOwning>
template <typename, typename H = void, DelegateType = DelegateType::NonOwning>
class Delegate;

/// Delegate type that allows type erasure of a callable without allocation
Expand All @@ -34,17 +34,19 @@ class Delegate;
/// 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 H Holder type that is used to store an instance
/// @tparam O Ownership type of the delegate: Owning or NonOwning
/// @tparam Args Types of the arguments of the function signatures
///
template <typename R, DelegateType O, typename... Args>
class Delegate<R(Args...), O> {
template <typename R, typename H, DelegateType O, typename... Args>
class Delegate<R(Args...), H, O> {
static constexpr DelegateType kOwnership = O;

/// Alias of the return type
using return_type = R;
using holder_type = H;
/// Alias to the function pointer type this class will store
using function_type = return_type (*)(const void *, Args...);
using function_type = return_type (*)(const holder_type *, Args...);

using function_ptr_type = return_type (*)(Args...);

Expand All @@ -54,8 +56,10 @@ class Delegate<R(Args...), O> {
using isSignatureCompatible =
decltype(std::declval<T &>() = std::declval<C>());

using OwningDelegate = Delegate<R(Args...), DelegateType::Owning>;
using NonOwningDelegate = Delegate<R(Args...), DelegateType::NonOwning>;
using OwningDelegate =
Delegate<R(Args...), holder_type, DelegateType::Owning>;
using NonOwningDelegate =
Delegate<R(Args...), holder_type, DelegateType::NonOwning>;
template <typename T>
using isNoFunPtr = std::enable_if_t<
not std::is_convertible_v<std::decay_t<T>, function_type> and
Expand Down Expand Up @@ -127,7 +131,8 @@ class Delegate<R(Args...), O> {
"Callable given does not correspond exactly to required call "
"signature");

m_function = [](const void * /*payload*/, Args... args) -> return_type {
m_function = [](const holder_type * /*payload*/,
Args... args) -> return_type {
return std::invoke(Callable, std::forward<Args>(args)...);
};
}
Expand Down Expand Up @@ -177,7 +182,7 @@ class Delegate<R(Args...), O> {

m_payload.payload = instance;

m_function = [](const void *payload, Args... args) -> return_type {
m_function = [](const holder_type *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,
Expand All @@ -199,16 +204,16 @@ class Delegate<R(Args...), O> {
"signature");

if constexpr (kOwnership == DelegateType::Owning) {
m_payload.payload = std::unique_ptr<const void, deleter_type>(
instance.release(), [](const void *payload) {
m_payload.payload = std::unique_ptr<const holder_type, deleter_type>(
instance.release(), [](const holder_type *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 {
m_function = [](const holder_type *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,
Expand Down Expand Up @@ -239,28 +244,35 @@ class Delegate<R(Args...), O> {
m_function = nullptr;
}

template <typename holder_t = holder_type,
typename = std::enable_if_t<!std::is_same_v<holder_t, void>>>
const holder_type *instance() const {
return m_payload.ptr();
}

private:
// Deleter that does not do anything
static void noopDeleter(const void * /*unused*/) {}
static void noopDeleter(const holder_type * /*unused*/) {}

/// @cond

// Payload object without a deleter
struct NonOwningPayload {
void clear() { payload = nullptr; }

const void *ptr() const { return payload; }
const holder_type *ptr() const { return payload; }

const void *payload{nullptr};
const holder_type *payload{nullptr};
};

// Payload object with a deleter
struct OwningPayload {
void clear() { payload.reset(); }

const void *ptr() const { return payload.get(); }
const holder_type *ptr() const { return payload.get(); }

std::unique_ptr<const void, deleter_type> payload{nullptr, &noopDeleter};
std::unique_ptr<const holder_type, deleter_type> payload{nullptr,
&noopDeleter};
};

/// Stores the instance pointer and maybe a deleter
Expand All @@ -274,12 +286,12 @@ class Delegate<R(Args...), O> {
function_type m_function{nullptr};
};

template <typename>
template <typename, typename H = void>
class OwningDelegate;

/// Alias for an owning delegate
template <typename R, typename... Args>
class OwningDelegate<R(Args...)>
: public Delegate<R(Args...), DelegateType::Owning> {};
template <typename R, typename H, typename... Args>
class OwningDelegate<R(Args...), H>
: public Delegate<R(Args...), H, DelegateType::Owning> {};

} // namespace Acts
89 changes: 83 additions & 6 deletions Tests/UnitTests/Core/Utilities/DelegateTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <optional>
#include <random>
#include <tuple>
#include <type_traits>

using namespace Acts;

Expand Down Expand Up @@ -234,11 +235,11 @@ BOOST_AUTO_TEST_CASE(OwningDelegateTest) {
auto s = std::make_unique<const CheckDestructor>(&destructorCalled);
{
BOOST_CHECK_EQUAL(destructorCalled, false);
Delegate<int(), DelegateType::NonOwning> d;
Delegate<int(), void, 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};
Delegate<int(), void, DelegateType::NonOwning> dCopy{d};
BOOST_CHECK_EQUAL(d(), 4);
BOOST_CHECK_EQUAL(dCopy(), 4);
BOOST_CHECK_EQUAL(destructorCalled, false);
Expand All @@ -248,7 +249,7 @@ BOOST_AUTO_TEST_CASE(OwningDelegateTest) {

{
BOOST_CHECK_EQUAL(destructorCalled, false);
Delegate<int(), DelegateType::Owning> d;
Delegate<int(), void, DelegateType::Owning> d;
// This doesn't compile: owning delegate is not copyable
// Delegate<int(), DelegateType::Owning> dCopy = d;
BOOST_CHECK_EQUAL(destructorCalled, false);
Expand Down Expand Up @@ -285,10 +286,10 @@ BOOST_AUTO_TEST_CASE(OwningDelegateTest) {
auto s = std::make_unique<const CheckDestructor>(&destructorCalled);
{
BOOST_CHECK_EQUAL(destructorCalled, false);
Delegate<int(), DelegateType::NonOwning> d;
Delegate<int(), void, DelegateType::NonOwning> d;
BOOST_CHECK_EQUAL(destructorCalled, false);
d.connect<&CheckDestructor::func>(s.get());
Delegate<int(), DelegateType::NonOwning> dCopy{d};
Delegate<int(), void, DelegateType::NonOwning> dCopy{d};
BOOST_CHECK_EQUAL(destructorCalled, false);
BOOST_CHECK_EQUAL(d(), 4);
BOOST_CHECK_EQUAL(dCopy(), 4);
Expand All @@ -299,7 +300,7 @@ BOOST_AUTO_TEST_CASE(OwningDelegateTest) {

{
BOOST_CHECK_EQUAL(destructorCalled, false);
Delegate<int(), DelegateType::Owning> d;
Delegate<int(), void, DelegateType::Owning> d;
// This doesn't compile: owning delegate is not copyable
// Delegate<int(), DelegateType::Owning> dCopy = d;
BOOST_CHECK_EQUAL(destructorCalled, false);
Expand Down Expand Up @@ -332,4 +333,80 @@ BOOST_AUTO_TEST_CASE(OwningDelegateTest) {
}
}

struct DelegateInterface {
virtual ~DelegateInterface() = 0;

virtual std::string func() const { return "base"; }
};
inline DelegateInterface::~DelegateInterface() = default;

struct ConcreteDelegate : public DelegateInterface {
std::string func() const final { return "derived"; }
};

struct SeparateDelegate {
std::string func() const { return "separate"; }
};

BOOST_AUTO_TEST_CASE(NonVoidDelegateTest) {
// check void behavior with virtuals
{
Delegate<std::string(), void> d;
ConcreteDelegate c;
d.connect<&ConcreteDelegate::func>(&c);
BOOST_CHECK_EQUAL(d(), "derived");

// does not compile: delegate won't hand out void pointer
// d.instance();
}
{
Delegate<std::string(), void> d;
ConcreteDelegate c;
d.connect<&DelegateInterface::func>(&c);
BOOST_CHECK_EQUAL(
d(), "derived"); // <- even if you plug in the base class member
// pointer you get the derived class call
}

{
Delegate<std::string(), DelegateInterface> d;
ConcreteDelegate c;
d.connect<&ConcreteDelegate::func>(&c);
BOOST_CHECK_EQUAL(d(), "derived");

const auto* instance = d.instance();
static_assert(
std::is_same_v<
std::remove_const_t<std::remove_pointer_t<decltype(instance)>>,
DelegateInterface>,
"Did not get correct instance pointer");
BOOST_CHECK_NE(dynamic_cast<const DelegateInterface*>(instance), nullptr);
BOOST_CHECK_NE(dynamic_cast<const ConcreteDelegate*>(instance), nullptr);
}
{
Delegate<std::string(), ConcreteDelegate> d;
ConcreteDelegate c;
d.connect<&ConcreteDelegate::func>(&c);
BOOST_CHECK_EQUAL(d(), "derived");

const auto* instance = d.instance();
static_assert(
std::is_same_v<
std::remove_const_t<std::remove_pointer_t<decltype(instance)>>,
ConcreteDelegate>,
"Did not get correct instance pointer");
BOOST_CHECK_NE(dynamic_cast<const DelegateInterface*>(instance), nullptr);
BOOST_CHECK_NE(dynamic_cast<const ConcreteDelegate*>(instance), nullptr);
}

{
Delegate<std::string(), DelegateInterface> d;
SeparateDelegate c;
// Does not compile: cannot assign unrelated type
// d.connect<&SeparateDelegate::func>(&c);
(void)d;
(void)c;
}
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 22ee1e9

Please sign in to comment.