http://d.puremagic.com/issues/show_bug.cgi?id=9347


Denis Shelomovskij <verylonglogin.reg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |verylonglogin.reg@gmail.com


--- Comment #1 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-02-27 13:49:19 MSK ---
(In reply to comment #0)
> The current std.signals implementation has various short comings.

For closure related shortcomings see Issue 9603. I also would like to have a compiler solution of it instead of a library one (see Issue 9603 Comment 1).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9347



--- Comment #2 from jfanatiker@gmx.at 2013-02-28 05:43:16 PST ---
(In reply to comment #1)
> (In reply to comment #0)
> > The current std.signals implementation has various short comings.
> 
> For closure related shortcomings see Issue 9603. I also would like to have a compiler solution of it instead of a library one (see Issue 9603 Comment 1).

The new implementation supports wrapping delegates, this is fact was a very important feature for me.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9347



--- Comment #3 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-02-28 18:11:02 MSK ---
(In reply to comment #2)
> The new implementation supports wrapping delegates, this is fact was a very important feature for me.

But it doesn't do it correctly as it can't because of absence of stuff for controlling/watching delegate lifetime in D. See bug 9603 comment 2 for a test case.

Also new implementation doesn't handle threading issue 9606.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9347



--- Comment #4 from jfanatiker@gmx.at 2013-02-28 07:48:50 PST ---
(In reply to comment #3)
> (In reply to comment #2)
> > The new implementation supports wrapping delegates, this is fact was a very important feature for me.
> 
> But it doesn't do it correctly as it can't because of absence of stuff for controlling/watching delegate lifetime in D. See bug 9603 comment 2 for a test case.
It does, because it holds a strong ref to the delegate context, but a weak ref to the target object.

The following:
  obj.actionDone.connect(i => watch(i));

would become:
  obj.actionDone.connect!Observer(this, (o, i) => o.watch(i));

with the new implementation. Might not be as pretty, but is the only way I could think of to make wrapper delegates work properly. This also enables the signal to provide the strongConnect() methods which allows connecting to non object targets, but with strong ref semantics.

The signal needs to have a strong ref to the delegate's context, but also needs to have a weak ref to the target object. In addition the delegate's context can not contain a reference to the target object, because this would simply destroy the weak ref semantics. Decoupling the two and passing the object to the delegate via a parameter, seems to be the only way to solve this issue. and at least in my opinion not an entirely bad one.



> 
> Also new implementation doesn't handle threading issue 9606.

Thanks for pointing this out, I will fix it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9347



--- Comment #5 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-03-16 15:03:59 MSK ---
(In reply to comment #4)
> > Also new implementation doesn't handle threading issue 9606.
> 
> Thanks for pointing this out, I will fix it.

Probably weak references (issue 4151) have to be implemented first.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9347



--- Comment #6 from jfanatiker@gmx.at 2013-03-16 05:34:52 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> > > Also new implementation doesn't handle threading issue 9606.
> > 
> > Thanks for pointing this out, I will fix it.
> 
> Probably weak references (issue 4151) have to be implemented first.

They would of course be nice and make the signal implementation trivial. But they are not strictly required, I already tackled the issue in the current master of my implementation. (Not yet tested, won't even compile, but the basic concept should work)

Best regards,
Robert

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------