Hi there!

I just developed a proof-of-concept implementation of an improved std.signals.

Things I did not like about the current implementation:

1. Passing a delegate to connect, which basically is an extended (void*) and assuming it is an object delegate, does not feel quite right in D, where we usually try to guarantee correctness by the compiler wherever possible.

2. The restriction that only objects can be connected.

Point 2 does not really bother me, because from my little experience I
never really had connected anything else than objects to a signal. But
the restriction that the connected delegate must not be some wrapper, is
quite a restriction I came to dislike about Qt and even more so with
this implementation because unlike Qt the signatures have to match
exactly, you can't omit parameters, like:
// Qt code
connect(button, SIGNAL(clicked(bool)), this, SLOT(buttonClicked());

-> In the current implementation buttonClicked would have to take a bool.

In addition boost::signals together with boost::bind offered even more comfort like passing additional parameters to a slot, which is really very, very useful:

for(int i=0; i<buttons.length(); i++) {
	buttons[i].clicked.connect(boost::bind(&MyObj::addNumber, this,
 i));
}

So I tried to improve std.signals, code:
(warning at least one bug is remaining, explained below)

https://github.com/eskimor/phobos/tree/new_signal

You can easily connect to an object's method with:

obj.signal.connect!"someMethod"(obj);

instead of the old implementation:

obj.signal.connect(&obj.someMethod);

-> The interface is clean and type safe, all the ugly details are hidden from the user. And it is just one character more typing. Simple things stay simple.

In addition a method allowing wrapper delegates was added:

class Observer {
	void addNumber(int i) {
		sum+=i;
	}
	int sum;
}

class Button {
	Signal!(bool) clicked;
	// ...
}

void main() {
	auto b=new Button;
	auto o=new Observer;
	// Ignore boolean parameter and pass some int:
	b.connect!Observer(o, (o, p) { o.addNumber(7); });
	// or even:
	b.connect!Observer(o, (o, p) => o.addNumber(7));
	// For some reason the compiler is not able to deduce "o" being
	// Observer, so the !Observer is needed, but it is still very
	// neat and readable.
}

Thanks to D's lamdas the syntax is even more concise as boost::bind and far more powerful.

By passing the object explicitly to the delegate, it is possible to maintain the 'weak ref' semantics to the target object, while ensuring that the delegates context won't be freed.

As a side effect it is now even possible to use struct delegates or even any non object delegate. Simply pass null for the obj parameter. It is completely safe, the only drawback is that the struct won't be deleted until the Button gets destroyed. (Because it holds a reference to the struct, by means of the delegate.) But for long lived structs this usually is perfectly acceptable.

Implementation:

In my implementation I changed the Signal mixin to be a simple template struct, because I hit strange compiler errors with it being a mixin. The most prominent:

std/signals.d(443): Error: no overload matches for connect(string
method,T2) if (is(T2 : Object))

You can find the version triggering these errors at:

https://github.com/eskimor/phobos/tree/new_signal_mixin

Also I did not really get the point why a mixin was used in the first
place, it does not really gain us anything? What was the reasoning about
it?
I almost thought I found the reason, because my implementations suffers
from unhook not being called, although it was properly registered with
"rt_attachDisposeEvent(obj, &unhook);", thus causing a segmentation
fault when the observer gets deleted. I did not really find any
difference from the original version that could explain this behavior,
despite the original implementation being a mixin. So I thought, well
maybe the delegate passed to "rt_attachDisposeEvent(obj, &unhook);" must
be an object delegate (that's would be why the mixin was needed), but
after digging in object_.d I did not find any code assuming that the
delegate was an object delegate. Any ideas on this?

Another thing I'd like to ask Walter, is what the "L1:" label is for in connect(), is it just some left over or has it some special internal compiler thing meaning?

What do you think?

Best regards,

Robert

On Monday, 5 November 2012 at 13:35:26 UTC, Robert wrote:
> Hi there!
>
> I just developed a proof-of-concept implementation of an improved
> std.signals.
>
> Things I did not like about the current implementation:
>
> 1. Passing a delegate to connect, which basically is an extended (void*)
> and assuming it is an object delegate, does not feel quite right in D,
> where we usually try to guarantee correctness by the compiler wherever
> possible.
>
> 2. The restriction that only objects can be connected.
>
> Point 2 does not really bother me, because from my little experience I
> never really had connected anything else than objects to a signal. But
> the restriction that the connected delegate must not be some wrapper, is
> quite a restriction I came to dislike about Qt and even more so with
> this implementation because unlike Qt the signatures have to match
> exactly, you can't omit parameters, like:
> // Qt code
> connect(button, SIGNAL(clicked(bool)), this, SLOT(buttonClicked());
>
> -> In the current implementation buttonClicked would have to take a
> bool.
>
> In addition boost::signals together with boost::bind offered even more
> comfort like passing additional parameters to a slot, which is really
> very, very useful:
>
> for(int i=0; i<buttons.length(); i++) {
> 	buttons[i].clicked.connect(boost::bind(&MyObj::addNumber, this,
>  i));
> }
>
> So I tried to improve std.signals, code:
> (warning at least one bug is remaining, explained below)
>
> https://github.com/eskimor/phobos/tree/new_signal
>
> You can easily connect to an object's method with:
>
> obj.signal.connect!"someMethod"(obj);
>
> instead of the old implementation:
>
> obj.signal.connect(&obj.someMethod);
>
> -> The interface is clean and type safe, all the ugly details are hidden
> from the user. And it is just one character more typing. Simple things
> stay simple.
>
> In addition a method allowing wrapper delegates was added:
>
> class Observer {
> 	void addNumber(int i) {
> 		sum+=i;
> 	}
> 	int sum;
> }
>
> class Button {
> 	Signal!(bool) clicked;
> 	// ...
> }
>
> void main() {
> 	auto b=new Button;
> 	auto o=new Observer;
> 	// Ignore boolean parameter and pass some int:
> 	b.connect!Observer(o, (o, p) { o.addNumber(7); });
> 	// or even:
> 	b.connect!Observer(o, (o, p) => o.addNumber(7));
> 	// For some reason the compiler is not able to deduce "o" being
> 	// Observer, so the !Observer is needed, but it is still very
> 	// neat and readable.
> }
>
> Thanks to D's lamdas the syntax is even more concise as boost::bind and
> far more powerful.
>
> By passing the object explicitly to the delegate, it is possible to
> maintain the 'weak ref' semantics to the target object, while ensuring
> that the delegates context won't be freed.
>
> As a side effect it is now even possible to use struct delegates or even
> any non object delegate. Simply pass null for the obj parameter. It is
> completely safe, the only drawback is that the struct won't be deleted
> until the Button gets destroyed. (Because it holds a reference to the
> struct, by means of the delegate.) But for long lived structs this
> usually is perfectly acceptable.
>
> Implementation:
>
> In my implementation I changed the Signal mixin to be a simple template
> struct, because I hit strange compiler errors with it being a mixin. The
> most prominent:
>
> std/signals.d(443): Error: no overload matches for connect(string
> method,T2) if (is(T2 : Object))
>
> You can find the version triggering these errors at:
>
> https://github.com/eskimor/phobos/tree/new_signal_mixin
>
> Also I did not really get the point why a mixin was used in the first
> place, it does not really gain us anything? What was the reasoning about
> it?
> I almost thought I found the reason, because my implementations suffers
> from unhook not being called, although it was properly registered with
> "rt_attachDisposeEvent(obj, &unhook);", thus causing a segmentation
> fault when the observer gets deleted. I did not really find any
> difference from the original version that could explain this behavior,
> despite the original implementation being a mixin. So I thought, well
> maybe the delegate passed to "rt_attachDisposeEvent(obj, &unhook);" must
> be an object delegate (that's would be why the mixin was needed), but
> after digging in object_.d I did not find any code assuming that the
> delegate was an object delegate. Any ideas on this?
>
> Another thing I'd like to ask Walter, is what the "L1:" label is for in
> connect(), is it just some left over or has it some special internal
> compiler thing meaning?
>
> What do you think?
>
> Best regards,
>
> Robert

Hi!Could you write some examples for struct and non-object delegates?

> Hi!Could you write some examples for struct and non-object delegates?
> 

Sure!

Something like:

struct Observer {
	void observe(int a, int b) {
		// ...
	}
}

void main() {
	Signal!(int, int) s1;
	Signal!int s2
	Observer o;
	s1.connect!Object(null, (null_object, a, b) => o.observe(a, b));
	s2.connect!Object(null, (null_object, a) => o.observe(7, a));

}

Having the delegate accept a null parameter might not be pretty, but I
consider this a good thing, because of the changed semantics: The signal
will keep a reference to the struct now, so the signals weak reference
semantics are no longer in place. (If struct is allocated on the heap,
it won't be freed as long as the signal is alive.)
But it is possible and safe. And if you know what you are doing also
very reasonable.

But the main benefit is not that you can connect to structs (which is a side effect), but that you can use wrapping delegates which do parameter adoptions. That's the killer feature that proved to be so indispensable and neat for me and others.

If really required it would not be to hard to provide an overload of connect() which takes a struct pointer directly, just like the one taking an object, but because of the changed semantics and the rare uses I'd expect, probably not worthwhile. But comments are appreciated.

On Monday, 5 November 2012 at 19:07:13 UTC, Robert wrote:
>
>> Hi!Could you write some examples for struct and non-object delegates?
>> 
>
> Sure!
>
> Something like:
>
> struct Observer {
> 	void observe(int a, int b) {
> 		// ...
> 	}
> }
>
> void main() {
> 	Signal!(int, int) s1;
> 	Signal!int s2
> 	Observer o;
> 	s1.connect!Object(null, (null_object, a, b) => o.observe(a, b));
> 	s2.connect!Object(null, (null_object, a) => o.observe(7, a));
>
> }
>
> Having the delegate accept a null parameter might not be pretty, but I
> consider this a good thing, because of the changed semantics: The signal
> will keep a reference to the struct now, so the signals weak reference
> semantics are no longer in place. (If struct is allocated on the heap,
> it won't be freed as long as the signal is alive.)
> But it is possible and safe. And if you know what you are doing also
> very reasonable.
>
> But the main benefit is not that you can connect to structs (which is a
> side effect), but that you can use wrapping delegates which do parameter
> adoptions. That's the killer feature that proved to be so indispensable
> and neat for me and others.
>
> If really required it would not be to hard to provide an overload of
> connect() which takes a struct pointer directly, just like the one
> taking an object, but because of the changed semantics and the rare uses
> I'd expect, probably not worthwhile. But comments are appreciated.

I am embarrassed a little that a member function of the structure looks like a static function,maybe it would be better if connect took struct pointer directly.I think if struct become immortal it will not be a big trouble,in other
case we have disconnect,that will help us.

Sorry for my awful english

On 11/05/2012 05:36 AM, Robert wrote:
> I just developed a proof-of-concept implementation of an improved
> std.signals.

Cool.

> 1. Passing a delegate to connect, which basically is an extended (void*)
> and assuming it is an object delegate, does not feel quite right in D,
> where we usually try to guarantee correctness by the compiler wherever
> possible.

Not sure I understand why such hatred is rational?

> Point 2 does not really bother me,

It bothers me.

>
> So I tried to improve std.signals, code:
> (warning at least one bug is remaining, explained below)
>
> https://github.com/eskimor/phobos/tree/new_signal

in emit:

if(slot.indirect.ptr) {
 <stuff>
}else{
 <otherstuff>
 slot.indirect.ptr = <thing>
 slot.indirect(i);
}

<stuff> will not be executed more than once?

in addSlot:

you use malloc &friends to allocate objs, but the gc to allocate slots? The correct thing to do here is use allocators, which we will totally have fleshed out by Christmas (right, Andrei?).

in connect [Object]:

<Tony Shalhoub voice> It's not formatted right! </Tony Shalhoub voice>

You might s/is(T2 : Object)/is(T2 == class)/

Also, it looks like it will not delegate to any overriding methods, but I don't know if that is desired. I doubt it differs from existing std.signal.

invariant:

> 	assert(slots_idx==slots.length); // I probably even remove slots_idx all together.

Yes you should. Also, you should assert that slots.length == objs.length.

private:
> union DelegateTypes
> {
> void delegate(void*, T1) indirect;
> void delegate(T1) direct;
> }

Could you explain why indirect must have its first param void* ?

>
> You can easily connect to an object's method with:
>
> obj.signal.connect!"someMethod"(obj);
>
> instead of the old implementation:
>
> obj.signal.connect(&obj.someMethod);

The improvement is where?

> In addition a method allowing wrapper delegates was added:
>
> class Observer {
> 	void addNumber(int i) {
> 		sum+=i;
> 	}
> 	int sum;
> }
>
> class Button {
> 	Signal!(bool) clicked;
> 	// ...
> }
>
> void main() {
> 	auto b=new Button;
> 	auto o=new Observer;
> 	// Ignore boolean parameter and pass some int:
> 	b.connect!Observer(o, (o, p) { o.addNumber(7); });
> 	// or even:
> 	b.connect!Observer(o, (o, p) => o.addNumber(7));
> 	// For some reason the compiler is not able to deduce "o" being
> 	// Observer, so the !Observer is needed, but it is still very
> 	// neat and readable.
> }

Nice.
Should that be b.clicked.connect?

>
> By passing the object explicitly to the delegate, it is possible to
> maintain the 'weak ref' semantics to the target object, while ensuring
> that the delegates context won't be freed.

When would the delegate's context be freed? I think it wouldn't.

>
> As a side effect it is now even possible to use struct delegates or even
> any non object delegate. Simply pass null for the obj parameter. It is
> completely safe, the only drawback is that the struct won't be deleted
> until the Button gets destroyed. (Because it holds a reference to the
> struct, by means of the delegate.) But for long lived structs this
> usually is perfectly acceptable.

I like this capability, but the api could handle the case of non-object delegates better.

<nitpick> Not completely safe if the struct was allocated on the stack </nitpick>

> In my implementation I changed the Signal mixin to be a simple template
> struct, because I hit strange compiler errors with it being a mixin. The
> most prominent:

Welcome to D.

> Also I did not really get the point why a mixin was used in the first
> place, it does not really gain us anything? What was the reasoning about
> it?

So you can do b.connect in the simplest case?

>
> What do you think?

I like the way this is headed.

First a more direct link to the code: https://github.com/eskimor/phobos/blob/new_signal/std/signals.d

> Not sure I understand why such hatred is rational?
I don't know where you saw hate. I just believe that a strong type system is a good idea to avoid many errors, that's one of the reason I am a C++ and D developer and not use some clumsy scripting language where everything blows at run time. I believe that you should have a type safe public API where ever possible and if you need a delegate pointing to an object, you should state that, if it is possible. The only reason Walter probably has chosen this way, is that he most likely had something in mind to lift this restriction in the future. Which would be, in my opinion the only valid reason to leave it the way it is. In the current situation it would not be possible to use signals from safe D, in my version you can mark it as trusted.

I wrote my version to show that very powerful signals are possible within the current language and without proper weak references or something. I also don't know how Walter would have intended to support wrapper delegates for parameter matching in the current design. I can't think of any language feature that would make this possible. (You would only have an opaque pointer to a delegates context, which somehow has a reference to the target object. You need a weak ref to the target object, but a strong ref to the delegates context. If the target ref is contained in an unknown way in the delegates context, there is no way to achieve this.)

> in emit:
> 
> if(slot.indirect.ptr) {
>   <stuff>
> }else{
>   <otherstuff>
>   slot.indirect.ptr = <thing>
>   slot.indirect(i);
> }
> 
> <stuff> will not be executed more than once?
<stuff> is: 	"slot.indirect(cast(void*)(objs[index]), i);"
and gets executed for every slot. I am not sure I understand your
question.

> 
> in addSlot:
> 
> you use malloc &friends to allocate objs, but the gc to allocate slots? The correct thing to do here is use allocators, which we will totally have fleshed out by Christmas (right, Andrei?).
> 

Yes, because I need the references in slots to be considered by the gc, I could equally well have used malloc and afterwards add it to gc roots, but the current implementation is just proof of concept, not a perfectly tweaked one. If people want it in phobos I would most certainly try to make it as good as possible first.
> in connect [Object]:
> 
> <Tony Shalhoub voice> It's not formatted right! </Tony Shalhoub voice>
> 
> You might s/is(T2 : Object)/is(T2 == class)/

well yes, because every class derives from Object. But the requirement I need is that T2 is derived from Object, so it makes the intent more clear.
> 
> Also, it looks like it will not delegate to any overriding methods, but I don't know if that is desired. I doubt it differs from existing std.signal.
> 
> invariant:
> 
> > 	assert(slots_idx==slots.length); // I probably even remove slots_idx all together.
> 
> Yes you should. Also, you should assert that slots.length == objs.length.
-> No I should not, at least in the current implementation this would be wrong. I did not yet made up my mind how I want to implement it in the end, so I left it for now.
> 
> private:
> > union DelegateTypes
> > {
> > void delegate(void*, T1) indirect;
> > void delegate(T1) direct;
> > }
> 
> Could you explain why indirect must have its first param void* ?
Well it does not have to, but I know for sure that a cast to void* is just a reinterpretation. I could equally well have used Object, because at least in my understanding of a non-multiple inheritance hierarchy, the cast to Object should not change the pointer value either. But in fact it does not really matter as you don't do anything with it, other than passing it on. It would avoid some casts though, so I will consider it.
> >
> > You can easily connect to an object's method with:
> >
> > obj.signal.connect!"someMethod"(obj);
> >
> > instead of the old implementation:
> >
> > obj.signal.connect(&obj.someMethod);
> 
> The improvement is where?
That you can not pass some arbitrary slot like:

obj.signal.connect(() { /* my very funny delegate. */ })
which would compile with the standard implementation but would fail
badly at runtime.

> > 	b.connect!Observer(o, (o, p) => o.addNumber(7));

> Nice.
> Should that be b.clicked.connect?
Yes, it should be. Thanks.
> 
> >
> > By passing the object explicitly to the delegate, it is possible to maintain the 'weak ref' semantics to the target object, while ensuring that the delegates context won't be freed.
> 
> When would the delegate's context be freed? I think it wouldn't.
If you keep the delegate references in non gc managed memory, the gc will not see any references to the delegates' context if client code does not keep a reference, which is very likely for lamdas and thus will free it in the next collection cycle. If the delegates context is not the object itself, you need a strong reference to it.
> 
> >
> > As a side effect it is now even possible to use struct delegates or even any non object delegate. Simply pass null for the obj parameter. It is completely safe, the only drawback is that the struct won't be deleted until the Button gets destroyed. (Because it holds a reference to the struct, by means of the delegate.) But for long lived structs this usually is perfectly acceptable.
> 
> I like this capability, but the api could handle the case of non-object delegates better.
> 
> <nitpick> Not completely safe if the struct was allocated on the stack </nitpick>
I knew this was coming. Well I haven't done anything with closures yet, the usage is an escaping of a reference so the compiler should allocate it on the heap, if not than you got a problem, but its not specific to signals -> So I still consider the signal API safe ;-)

> 
> > Also I did not really get the point why a mixin was used in the first place, it does not really gain us anything? What was the reasoning about it?
> 
> So you can do b.connect in the simplest case?
Yes, that's the only reason that comes to mind. I personally think that a struct is a cleaner solution, that also avoids cluttering the containing objects namespace.
> 
> >
> > What do you think?
> 
> I like the way this is headed.

Thanks, and thanks for you review.

On 2012-11-06 13:10, Robert Klotzner wrote:

>> The improvement is where?
> That you can not pass some arbitrary slot like:
>
> obj.signal.connect(() { /* my very funny delegate. */ })
> which would compile with the standard implementation but would fail
> badly at runtime.

And why would it be bad to pass a delegate literal like this?

-- 
/Jacob Carlborg

On Tue, 2012-11-06 at 14:13 +0100, Jacob Carlborg wrote:
> > obj.signal.connect(() { /* my very funny delegate. */ })
> > which would compile with the standard implementation but would fail
> > badly at runtime.
> 
> And why would it be bad to pass a delegate literal like this?
> 
> 

Because the connect method in std.signal interprets the context pointer of the delegate to be an Object. If it is not, your program crashes. To quote the author of _d_toObject (which is used in the original std.signal):


/******************************************
 * Given a pointer:
 *      If it is an Object, return that Object.
 *      If it is an interface, return the Object implementing the
interface.
 *      If it is null, return null.
 *      Else, undefined crash
 */
Object _d_toObject(void* p);

Also check out the bugs section in the html documentation: http://dlang.org/phobos/std_signals.html

On 2012-11-06 14:39, eskimo wrote:

> Because the connect method in std.signal interprets the context pointer
> of the delegate to be an Object. If it is not, your program crashes. To
> quote the author of _d_toObject (which is used in the original
> std.signal):

I've not read the code and I'm not 100% sure of the intentions of std.signal but why not just call the delegate as is?

-- 
/Jacob Carlborg

> I've not read the code and I'm not 100% sure of the intentions of std.signal but why not just call the delegate as is?
> 

Signals are a way of a very loose coupling of components. This loose coupling is the reason why people usually expect weak reference semantics from signals. So people expect a signal connection to simply vanish when the observer object dies, instead of keeping it alive because it holds a reference to it.

The solution at the moment is to hold a reference to the object in memory not seen by the gc. So it gets destroyed if no one else holds a reference. But to avoid calling a dead object's method the signal needs to be notified about this, which is currently only possible for objects. (so no generic delegate support)

The only reason why a simple delegate is not enough, is the weak reference semantics. If it was not for that, a signal would just be a simple array of delegates.

Things get even more complex if you allow wrapper delegates. That's because you have to keep the wrapper delegates context in memory even if nobody else references it. Thus you need a strong ref to the delegates context and a weak ref to the target object, that's why the target object is passed to the delegate, instead of simply containing a ref in the context pointer.

The new implementation supports wrapper delegates, the old one does not and can't with the current API. If we had generic weak references and you would pass a lamda to connect, (which would be syntactically correct), the connection would be gone on the next gc collect cycle.

The reason I think wrapper delegates are really needed is that signals, as already mentioned, are a way of loose coupling. If the signatures of signal and final receiver have to match exactly, the usefulness is seriously limited. And very powerful and elegant techniques are possible if you can pass arbitrary parameters to a slot via the delegate. A very simple example with button.clicked & addNumber was already presented in may first e-mail on this topic, but it is really just the top of the iceberg.

Almost every signal/slot implementation has weak reference semantics, the only one I found not having weak reference semantics was the implementation in Tango, but only because D has no weak references. In some occasions weak reference semantics might not needed (if you have objects connected together with similar life times), but a general working approach should support weak coupling, otherwise you could simply use a delegate array.