Deprecate std.signals?

[Ahmet Sait]

On Tuesday, 30 March 2021 at 11:23:21 UTC, Berni44 wrote:
> I've got the feeling, that std.signals is rarely used and not 
> up to D's standards... Therefore I suggest to move it to undead 
> and deprecate it.
>
> But I might err on this. What do you think?

So I did some research on this looking at phobos and a bunch of 
other signal/event implementations and concluded that std.signals 
isn't even that bad (in fact it's better than some of the stuff 
found in the wild). It has issues but they are fixable.

It's nonsensical to me how people want to remove code before 
talking about why is it bad and how can it be done better. So 
let's do that first.

Issues with std.signals as far as I see:

- Signal is a mixin template
   This one wasn't supposed to be a problem but semantics of mixin 
templates result in this issue: [Issue 5028 - Problem with named 
mixins in base class and derived 
class](https://issues.dlang.org/show_bug.cgi?id=5028)
   I would be in favor of fixing the semantics for the better 
instead of blaming this particular design. Mixin templates even 
allow for some neat tricks like making `emit()` function 
protected and therefore making it only callable in the class the 
signal is declared in (similar to event keyword in C#).

- Signal uses weak reference semantics
   This is not actually an issue but a questionable choice 
nonetheless. Currently, Signal puts delegates into a malloc-ed 
array which is not scanned by the GC and it subscribes to the 
object's dispose event using `rt_attachDisposeEvent()`, so when 
some object is collected (or manually destroyed), the Signal gets 
notified and it removes those delegates which reference that 
destroyed object. I don't know what behavior is the better 
default but events are strong references where I'm coming from 
(C#). Making it optional could be a better decision.

- Signal does not work with lambda delegates
   This one is basically a consequence of the previous point and 
looks like the biggest reason people handroll their own; 
`rt_attachDisposeEvent()` requires an Object and therefore Signal 
depends on the delegate context pointer referring to a class 
instance. If there was a proper way to know what kind of context 
pointer a delegate has, this wouldn't be an issue. See [Issue 
19842 - std.signals Segfault when connecting with a 
lambda](https://issues.dlang.org/show_bug.cgi?id=19842).
   - Making closures capture the variables into an object instance 
would solve this for most cases except struct member functions 
still won't work. See [Issue 9601 - Make regular D objects on 
closures](https://issues.dlang.org/show_bug.cgi?id=9601).
   - Introducing different delegate types to the language would 
potentially solve this. The idea is to have different delegates 
that are compiled down to the same runtime code but allow for 
overloading via type system similar to how char & byte types 
work. I suspect it is unlikely for this idea to see the light of 
day considering the immense bureaucratic nonsense in the DIP 
process. Happy to be proven wrong of course.
   - Making delegates even fatter by introducing a simple 
`ptrType` field so that we can just check it. Not a fan of this 
idea.
   - Making `rt_attachDisposeEvent()` work with things other than 
Object. I have no idea if this is feasible but theoretically the 
GC can provide such an API (callback for when a memory region is 
collected).
   - Alternatively, screw that runtime dispose event feature and 
just stop. Make delegates strongly referenced and tell users to 
be careful and not destroy/free stuff if it's referenced by 
delegates somewhere.
   - Have a better idea? Let's see what you got.

- Signal is not thread safe
   I don't think this is even relevant. Maybe fix shared first? :P

- Signal.emit() is not reentrant
   It currently does not allow recursive calls to the same signal. 
Looks like a limitation imposed in order to fix other bugs.

Now let's look at bugzilla:

- [Issue 4150 - std.signals causes memory corruption and 
heisenbugs](https://issues.dlang.org/show_bug.cgi?id=4150)
   This bug is no longer valid, not reproducable, and should be 
closed.

- [Issue 9606 - `std.signal` implementation is fundamentally 
incorrect](https://issues.dlang.org/show_bug.cgi?id=9606)
   Looks like a valid bug. Signal assumes 
`rt_attachDisposeEvent()` callback is not called inside emit(). 
Related to weakref business above.

- [Issue 16203 - std.signals connect() 
error](https://issues.dlang.org/show_bug.cgi?id=16203)
   Duplicate of [Issue 19842 - std.signals Segfault when 
connecting with a 
lambda](https://issues.dlang.org/show_bug.cgi?id=19842).

- [Issue 17011 - cleanup std.signals 
documentation](https://issues.dlang.org/show_bug.cgi?id=17011)
   Documentation enchanment.

- [Issue 18903 - std.signals uses 
_dtor](https://issues.dlang.org/show_bug.cgi?id=18903)
   Not actually a bug.

- [Issue 19842 - std.signals Segfault when connecting with a 
lambda](https://issues.dlang.org/show_bug.cgi?id=19842)
   The biggest actual problem with std.signals as has been 
mentioned above.

There is literally nothing interesting about what other 
implementations do either. Everyone just does the same thing 
creating yet another struct with delegate[] array over and over 
again thinking what they do is magically better. Idk maybe that's 
what std.signals should do after all ¯\\\_(ツ)_/¯

Let's see what people think.