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

We have found that a very common pattern for event handlers is to capture a weak reference into a lambda, and in the event handler, try to upgrade the weak reference to a strong one, and if so, do some work: ```cpp widget.Closed([weak = get_weak(), data](auto&& sender, auto&& args) { if (auto strongThis = weak.get()) { strongThis->do_all_the_things(data); } }); ``` This commit extends the existing delegate constructors to permit a `winrt::weak_ref` + lambda (or `std::weak_ptr` + lambda), which simplifies the above to ```cpp widget.Closed({ get_weak(), [this, data](auto&& sender, auto&& args) { do_all_the_things(data); } }); ``` ## Implementation notes A lambda and pointer to member function are hard to distinguish in a template parameter list. In theory, we could use SFINAE or partial specialization, but a simpler solution is to distinguish the two inside the body of the constructor, via `std::is_member_function_pointer_v`. The `com_ptr` and `shared_ptr` variants of the test were unified, since I found myself editing two nearly identical tests. Fixes #1371 Co-authored-by: Jon Wiswall <[email protected]>
microsoft · Nov 26, 2023 · fc587f3 · fc587f3
1 parent 912aa47
commit fc587f3
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 86 deletions.
diff --git a/cppwinrt/code_writers.h b/cppwinrt/code_writers.h
@@ -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;
  };
 )";
@@ -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
@@ -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

diff --git a/strings/base_delegate.h b/strings/base_delegate.h
@@ -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...);
+ }})
  {
  }
 
@@ -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...);
+ }})
  {
  }
 

diff --git a/test/old_tests/UnitTests/delegate_weak_strong.cpp b/test/old_tests/UnitTests/delegate_weak_strong.cpp
@@ -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;
@@ -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>