palacaze/sigslot

Connect lamda with auto disconnection context

OlivierLDff opened this issue · 8 comments

OlivierLDff commented

Hi I would like to connect to lambda and signal, but using an object for auto disconnection.
Something like:

#include <sigslot/signal.hpp>

int sum = 0;

struct s : sigslot::observer_st {
    signal<int> sig;
};

int main() {
    sum = 0;
    signal<int> sig;
    
    {
        s p;
        // Connect lambda with p as context
        sig.connect([](int i){ sum += i; }, p);

        // Connect signal with p as context
        sig.connect(p.sig, p);
    }
    
    // The slots got disconnected at instance destruction
    sig(1);         // sum == 1
}

Use cases:

  • With qt, I often connect signal to lambda to create "adapter" to the real function
  • I like to connect signal to another signal to create a chain of signal.

Is this possible? Is it hard to achieve? I haven't dig into source code yet. I guess it is possible, since I'm doing that all the time with Qt.
I can work on PR if you want? If you can point me to where add unit tests, where is the code I should modify?

Thanks.

palacaze commented

Hi, yes there are multiple ways of handling auto disconnection.

The quick and easy way is to use scoped connections, which are implicitly constructed from connections, or use the connect_scoped overload to construct one. Those are RAII type handles that disconnect the slot upon destruction.

#include <sigslot/signal.hpp>

int main() {
    int sum = 0;
    signal<int> sig;
    
    {
        sigslot::scoped_connection c1 = sig.connect([](int i){ sum += i; });
        auto c2 = sig.connect_scoped([](int i){ sum += i; });
    }
    
    // The slots got disconnected at the end of the scope
    sig(1);         // sum == 0
}

Sigslot also uses the concept of tracking objects, which can be perused for your needs. Sigslot already distributes a Qt compatibility
header, that allows tracking QObjects, so you can already do something along the lines of:

#include <sigslot/adapter/qt.hpp>
#include <sigslot/signal.hpp>

int main() {
    int sum = 0;
    sigslot::signal<int> sig;

    {
        QObject o;
        sig.connect([](int i) { sum += i; }, &o);
        sig(1);
        assert(sum == 1);
    }

    sig(1);
    assert(sum == 1);
    return 0;
}

But you can also use std::shared_ptr or any other object that is trackable.

#include <sigslot/signal.hpp>

auto make_scoped_tracker() {
    struct scoped_tracker {};
    return std::make_shared<scoped_tracker>();
}

int main() {
    int sum = 0;
    sigslot::signal<int> sig;

    {
        auto tracker = make_scoped_tracker();
        sig.connect([](int i) { sum += i; }, tracker);
        sig(1);
        assert(sum == 1);
    }

    sig(1);
    assert(sum == 1);
    return 0;
}

For you second point, a wrapper seems to do the job, but I admit builtin support might be nicer:

template <typename... T>
struct signal_wrapper {
    signal_wrapper(sigslot::signal<T...> &sig)
        : m_sig{std::addressof(sig)}
    {}

    template <typename... U>
    void operator()(U && ...u) {
        (*m_sig)(std::forward<U>(u)...);
    }
private:
    sigslot::signal<T...> *m_sig{};
};

template <typename... T>
auto signal_wrap(sigslot::signal<T...> &sig)
{
    return signal_wrapper<T...>(sig);
}

void test_chain_signal() {
    sum = 0;
    sigslot::signal<int> sig1;
    sigslot::signal<int> sig2;

    {
        auto tracker = make_scoped_tracker();
        sig2.connect([](int i) { sum += i; }, tracker);
        sig1.connect(signal_wrap(sig2), tracker);
        sig1(1);
        assert(sum == 1);
    }

    sig1(1);
    assert(sum == 1);
}
OlivierLDff commented
  1. I was trying to connect lambda with sigslot::observer_st as trackable, but it doesn't seem to work, maybe I'm missing something in the syntax:
#include <sigslot/signal.hpp>

int sum = 0;

struct s : sigslot::observer_st
{
    void f(int i)
    {
        sum += i;
    }
};

