Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow delegates to be created with weak reference + lambda #1372

Merged
merged 2 commits into from
Nov 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions cppwinrt/code_writers.h
Original file line number Diff line number Diff line change
Expand Up @@ -2488,9 +2488,9 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
template <typename F> %(F* function);
template <typename O, typename M> %(O* object, M method);
template <typename O, typename M> %(com_ptr<O>&& object, M method);
template <typename O, typename M> %(weak_ref<O>&& object, M method);
template <typename O, typename LM> %(weak_ref<O>&& object, LM&& lambda_or_method);
template <typename O, typename M> %(std::shared_ptr<O>&& object, M method);
template <typename O, typename M> %(std::weak_ptr<O>&& object, M method);
template <typename O, typename LM> %(std::weak_ptr<O>&& object, LM&& lambda_or_method);
auto operator()(%) const;
};
)";
Expand Down Expand Up @@ -2566,16 +2566,22 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
%([o = std::move(object), method](auto&&... args) { return ((*o).*(method))(args...); })
{
}
template <%> template <typename O, typename M> %<%>::%(weak_ref<O>&& object, M method) :
%([o = std::move(object), method](auto&&... args) { if (auto s = o.get()) { ((*s).*(method))(args...); } })
template <%> template <typename O, typename LM> %<%>::%(weak_ref<O>&& object, LM&& lambda_or_method) :
%([o = std::move(object), lm = std::forward<LM>(lambda_or_method)](auto&&... args) { if (auto s = o.get()) {
if constexpr (std::is_member_function_pointer_v<LM>) ((*s).*(lm))(args...);
else lm(args...);
} })
{
}
template <%> template <typename O, typename M> %<%>::%(std::shared_ptr<O>&& object, M method) :
%([o = std::move(object), method](auto&&... args) { return ((*o).*(method))(args...); })
{
}
template <%> template <typename O, typename M> %<%>::%(std::weak_ptr<O>&& object, M method) :
%([o = std::move(object), method](auto&&... args) { if (auto s = o.lock()) { ((*s).*(method))(args...); } })
template <%> template <typename O, typename LM> %<%>::%(std::weak_ptr<O>&& object, LM&& lambda_or_method) :
%([o = std::move(object), lm = std::forward<LM>(lambda_or_method)](auto&&... args) { if (auto s = o.lock()) {
if constexpr (std::is_member_function_pointer_v<LM>) ((*s).*(lm))(args...);
else lm(args...);
} })
{
}
template <%> auto %<%>::operator()(%) const
Expand Down Expand Up @@ -2652,16 +2658,22 @@ struct WINRT_IMPL_EMPTY_BASES produce_dispatch_to_overridable<T, D, %>
%([o = std::move(object), method](auto&&... args) { return ((*o).*(method))(args...); })
{
}
template <typename O, typename M> %::%(weak_ref<O>&& object, M method) :
%([o = std::move(object), method](auto&&... args) { if (auto s = o.get()) { ((*s).*(method))(args...); } })
template <typename O, typename LM> %::%(weak_ref<O>&& object, LM&& lambda_or_method) :
%([o = std::move(object), lm = std::forward<LM>(lambda_or_method)](auto&&... args) { if (auto s = o.get()) {
if constexpr (std::is_member_function_pointer_v<LM>) ((*s).*(lm))(args...);
else lm(args...);
} })
{
}
template <typename O, typename M> %::%(std::shared_ptr<O>&& object, M method) :
%([o = std::move(object), method](auto&&... args) { return ((*o).*(method))(args...); })
{
}
template <typename O, typename M> %::%(std::weak_ptr<O>&& object, M method) :
%([o = std::move(object), method](auto&&... args) { if (auto s = o.lock()) { ((*s).*(method))(args...); } })
template <typename O, typename LM> %::%(std::weak_ptr<O>&& object, LM&& lambda_or_method) :
%([o = std::move(object), lm = std::forward<LM>(lambda_or_method)](auto&&... args) { if (auto s = o.lock()) {
if constexpr (std::is_member_function_pointer_v<LM>) ((*s).*(lm))(args...);
else lm(args...);
} })
{
}
inline auto %::operator()(%) const
Expand Down
14 changes: 10 additions & 4 deletions strings/base_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,11 @@ namespace winrt::impl
{
}

