palacaze / sigslot

A simple C++14 signal-slots implementation
MIT License
709 stars 97 forks source link

Unable to emit signals from const member functions #45

Closed beeka closed 4 months ago

beeka commented 5 months ago

I am using the library to decouple classes in my application and got a confusing compiler error when trying to emit a signal from one of the classes (no matching function). This looks like it is due to no "operator() const" function being available, which means the compiler can't find a matching operator function when being called from a const function. Such as in this guesstimated code:

class Mapper { public: sigslot::signal<int> event; void mapEvent(int e) const { event(e); // fails } };

My work-around is to use const_cast:

void mapEvent(int e) const { const_cast<Mapper*>(this).event(e); // works }

Is there some conceptual reason for the lack a const function, e.g. does emitting change the state of the signal?

palacaze commented 4 months ago

Technically slots may change state during emission, for instance tracked object may disconnect themselves from the signal, which changes its state.

However I constified the call operator of signal_base in the perf branch and the tests compile and run just fine. I suspect the reason lies in C++ not propagating const correctly. If I also constify the slots call_slot() methods compilation fails, on account of signal_base::clean being called.

So I guess I can make it const, somehow.

beeka commented 4 months ago

Ideally the signal emitter should not be forced to be forced to be "volatile" (non-const) and the slots should not be force to be const. This feels possible, as a const instance can point to a non-const object... and if it weren't then a the library doing a const-cast would be safest (preserves the intention).

A slot could still disconnect, assuming it has a route to a non-const object reference / pointer. I couldn't find an equivalent to the Qt sender() function, so the slots have to be quite deliberate with their connection management.

palacaze commented 4 months ago

I agree. I just pushed a fix to master. Please reopen if it breaks something but I think it should be fine.

beeka commented 4 months ago

Thanks. Seems to work for me.