int main()
{
    sum = 0;
    sigslot::signal<int> sig;

    {
        // Lifetime of object instance p is tracked
        s p;
        sig.connect([](int i) { sum += i; }, &p);
        sig(1); // sum == 1
    }

    // The slots got disconnected at instance destruction
    sig(1); // sum == 1
}
  1. Would you consider adding builtin support?
    Would be very nice to have everything hidden:
void test_chain_signal() {
    sum = 0;
    sigslot::signal<int> sig1;
    sigslot::signal<int> sig2;

    {
        auto tracker = make_scoped_tracker();
        sig2.connect([](int i) { sum += i; }, tracker);
        sig1.connect(sig2, tracker);
        sig1(1);
        assert(sum == 1);
    }

    sig1(1);
    assert(sum == 1);
}
palacaze commented

Yep, observer is a new feature I never use, I seem to have added support for pointer over member functions but no other callable types. It will need another connect() overload.

Yes I might do just that, I will have to consider thread-safety though.

OlivierLDff commented

Ok thanks, do you have any idea when you will implement such feature for to decice which road to go?
I don't feel competent enough to modify and create PR with all the template code, sorry.

OlivierLDff commented

I tried to go with the shared_ptr, but I'm having an issue:

class c : std::enable_shared_from_this<c> 
{
public:
  c()
  {
    sig.connect([](int){}, shared_from_this());
  }

  sigslot::signal<int> sig;
}

int main()
{
  auto cl = std::make_shared<c>(); // crash
}

Of course I never tried/tought about using shared_from_this before in constructor. Now I understand why that can't work.
https://stackoverflow.com/questions/31924396/why-shared-from-this-cant-be-used-in-constructor-from-technical-standpoint

So I'm looking forward to your implentation with sigslot::observer to be able to use a raw pointer from constructor:

class c : sigslot::observer
{
public:
  c()
  {
    sig.connect([](int v){ val += v; }, this);
  }

  sigslot::signal<int> sig;
  int val = 0;
}

int main()
{
  c cl;
  cl.sig(1);
  assert(val == 1);
}

Thanks a lot for quick support.

palacaze commented

Hi, I have a question related to you piece of code, does your design enforce the signal to be a member of your observer?

If I look at your last sample, no observer is even needed:

class c
{
public:
  c()
  {
    sig.connect([](int v){ val += v; });
  }

  int val = 0;
  sigslot::signal<int> sig;
}

The signal gets destroyed first and disconnects the lambda before val gets destroyed, so your are safe here.

Regarding chaining signals, I think a free connect function akin to what Qt uses might be a good addition instead of bloating the signal_base class with even more overloads.

OlivierLDff commented

Hi, I have a question related to you piece of code, does your design enforce the signal to be a member of your observer?

Almost never, otherwise yes you are right, that would be trivial.

  • I'm either connecting to signal of objects owned by my object. In those case, I don't need observer.
  • But I want to connect to, let's say a list of object that i don't own.

I will give you snippet of code that illustrate my use case of what I was doing with Qt and I want to reproduce.

class MyObject
{
  sigslot::signal<int> sig;
};

class MyList
{
  sigslot::signal<MyObject*> onInserted;
  sigslot::signal<MyObject*> onRemoved;
};

class MyObserver : sigslot::observer
{
  void setList(MyList* list)
  {
    if(_list != list)
    {
      if(_list)
        disconnectToList(_list);
      _list = list;
      if(_list)
        connectToList(_list);
    }
  }

  void connectToList(MyList* list)
  {
    list->onInserted.connect([this](MyObject* obj) { connectToObject(obj); }, this);
    // Could also be :
    // list->onInserted.connect(&MyObserver::connectToObject, this);
    list->onRemoved.connect([this](MyObject* obj) { disconnectToObject(obj); }, this);
  }

  void disconnectToList(MyList* list)
  {
    for(auto* obj : *list)
      disconnectToObject(obj);

    list->onInserted.disconnect(this);
    list->onRemoved.disconnect(this);
  }

  void connectToObject(MyObject* obj)
  {
    obj->sig.connect([](int){/* do stuff */}, this);
  }

  void disconnectToObject(MyObject* obj)
  {
    obj->sig.disconnect(this);
  }