template <typename O, typename M> delegate_base(winrt::weak_ref<O>&& object, M method) :
delegate_base([o = std::move(object), method](auto&& ... args) { if (auto s = o.get()) { ((*s).*(method))(args...); } })
template <typename O, typename LM> delegate_base(winrt::weak_ref<O>&& object, LM&& lambda_or_method) :
delegate_base([o = std::move(object), lm = std::forward<LM>(lambda_or_method)](auto&&... args) { if (auto s = o.get()) {
if constexpr (std::is_member_function_pointer_v<LM>) ((*s).*(lm))(args...);
else lm(args...);
}})
{
}

Expand All @@ -190,8 +193,11 @@ namespace winrt::impl
{
}

template <typename O, typename M> delegate_base(std::weak_ptr<O>&& object, M method) :
delegate_base([o = std::move(object), method](auto&& ... args) { if (auto s = o.lock()) { ((*s).*(method))(args...); } })
template <typename O, typename LM> delegate_base(std::weak_ptr<O>&& object, LM&& lambda_or_method) :
delegate_base([o = std::move(object), lm = std::forward<LM>(lambda_or_method)](auto&&... args) { if (auto s = o.lock()) {
if constexpr (std::is_member_function_pointer_v<LM>) ((*s).*(lm))(args...);
else lm(args...);
}})
{
}

Expand Down
146 changes: 74 additions & 72 deletions test/old_tests/UnitTests/delegate_weak_strong.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,85 @@ using namespace Windows::Foundation::Collections;

