signal: Typed signals (part 2) (#60)

* signals: make CSignalT API compatible with CSignal

Also fixes emitting reference types

* signals: add a lot of tests

* animation: use CSignalT

* signals: automatically const-ref non arithmetic value types

* signals: allow listeners to ignore args

* signals: add forward()

include/hyprutils/animation/AnimationManager.hpp

@@ -35,8 +35,8 @@ namespace Hyprutils {
             const std::unordered_map<std::string, Memory::CSharedPointer<CBezierCurve>>& getAllBeziers();
 
             struct SAnimationManagerSignals {
-                Signal::CSignal connect;    // WP<CBaseAnimatedVariable>
+                Signal::CSignalT<Memory::CWeakPointer<CBaseAnimatedVariable>> connect;
-                Signal::CSignal disconnect; // WP<CBaseAnimatedVariable>
+                Signal::CSignalT<Memory::CWeakPointer<CBaseAnimatedVariable>> disconnect;
             };
 
             Memory::CWeakPointer<SAnimationManagerSignals>           getSignals() const;
@@ -48,9 +48,6 @@ namespace Hyprutils {
 
             bool                                                                  m_bTickScheduled = false;
 
-            void                                                                  onConnect(std::any data);
-            void                                                                  onDisconnect(std::any data);
-
             struct SAnimVarListeners {
                 Signal::CHyprSignalListener connect;
                 Signal::CHyprSignalListener disconnect;

include/hyprutils/signal/Listener.hpp

@@ -6,7 +6,7 @@
 
 namespace Hyprutils {
     namespace Signal {
-        class CUntypedSignal;
+        class CSignalBase;
 
         class CSignalListener {
           public:
@@ -24,7 +24,7 @@ namespace Hyprutils {
 
             std::function<void(void*)> m_fHandler;
 
-            friend class CUntypedSignal;
+            friend class CSignalBase;
         };
 
         typedef Hyprutils::Memory::CSharedPointer<CSignalListener> CHyprSignalListener;

include/hyprutils/signal/Signal.hpp

@@ -2,53 +2,107 @@
 
 #include <functional>
 #include <any>
+#include <type_traits>
+#include <utility>
 #include <vector>
 #include <memory>
 #include <tuple>
+#include <hyprutils/memory/SharedPtr.hpp>
 #include <hyprutils/memory/WeakPtr.hpp>
 #include "./Listener.hpp"
 
 namespace Hyprutils {
     namespace Signal {
-        class CUntypedSignal {
+        class CSignalBase {
           protected:
-            CHyprSignalListener                                           registerListenerInternal(std::function<void(void*)> handler);
+            CHyprSignalListener                                             registerListenerInternal(std::function<void(void*)> handler);
-            void                                                          registerStaticListenerInternal(std::function<void(void*)> handler);
+            void                                                            registerStaticListenerInternal(std::function<void(void*)> handler);
-            void                                                          emitInternal(void* args);
+            void                                                            emitInternal(void* args);
 
-            std::vector<Hyprutils::Memory::CWeakPointer<CSignalListener>> m_vListeners;
+            std::vector<Hyprutils::Memory::CWeakPointer<CSignalListener>>   m_vListeners;
-            std::vector<std::unique_ptr<CSignalListener>>                 m_vStaticListeners;
+            std::vector<Hyprutils::Memory::CSharedPointer<CSignalListener>> m_vStaticListeners;
         };
 
         template <typename... Args>
-        class CSignalT : public CUntypedSignal {
+        class CSignalT : public CSignalBase {
+            template <typename T>
+            using RefArg = std::conditional_t<std::is_reference_v<T> || std::is_arithmetic_v<T>, T, const T&>;
+
           public:
-            void emit(Args... args) {
+            void emit(RefArg<Args>... args) {
-                auto argsTuple = std::make_tuple(args...);
+                if constexpr (sizeof...(Args) == 0)
-                emitInternal(&argsTuple);
+                    emitInternal(nullptr);
+                else {
+                    auto argsTuple = std::tuple<RefArg<Args>...>(args...);
+
+                    if constexpr (sizeof...(Args) == 1)
+                        // NOLINTNEXTLINE: const is reapplied by handler invocation if required
+                        emitInternal(const_cast<void*>(static_cast<const void*>(&std::get<0>(argsTuple))));
+                    else
+                        emitInternal(&argsTuple);
+                }
             }
 
-            [[nodiscard("Listener is unregistered when the ptr is lost")]] CHyprSignalListener registerListener(std::function<void(Args...)> handler) {
+            [[nodiscard("Listener is unregistered when the ptr is lost")]] CHyprSignalListener listen(std::function<void(RefArg<Args>...)> handler) {
-                return registerListenerInternal([handler](void* argsPtr) { std::apply(handler, *static_cast<std::tuple<Args...>*>(argsPtr)); });
+                return registerListenerInternal(mkHandler(handler));
+            }
+
+            [[nodiscard("Listener is unregistered when the ptr is lost")]] CHyprSignalListener listen(std::function<void()> handler)
+                requires(sizeof...(Args) != 0)
+            {
+                return listen([handler](RefArg<Args>... args) { handler(); });
+            }
+
+            template <typename... OtherArgs>
+            [[nodiscard("Listener is unregistered when the ptr is lost")]] CHyprSignalListener forward(CSignalT<OtherArgs...>& signal) {
+                if constexpr (sizeof...(OtherArgs) == 0)
+                    return listen([&signal](RefArg<Args>... args) { signal.emit(); });
+                else
+                    return listen([&signal](RefArg<Args>... args) { signal.emit(args...); });
+            }
+
+            [[deprecated("Use listener()")]] CHyprSignalListener registerListener(std::function<void(std::any d)> handler) {
+                return listen([handler](const Args&... args) {
+                    constexpr auto mkAny = [](std::any d = {}) { return d; };
+                    handler(mkAny(args...));
+                });
             }
 
             // this is for static listeners. They die with this signal.
-            void registerStaticListener(std::function<void(Args...)> handler) {
+            void listenStatic(std::function<void(RefArg<Args>...)> handler) {
-                registerStaticListenerInternal([handler](void* argsPtr) { std::apply(handler, *static_cast<std::tuple<Args...>*>(argsPtr)); });
+                registerStaticListenerInternal(mkHandler(handler));
             }
 
-            template <typename Owner>
+            void listenStatic(std::function<void()> handler)
-            void registerStaticListener(std::function<void(Owner*, Args...)> handler, Owner* owner) {
+                requires(sizeof...(Args) != 0)
-                registerStaticListener([owner, handler](Args... args) { handler(owner, args...); });
+            {
+                return listenStatic([handler](RefArg<Args>... args) { handler(); });
+            }
+
+            [[deprecated("Use staticListener()")]] void registerStaticListener(std::function<void(void*, std::any)> handler, void* owner) {
+                return listenStatic([handler, owner](const RefArg<Args>&... args) {
+                    constexpr auto mkAny = [](std::any d = {}) { return d; };
+                    handler(owner, mkAny(args...));
+                });
+            }
+
+          private:
+            std::function<void(void*)> mkHandler(std::function<void(RefArg<Args>...)> handler) {
+                return [handler](void* args) {
+                    if constexpr (sizeof...(Args) == 0)
+                        handler();
+                    else if constexpr (sizeof...(Args) == 1)
+                        handler(*static_cast<std::remove_reference_t<std::tuple_element_t<0, std::tuple<RefArg<Args>...>>>*>(args));
+                    else
+                        std::apply(handler, *static_cast<std::tuple<RefArg<Args>...>*>(args));
+                };
             }
         };
 
         // compat
-        class CSignal : public CSignalT<std::any> {
+        class [[deprecated("Use CSignalT")]] CSignal : public CSignalT<std::any> {
           public:
-            void                                                                               emit(std::any data = {});
+            void emit(std::any data = {});
-            [[nodiscard("Listener is unregistered when the ptr is lost")]] CHyprSignalListener registerListener(std::function<void(std::any)> handler);
-            void                                                                               registerStaticListener(std::function<void(void*, std::any)> handler, void* owner);
         };
     }
 }

src/animation/AnimationManager.cpp

@@ -20,31 +20,18 @@ CAnimationManager::CAnimationManager() {
     m_events    = makeUnique<SAnimationManagerSignals>();
     m_listeners = makeUnique<SAnimVarListeners>();
 
-    m_listeners->connect    = m_events->connect.registerListener([this](std::any data) { onConnect(data); });
+    m_listeners->connect = m_events->connect.listen([this](const WP<CBaseAnimatedVariable>& animVar) {
-    m_listeners->disconnect = m_events->disconnect.registerListener([this](std::any data) { onDisconnect(data); });
+        if (!m_bTickScheduled)
-}
+            scheduleTick();
 
-void CAnimationManager::onConnect(std::any data) {
+        if (animVar)
-    if (!m_bTickScheduled)
+            m_vActiveAnimatedVariables.emplace_back(animVar);
-        scheduleTick();
+    });
 
-    try {
+    m_listeners->disconnect = m_events->disconnect.listen([this](const WP<CBaseAnimatedVariable>& animVar) {
-        const auto PAV = std::any_cast<WP<CBaseAnimatedVariable>>(data);
+        if (animVar)
-        if (!PAV)
+            std::erase_if(m_vActiveAnimatedVariables, [&](const auto& other) { return !other || other == animVar; });
-            return;
+    });
-
-        m_vActiveAnimatedVariables.emplace_back(PAV);
-    } catch (const std::bad_any_cast&) { return; }
-}
-
-void CAnimationManager::onDisconnect(std::any data) {
-    try {
-        const auto PAV = std::any_cast<WP<CBaseAnimatedVariable>>(data);
-        if (!PAV)
-            return;
-
-        std::erase_if(m_vActiveAnimatedVariables, [&](const auto& other) { return !other || other == PAV; });
-    } catch (const std::bad_any_cast&) { return; }
 }
 
 void CAnimationManager::removeAllBeziers() {

src/signal/Signal.cpp

@@ -1,3 +1,4 @@
+#include "hyprutils/memory/SharedPtr.hpp"
 #include <hyprutils/signal/Signal.hpp>
 #include <hyprutils/memory/WeakPtr.hpp>
 #include <algorithm>
@@ -8,7 +9,7 @@ using namespace Hyprutils::Memory;
 #define SP CSharedPointer
 #define WP CWeakPointer
 
-void Hyprutils::Signal::CUntypedSignal::emitInternal(void* args) {
+void Hyprutils::Signal::CSignalBase::emitInternal(void* args) {
     std::vector<SP<CSignalListener>> listeners;
     for (auto& l : m_vListeners) {
         if (l.expired())
@@ -17,11 +18,7 @@ void Hyprutils::Signal::CUntypedSignal::emitInternal(void* args) {
         listeners.emplace_back(l.lock());
     }
 
-    std::vector<CSignalListener*> statics;
+    auto statics = m_vStaticListeners;
-    statics.reserve(m_vStaticListeners.size());
-    for (auto& l : m_vStaticListeners) {
-        statics.emplace_back(l.get());
-    }
 
     for (auto& l : listeners) {
         // if there is only one lock, it means the event is only held by the listeners
@@ -43,7 +40,7 @@ void Hyprutils::Signal::CUntypedSignal::emitInternal(void* args) {
     // as such we'd be doing a UAF
 }
 
-CHyprSignalListener Hyprutils::Signal::CUntypedSignal::registerListenerInternal(std::function<void(void*)> handler) {
+CHyprSignalListener Hyprutils::Signal::CSignalBase::registerListenerInternal(std::function<void(void*)> handler) {
     CHyprSignalListener listener = SP<CSignalListener>(new CSignalListener(handler));
     m_vListeners.emplace_back(listener);
 
@@ -53,18 +50,10 @@ CHyprSignalListener Hyprutils::Signal::CUntypedSignal::registerListenerInternal(
     return listener;
 }
 
-void Hyprutils::Signal::CUntypedSignal::registerStaticListenerInternal(std::function<void(void*)> handler) {
+void Hyprutils::Signal::CSignalBase::registerStaticListenerInternal(std::function<void(void*)> handler) {
-    m_vStaticListeners.emplace_back(std::unique_ptr<CSignalListener>(new CSignalListener(handler)));
+    m_vStaticListeners.emplace_back(SP<CSignalListener>(new CSignalListener(handler)));
 }
 
 void Hyprutils::Signal::CSignal::emit(std::any data) {
     CSignalT::emit(data);
 }
-
-CHyprSignalListener Hyprutils::Signal::CSignal::registerListener(std::function<void(std::any)> handler) {
-    return CSignalT::registerListener(handler);
-}
-
-void Hyprutils::Signal::CSignal::registerStaticListener(std::function<void(void*, std::any)> handler, void* owner) {
-    CSignalT<std::any>::registerStaticListener<void>(handler, owner);
-}

tests/signal.cpp

@@ -1,11 +1,18 @@
 #include <any>
 #include <hyprutils/signal/Signal.hpp>
 #include <hyprutils/memory/WeakPtr.hpp>
+#include <memory>
+#include "hyprutils/memory/SharedPtr.hpp"
+#include "hyprutils/signal/Listener.hpp"
 #include "shared.hpp"
 
 using namespace Hyprutils::Signal;
 using namespace Hyprutils::Memory;
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+//
+
 void legacy(int& ret) {
     CSignal signal;
     int     data     = 0;
@@ -33,11 +40,32 @@ void legacyListenerEmit(int& ret) {
     EXPECT(data, 1);
 }
 
+void legacyListeners(int& ret) {
+    int           data = 0;
+
+    CSignalT<>    signal0;
+    CSignalT<int> signal1;
+
+    auto          listener0 = signal0.registerListener([&](std::any d) { data += 1; });
+    auto          listener1 = signal1.registerListener([&](std::any d) { data += std::any_cast<int>(d); });
+
+    signal0.registerStaticListener([&](void* o, std::any d) { data += 10; }, nullptr);
+    signal1.registerStaticListener([&](void* o, std::any d) { data += std::any_cast<int>(d) * 10; }, nullptr);
+
+    signal0.emit();
+    signal1.emit(2);
+
+    EXPECT(data, 33);
+}
+
+#pragma GCC diagnostic pop
+//
+
 void empty(int& ret) {
     int        data = 0;
 
     CSignalT<> signal;
-    auto       listener = signal.registerListener([&] { data = 1; });
+    auto       listener = signal.listen([&] { data = 1; });
 
     signal.emit();
     EXPECT(data, 1);
@@ -52,19 +80,31 @@ void typed(int& ret) {
     int           data = 0;
 
     CSignalT<int> signal;
-    auto          listener = signal.registerListener([&](int newData) { data = newData; });
+    auto          listener = signal.listen([&](int newData) { data = newData; });
 
     signal.emit(1);
     EXPECT(data, 1);
 }
 
+void ignoreParams(int& ret) {
+    int           data = 0;
+
+    CSignalT<int> signal;
+    auto          listener = signal.listen([&] { data += 1; });
+
+    signal.listenStatic([&] { data += 1; });
+
+    signal.emit(2);
+    EXPECT(data, 2);
+}
+
 void typedMany(int& ret) {
     int                     data1 = 0;
     int                     data2 = 0;
     int                     data3 = 0;
 
     CSignalT<int, int, int> signal;
-    auto                    listener = signal.registerListener([&](int d1, int d2, int d3) {
+    auto                    listener = signal.listen([&](int d1, int d2, int d3) {
         data1 = d1;
         data2 = d2;
         data3 = d3;
@@ -76,25 +116,255 @@ void typedMany(int& ret) {
     EXPECT(data3, 3);
 }
 
+void ref(int& ret) {
+    int            count = 0;
+    int            data  = 0;
+
+    CSignalT<int&> signal;
+    auto           l1 = signal.listen([&](int& v) { v += 1; });
+    auto           l2 = signal.listen([&](int v) { count += v; });
+    signal.emit(data);
+
+    CSignalT<const int&> constSignal;
+    auto                 l3 = constSignal.listen([&](const int& v) { count += v; });
+    auto                 l4 = constSignal.listen([&](int v) { count += v; });
+    constSignal.emit(data);
+
+    EXPECT(data, 1);
+    EXPECT(count, 3);
+}
+
+void refMany(int& ret) {
+    int                        count = 0;
+    int                        data1 = 0;
+    int                        data2 = 10;
+
+    CSignalT<int&, const int&> signal;
+    auto                       l1 = signal.listen([&](int& v, const int&) { v += 1; });
+    auto                       l2 = signal.listen([&](int v1, int v2) { count += v1 + v2; });
+
+    signal.emit(data1, data2);
+    EXPECT(data1, 1);
+    EXPECT(count, 11);
+}
+
+void autoRefTypes(int& ret) {
+    class CCopyCounter {
+      public:
+        CCopyCounter(int& createCount, int& destroyCount) : createCount(createCount), destroyCount(destroyCount) {
+            createCount += 1;
+        }
+
+        CCopyCounter(CCopyCounter&& other) noexcept : CCopyCounter(other.createCount, other.destroyCount) {}
+        CCopyCounter(const CCopyCounter& other) noexcept : CCopyCounter(other.createCount, other.destroyCount) {}
+
+        ~CCopyCounter() {
+            destroyCount += 1;
+        }
+
+      private:
+        int& createCount;
+        int& destroyCount;
+    };
+
+    auto                   createCount  = 0;
+    auto                   destroyCount = 0;
+
+    CSignalT<CCopyCounter> signal;
+    auto                   listener = signal.listen([](const CCopyCounter& counter) {});
+
+    signal.emit(CCopyCounter(createCount, destroyCount));
+    EXPECT(createCount, 1);
+    EXPECT(destroyCount, 1);
+}
+
+void forward(int& ret) {
+    int           count = 0;
+
+    CSignalT<int> sig;
+    CSignalT<int> connected1;
+    CSignalT<>    connected2;
+
+    auto          conn1 = sig.forward(connected1);
+    auto          conn2 = sig.forward(connected2);
+
+    auto          listener1 = connected1.listen([&](int v) { count += v; });
+    auto          listener2 = connected2.listen([&] { count += 1; });
+
+    sig.emit(2);
+
+    EXPECT(count, 3);
+}
+
+void listenerAdded(int& ret) {
+    int                 count = 0;
+
+    CSignalT<>          signal;
+    CHyprSignalListener secondListener;
+
+    auto                listener = signal.listen([&] {
+        count += 1;
+
+        if (!secondListener)
+            secondListener = signal.listen([&] { count += 1; });
+    });
+
+    signal.emit();
+    EXPECT(count, 1); // second should NOT be invoked as it was registed during emit
+
+    signal.emit();
+    EXPECT(count, 3); // second should be invoked
+}
+
+void lastListenerSwapped(int& ret) {
+    int                 count = 0;
+
+    CSignalT<>          signal;
+    CHyprSignalListener removedListener;
+    CHyprSignalListener addedListener;
+
+    auto                firstListener = signal.listen([&] {
+        removedListener.reset(); // dropped and should NOT be invoked
+
+        if (!addedListener)
+            addedListener = signal.listen([&] { count += 2; });
+    });
+
+    removedListener = signal.listen([&] { count += 1; });
+
+    signal.emit();
+    EXPECT(count, 0); // neither the removed nor added listeners should fire
+
+    signal.emit();
+    EXPECT(count, 2); // only the new listener should fire
+}
+
+void signalDestroyed(int& ret) {
+    int  count = 0;
+
+    auto signal = std::make_unique<CSignalT<>>();
+
+    // This ensures a destructor of a listener called before signal reset is safe.
+    auto preListener = signal->listen([&] { count += 1; });
+
+    auto listener = signal->listen([&] { signal.reset(); });
+
+    // This ensures a destructor of a listener called after signal reset is safe
+    // and gets called.
+    auto postListener = signal->listen([&] { count += 1; });
+
+    signal->emit();
+    EXPECT(count, 2); // all listeners should fire regardless of signal deletion
+}
+
+// purely an asan test
+void signalDestroyedBeforeListener() {
+    CHyprSignalListener listener1;
+    CHyprSignalListener listener2;
+
+    CSignalT<>          signal;
+
+    listener1 = signal.listen([] {});
+    listener2 = signal.listen([] {});
+}
+
+void signalDestroyedWithAddedListener(int& ret) {
+    int                 count = 0;
+
+    auto                signal = std::make_unique<CSignalT<>>();
+    CHyprSignalListener shouldNotRun;
+
+    auto                listener = signal->listen([&] {
+        shouldNotRun = signal->listen([&] { count += 2; });
+        signal.reset();
+    });
+
+    signal->emit();
+    EXPECT(count, 0);
+}
+
+void signalDestroyedWithRemovedAndAddedListener(int& ret) {
+    int                 count = 0;
+
+    auto                signal = std::make_unique<CSignalT<>>();
+    CHyprSignalListener removed;
+    CHyprSignalListener shouldNotRun;
+
+    auto                listener = signal->listen([&] {
+        removed.reset();
+        shouldNotRun = signal->listen([&] { count += 2; });
+        signal.reset();
+    });
+
+    removed = signal->listen([&] { count += 1; });
+
+    signal->emit();
+    EXPECT(count, 0);
+}
+
 void staticListener(int& ret) {
-    struct STestOwner {
+    int           data = 0;
-        int data = 0;
-    } owner;
 
     CSignalT<int> signal;
-    signal.registerStaticListener<STestOwner>([&](STestOwner* owner, int newData) { owner->data = newData; }, &owner);
+    signal.listenStatic([&](int newData) { data = newData; });
 
     signal.emit(1);
-    EXPECT(owner.data, 1);
+    EXPECT(data, 1);
+}
+
+void staticListenerDestroy(int& ret) {
+    int  count = 0;
+
+    auto signal = makeShared<CSignalT<>>();
+    signal->listenStatic([&] { count += 1; });
+
+    signal->listenStatic([&] {
+        // should not fire but SHOULD be freed
+        signal->listenStatic([&] { count += 3; });
+
+        signal.reset();
+    });
+
+    signal->listenStatic([&] { count += 1; });
+
+    signal->emit();
+    EXPECT(count, 2);
+}
+
+// purely an asan test
+void listenerDestroysSelf() {
+    CSignalT<>          signal;
+
+    CHyprSignalListener listener;
+    listener = signal.listen([&] { listener.reset(); });
+
+    // the static signal case is taken care of above
+
+    signal.emit();
 }
 
 int main(int argc, char** argv, char** envp) {
     int ret = 0;
     legacy(ret);
     legacyListenerEmit(ret);
+    legacyListeners(ret);
     empty(ret);
     typed(ret);
+    ignoreParams(ret);
     typedMany(ret);
+    ref(ret);
+    refMany(ret);
+    autoRefTypes(ret);
+    forward(ret);
+    listenerAdded(ret);
+    lastListenerSwapped(ret);
+    signalDestroyed(ret);
+    signalDestroyedBeforeListener();
+    signalDestroyedWithAddedListener(ret);
+    signalDestroyedWithRemovedAndAddedListener(ret);
     staticListener(ret);
+    staticListenerDestroy(ret);
+    signalDestroyed(ret);
+    listenerDestroysSelf();
     return ret;
 }