signal_with_accumulator derives from trackable

kjellahl · kjellahl · commit 8fb78907ccf3 · 2022-05-30T16:14:55.000+02:00
A slot made with signal_with_accumulator::make_slot() is then automatically disconnected when the signal is deleted, as in sigc++2. Fixes #80
diff --git a/sigc++/signal.h b/sigc++/signal.h
@@ -371,8 +371,14 @@ struct signal_emit<void, void, T_arg...>
 
 } /* namespace internal */
 
+// TODO: When we can break ABI, let signal_base instead of signal_with_accumulator
+// derive from trackable, as in sigc++2. One of them must derive from trackable.
+// Otherwise the slot returned from signal_with_accumulator::make_slot() is not
+// automatically disconnected when the signal is deleted.
+// https://github.com/libsigcplusplus/libsigcplusplus/issues/80
+
 /** Signal declaration.
- * signal_with_accumulator can be used to connect() slots that are invoked
+ * %signal_with_accumulator can be used to connect() slots that are invoked
  * during subsequent calls to emit(). Any functor or slot
  * can be passed into connect(). It is converted into a slot
  * implicitly.
@@ -396,7 +402,9 @@ struct signal_emit<void, void, T_arg...>
  * @ingroup signal
  */
 template<typename T_return, typename T_accumulator, typename... T_arg>
-class signal_with_accumulator : public signal_base
+class signal_with_accumulator
+: public signal_base
+, public trackable
 {
 public:
   using slot_type = slot<T_return(T_arg...)>;
@@ -461,11 +469,6 @@ class signal_with_accumulator : public signal_base
   }
 
   /** Creates a functor that calls emit() on this signal.
-   *
-   * @note %sigc::signal does not derive from sigc::trackable in sigc++3.
-   * If you connect the returned functor (calling %emit() on signal1) to
-   * another signal (signal2) and then delete signal1, you must manually
-   * disconnect signal1 from signal2 before you delete signal1.
    *
    * @code
    * sigc::mem_fun(mysignal, &sigc::signal_with_accumulator::emit)
@@ -485,19 +488,28 @@ class signal_with_accumulator : public signal_base
 
   signal_with_accumulator() = default;
 
-  signal_with_accumulator(const signal_with_accumulator& src) : signal_base(src) {}
+  signal_with_accumulator(const signal_with_accumulator& src) : signal_base(src), trackable(src) {}
 
-  signal_with_accumulator(signal_with_accumulator&& src) : signal_base(std::move(src)) {}
+  signal_with_accumulator(signal_with_accumulator&& src)
+  : signal_base(std::move(src)), trackable(std::move(src))
+  {
+  }
 
   signal_with_accumulator& operator=(const signal_with_accumulator& src)
   {
     signal_base::operator=(src);
+    // Don't call trackable::operator=(src).
+    // It calls notify_callbacks(). This signal is not destroyed.
     return *this;
   }
 
   signal_with_accumulator& operator=(signal_with_accumulator&& src)
   {
     signal_base::operator=(std::move(src));
+    if (src.impl_ != impl_)
+      src.notify_callbacks();
+    // Don't call trackable::operator=(std::move(src)).
+    // It calls notify_callbacks(). This signal is not destroyed.
     return *this;
   }
 };
@@ -531,7 +543,7 @@ class signal_with_accumulator : public signal_base
  * @par Example:
  * @code
  * void foo(int) {}
- * sigc::signal<void, long> sig;
+ * sigc::signal<void(long)> sig;
  * sig.connect(sigc::ptr_fun(&foo));
  * sig.emit(19);
  * @endcode
diff --git a/sigc++/signal_base.h b/sigc++/signal_base.h
@@ -266,9 +266,10 @@ struct SIGC_API signal_impl_holder
  * @ref sigc::signal_with_accumulator::connect() "sigc::signal::connect()".
  */
 
-// TODO: When we can break ABI, let signal_base derive from trackable again.
-// It does in sigc++2. Otherwise the slot returned from signal::make_slot()
-// is not automatically disconnected when the signal is deleted.
+// TODO: When we can break ABI, let signal_base instead of signal_with_accumulator
+// derive from trackable, as in sigc++2. One of them must derive from trackable.
+// Otherwise the slot returned from signal_with_accumulator::make_slot() is not
+// automatically disconnected when the signal is deleted.
 // https://github.com/libsigcplusplus/libsigcplusplus/issues/80
 
 /** Base class for the @ref sigc::signal<T_return(T_arg...)> "sigc::signal" template.
diff --git a/tests/test_signal.cc b/tests/test_signal.cc
@@ -5,6 +5,7 @@
 #include "testutilities.h"
 #include <sigc++/trackable.h>
 #include <sigc++/signal.h>
+#include <memory>
 
 namespace
 {
@@ -110,6 +111,17 @@ test_make_slot()
   sig2.connect(sig.make_slot());
   sig2(3);
   util->check_result(result_stream, "foo(int 3) bar(float 3) foo(int 3) ");
+
+  // Delete a signal that has been connected to sig2.
+  sig2.clear();
+  sig2.connect(sigc::ptr_fun(&bar));
+  auto sig3 = std::make_unique<sigc::signal<int(int)>>();
+  sig3->connect(sigc::ptr_fun(&foo));
+  sig2.connect(sig3->make_slot());
+  sig2(2);
+  sig3.reset();
+  sig2(42);
+  util->check_result(result_stream, "bar(float 2) foo(int 2) bar(float 42) ");
 }
 
 void