namespace
{
bool destroyed{};
int strong_count{};
int weak_count{};
struct Counters
{
bool destroyed{};
int strong_count{};
int weak_count{};
int weak_lambda_count{};

bool is_count(int value)
{
return strong_count == value && weak_count == value && weak_lambda_count == value;
}
};

template <typename Sender, typename Args>
struct Object : implements<Object<Sender, Args>, IInspectable>
{
std::shared_ptr<Counters> m_counters;

Object(std::shared_ptr<Counters> const& counters) : m_counters(counters) {}

static auto make(std::shared_ptr<Counters> const& counters)
{
return make_self<Object>(counters);
}

auto strong() { return this->get_strong(); }
auto weak() { return this->get_weak(); }

~Object()
{
destroyed = true;
m_counters->destroyed = true;
}

void StrongHandler(Sender const&, Args const&)
{
++strong_count;
REQUIRE(!m_counters->destroyed);
++m_counters->strong_count;
}

void WeakHandler(Sender const&, Args const&)
{
++weak_count;
REQUIRE(!m_counters->destroyed);
++m_counters->weak_count;
}
};

template <typename Sender, typename Args>
struct ObjectStd : std::enable_shared_from_this<ObjectStd<Sender, Args>>
{
std::shared_ptr<Counters> m_counters;

ObjectStd(std::shared_ptr<Counters> const& counters) : m_counters(counters) {}

static auto make(std::shared_ptr<Counters> const& counters)
{
return std::make_shared<ObjectStd>(counters);
}

auto strong() { return this->shared_from_this(); }
auto weak() { return this->weak_from_this(); }

~ObjectStd()
{
destroyed = true;
m_counters->destroyed = true;
}

void StrongHandler(Sender const&, Args const&)
{
++strong_count;
++m_counters->strong_count;
}

void WeakHandler(Sender const&, Args const&)
{
++weak_count;
++m_counters->weak_count;
}
};

struct ReturnObject : implements<ReturnObject, IInspectable>
{
~ReturnObject()
{
destroyed = true;
}

int Handler(int a, int b)
{
return a + b;
Expand All @@ -65,91 +95,63 @@ namespace

struct ReturnObjectStd : std::enable_shared_from_this<ReturnObjectStd>
{
~ReturnObjectStd()
{
destroyed = true;
}

int Handler(int a, int b)
{
return a + b;
}
};

template <typename Delegate, typename Sender, typename Args>
void test_delegate_winrt()
template <typename Recipient, typename Delegate>
void test_delegate_pattern()
{
auto object = make_self<Object<Sender, Args>>();

Delegate strong{ object->get_strong(), &Object<Sender, Args>::StrongHandler };
Delegate weak{ object->get_weak(), &Object<Sender, Args>::WeakHandler };
auto counters = std::make_shared<Counters>();
auto object = Recipient::make(counters);

destroyed = false;
strong_count = 0;
weak_count = 0;
Delegate strong{ object->strong(), &Recipient::StrongHandler};
Delegate weak{ object->weak(), &Recipient::WeakHandler };
Delegate weak_lambda{ object->weak(),[counters](auto&&, auto&&) {
REQUIRE(!counters->destroyed);
++counters->weak_lambda_count;
} };

// Both weak and strong handlers
// All handlers are active at this point
strong({}, {});
weak({}, {});
REQUIRE(strong_count == 1);
REQUIRE(weak_count == 1);
weak_lambda({}, {});
REQUIRE(counters->is_count(1));

// Local 'object' strong reference is released
object = nullptr;

// Still both since strong handler keeps object alive
// Still invoked since strong handler keeps object alive
strong({}, {});
weak({}, {});
REQUIRE(strong_count == 2);
REQUIRE(weak_count == 2);
weak_lambda({}, {});
REQUIRE(counters->is_count(2));

// ~Object is called since the strong delegate is destroyed
REQUIRE(!destroyed);
// ~Recipient is called since the strong delegate is destroyed
REQUIRE(!counters->destroyed);
strong = nullptr;
REQUIRE(destroyed);
REQUIRE(counters->destroyed);

// Weak delegate remains but no longer fires
REQUIRE(weak_count == 2);
// Strong delegate shouldn't fire either
REQUIRE(counters->is_count(2));
weak({}, {});
REQUIRE(weak_count == 2);
weak_lambda({}, {});
REQUIRE(counters->is_count(2));
}

template <typename Delegate, typename Sender, typename Args>
void test_delegate_std()
void test_delegate_winrt()
{
auto object = std::make_shared<ObjectStd<Sender, Args>>();

Delegate strong{ object->shared_from_this(), &ObjectStd<Sender, Args>::StrongHandler };
Delegate weak{ object->weak_from_this(), &ObjectStd<Sender, Args>::WeakHandler };

destroyed = false;
strong_count = 0;
weak_count = 0;

// Both weak and strong handlers
strong({}, {});
weak({}, {});
REQUIRE(strong_count == 1);
REQUIRE(weak_count == 1);

// Local 'object' strong reference is released
object = nullptr;

// Still both since strong handler keeps object alive
strong({}, {});
weak({}, {});
REQUIRE(strong_count == 2);
REQUIRE(weak_count == 2);

// ~Object is called since the strong delegate is destroyed
REQUIRE(!destroyed);
strong = nullptr;
REQUIRE(destroyed);
test_delegate_pattern<Object<Sender, Args>, Delegate>();
}

// Weak delegate remains but no longer fires
REQUIRE(weak_count == 2);
weak({}, {});
REQUIRE(weak_count == 2);
template <typename Delegate, typename Sender, typename Args>
void test_delegate_std()
{
test_delegate_pattern<ObjectStd<Sender, Args>, Delegate>();
}

template <typename Delegate, typename Sender, typename Args>
Expand Down
Loading