On Monday, November 05, 2012 14:36:55 Robert wrote:
> 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?

It's a label. There's nothing special about it. You jump to them with gotos. However, there's no goto in that function (and D does not support using gotos across functions), so it doesn't do anything. Presumably, it's left over from some previous refactoring.

- Jonathan M Davis

Thought so. Thank you!
>  Presumably, it's left over from
> some previous refactoring.
> 
> - Jonathan M Davis

On Nov 5, 2012, at 5:36 AM, Robert <jfanatiker@gmx.at> wrote:
> 
> I just developed a proof-of-concept implementation of an improved std.signals.

Make sure that the behavior when calling std.signals operations from within a callback is well-defined.  It should either explicitly disallow this or, ideally, support it with deterministic results.

On Tue, 2012-11-06 at 11:16 -0800, Sean Kelly wrote:
> On Nov 5, 2012, at 5:36 AM, Robert <jfanatiker@gmx.at> wrote:
> > 
> > I just developed a proof-of-concept implementation of an improved std.signals.
> 
> Make sure that the behavior when calling std.signals operations from within a callback is well-defined.  It should either explicitly disallow this or, ideally, support it with deterministic results.

I am pretty sure I don't understand what you mean. Calling std.signals methods from an invoked slot is as safe as from any other random function. Am I missing something?

On Nov 6, 2012, at 2:09 PM, eskimo <jfanatiker@gmx.at> wrote:

> On Tue, 2012-11-06 at 11:16 -0800, Sean Kelly wrote:
>> On Nov 5, 2012, at 5:36 AM, Robert <jfanatiker@gmx.at> wrote:
>>> 
>>> I just developed a proof-of-concept implementation of an improved std.signals.
>> 
>> Make sure that the behavior when calling std.signals operations from within a callback is well-defined.  It should either explicitly disallow this or, ideally, support it with deterministic results.
> 
> I am pretty sure I don't understand what you mean. Calling std.signals methods from an invoked slot is as safe as from any other random function. Am I missing something?

What happens when a callback removes itself from the signal when called?  What happens when it adds a new callback?  Since these operations modify the list that's currently being iterated across, the implementation typically has to be done in a way that accounts for this.

Completely working version online, all unit tests pass:

https://github.com/eskimor/phobos/blob/new_signal/std/signals.d


Although I will add some more, especially for the new functionality.

I don't expect signals in D being as overused as in Qt (because we have delegates which are often the better choice), but nevertheless very often signals will be declared for a class but not used for many objects, so I think it is important to reduce the memory requirements of an empty signal to a bare minimum.

I intent to do this by using a single array for both the delegates and the object pointers, this will also simplify the implementation. For this to work I would put everything in a standard D array (no malloc) and will simply invert the object pointer so it won't keep the object in memory on garbage collection. This could also save us the slot_idx variable, by using assumeSafeAppend.

I don't think that using malloc for this single array will buy me anything, because I would have to add elements of the array to the gc roots, which can't really be more efficient in any way.

> What happens when a callback removes itself from the signal when called?  What happens when it adds a new callback?  Since these operations modify the list that's currently being iterated across, the implementation typically has to be done in a way that accounts for this.

Damn it. That's why the check for null was there. Thank you! So in the old implementation it would simply miss to call a slot in the list on removal, in the new implementation it would crash currently. (Because of the removed check) But also the original behavior is not really acceptable. Also this means that:

 slots[slots_idx] = null;        // not strictly necessary

in the original implementation was not as effectless as it seemed.

Thanks for this very good hint, I will improve it in this regard!