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>