  MyList* _list = nullptr;
};

And for chaining signals, my use case is to do composition.

struct MyFeature1
{
  sigslot::signal<int> feature1;
};

struct MyFeature2
{
  sigslot::signal<float> feature2;
};

struct MyPublicService
{
public:
  MyPublicService()
  {
    // so you are suggesting qt-ish solution like that ?
    sigslot::connect(f1.feature1, feature1);
    sigslot::connect(f2.feature2, feature2);
  }

  sigslot::signal<int> feature1;
  sigslot::signal<float> feature2;

private:
  MyFeature1 f1;
  MyFeature2 f2;
};
palacaze commented

Hi Olivier,

Looking at your code sample, it seems to me that a concept of connection_pool would be handier.
Here is a full working example that demonstrates the idea. It simplifies your observer object a lot and might be a nice addition to Sigslot, as it might fullfill most needs for scoped lifetime management.

I still agree that Sigslot should offer a default tracker object to simplify scoped lifetime management, I am not convinced that the current sigslot::observer is the right solution though. I added it with inheritance in mind, and it would need to implement some sort of ref-counting to handle lifetimes.

#include <iostream>
#include <optional>
#include <sigslot/signal.hpp>

/**
 * A connection pool stores scoped_connections and drops them all on destruction.
 * It is a simple way of managing the lifetime of a bunch of connections as a whole.
 */
struct connection_pool : private std::vector<scoped_connection> {
    template <typename SigT, typename ...Args>
    connection connect(SigT &sig, Args && ...args) {
        auto conn = sig.connect(std::forward<Args>(args)...);
        push_back(conn);
        return conn;
    }

    connection operator+=(connection && conn) {
        push_back(conn);
        return std::move(conn);
    }

    void drop() {
        std::vector<scoped_connection>::clear();
    }
};

}  // namespace sigslot

struct MyObject {
    explicit MyObject(int i) : i{i} {}

    void add(int a) {
        i += a;
        sig(i);
    }

    int i;
    sigslot::signal<int> sig;
};

class MyList : private std::vector<MyObject*> {
    using base = std::vector<MyObject*>;
    using value_type = typename base::value_type;
    using iterator = typename base::iterator;
    using const_iterator = typename base::const_iterator;

public:
    using base::begin;
    using base::end;

    ~MyList() {
        while (pop()) {}
    }

    void push(int i) {
        auto o = new MyObject(i);
        onInserted(o);
        push_back(std::move(o));
    }

    std::optional<int> pop() {
        if (empty()) {
            return std::nullopt;
        }
        auto o = std::move(back());
        onRemoved(o);
        pop_back();
        return o->i;
    }

    sigslot::signal<MyObject *> onInserted;
    sigslot::signal<MyObject *> onRemoved;
};

class MyObserver {
public:
    void setList(MyList *list)
    {
        if (_list != list) {
            disconnectFromList();
            _list = list;
            if (_list)
                connectToList();
        }
    }

private:
    void disconnectFromList()
    {
        _pool.drop();
    }

    void connectToList()
    {
        _pool.connect(_list->onInserted, &MyObserver::connectToObject, this);
        _pool.connect(_list->onRemoved, &MyObserver::disconnectFromObject, this);
    }

    void connectToObject(MyObject *obj)
    {
        _pool.connect(obj->sig, &MyObserver::doThings, this);
    }

    void disconnectFromObject(MyObject *obj)
    {
        obj->sig.disconnect(this);
    }

    void doThings(int i) {
        std::cout << "Val is " << i << std::endl;
    }

    MyList *_list = nullptr;
    sigslot::connection_pool _pool;
};

int main() {
    MyObserver obs;
    MyList list;
    obs.setList(&list);

    list.push(5);
    list.push(10);

    for (auto *item : list) {
        item->add(2);    // 7 and 12
    }

    list.pop();

    for (auto *item : list) {
        item->add(1);   // 8
    }

    obs.setList(nullptr);

    for (auto *item : list) {
        item->add(3);   // nothing printed
    }

    return 0;
}

Regarding signal chaining, I just pushed a freestanding sigslot::connect() that can do that on the safe-observer branch.