Re: std.signals regressions
Am Sat, 13 Jul 2013 22:37:45 +0200 schrieb Robert jfanati...@gmx.at: But one comment though: Do you really need string mixins? I think Signal!int mysignal; is a much nicer syntax in D than using I agree and you don't need the string mixin, it is just for convenience. The signal itself is a struct. But what was important to me was that one can easily restrict emitting the signal to the containing class. Now that you mention it I also remember that issue. That's a good reason to use the string/template mixin, although for me clearer syntax had higher priority.
Re: std.signals regressions
On Saturday, 13 July 2013 at 01:24:11 UTC, Andrej Mitrovic wrote: On 7/13/13, David d...@dav1d.de wrote: Bad timing, just got our[1] own implementation. If I have time, I'll play around with it! Thanks for the great work, maybe we can get something working into phobos... [1]https://github.com/AndrejMitrovic/new_signals Disclaimer: This is the work of Johannes Pfau which I've just hacked and butchered a little bit (and also updated for 2.064) because we needed std.signals that doesn't crash that often. Wow, I wasn't aware of either this implementation nor this thread. Thanks for sharing.
Re: std.signals regressions
[1]https://github.com/AndrejMitrovic/new_signals Hmm, ok this implementation does not implement weak ref semantics, but it does implement some fancy features like enabling/disabling emission, adding slots at a specified position and stuff. At least for enabling/disabling I decided not to include it for now, to keep the implementation simple and I also think it is not really needed. But in general what features of your new_signals implentation do you really need/like that are not present in my implementation? Ideally with use cases so I can get an idea? Thanks for the feedback! Best regards, Robert
Re: std.signals regressions
Am 13.07.2013 10:14, schrieb Robert: [1]https://github.com/AndrejMitrovic/new_signals Hmm, ok this implementation does not implement weak ref semantics, but it does implement some fancy features like enabling/disabling emission, adding slots at a specified position and stuff. At least for enabling/disabling I decided not to include it for now, to keep the implementation simple and I also think it is not really needed. But in general what features of your new_signals implentation do you really need/like that are not present in my implementation? Ideally with use cases so I can get an idea? Thanks for the feedback! Best regards, Robert What I like? It works and it was a opt-in for my old code (s/mixin Signal/Signal/), but you need to probably break the API-compatability with the weak ref thing. I don't really need the stop propagation, insert before/after, but I can see where it can come in handy. So far, I was really really happy when Andrej came up with this code, I really really needed something else than the old std.signals and this does exactly what I needed.
Re: std.signals regressions
Am Sat, 13 Jul 2013 15:47:35 +0200 schrieb David d...@dav1d.de: Am 13.07.2013 10:14, schrieb Robert: [1]https://github.com/AndrejMitrovic/new_signals Hmm, ok this implementation does not implement weak ref semantics, but it does implement some fancy features like enabling/disabling emission, adding slots at a specified position and stuff. At least for enabling/disabling I decided not to include it for now, to keep the implementation simple and I also think it is not really needed. But in general what features of your new_signals implentation do you really need/like that are not present in my implementation? Ideally with use cases so I can get an idea? Thanks for the feedback! Best regards, Robert What I like? It works and it was a opt-in for my old code (s/mixin Signal/Signal/), but you need to probably break the API-compatability with the weak ref thing. I don't really need the stop propagation, insert before/after, but I can see where it can come in handy. So far, I was really really happy when Andrej came up with this code, I really really needed something else than the old std.signals and this does exactly what I needed. When I initially wrote that code I wanted it to support everything that usual C/C++ signal implementations support. I looked at what features glib offered and implemented most of them. I was never sure whether a list as the internal data structure was the right choice though. I wanted to have a look at boost's signal implementation and especially do some more performance testing. Then I somehow forgot about it and it never seemed like there was a big demand for signals so I never finished that work. Anyway feel free to take anything you need from that code. I unfortunately don't have much time right now to help integrate those implementations but merging them would probably be a good idea. But one comment though: Do you really need string mixins? I think Signal!int mysignal; is a much nicer syntax in D than using mixins / string mixins. And I think it's also quite important that a signal implementation works with all of D's callable types - especially functions and delegates but opCall is nice as well.
Re: std.signals regressions
But one comment though: Do you really need string mixins? I think Signal!int mysignal; is a much nicer syntax in D than using I agree and you don't need the string mixin, it is just for convenience. The signal itself is a struct. But what was important to me was that one can easily restrict emitting the signal to the containing class. This is where you need some kind of mixin to avoid boilerplate. But if you don't need that or want to write the boilerplate yourself you can just use the FullSignal struct directly. I wanted to use template mixins because of the little less cluttered syntax, but as it turns out they are quite buggy, so I changed it to a string mixin with the added flexibility that you can select the protection of FullSignal, with private being the default. The syntax does not turn out to be that bad: mixin(signal!(string, int)(valueChanged)); or if you want for example protected instead of private: mixin(signal!(string, int)(valueChanged, protected)); If you don't want to use the mixin and don't care that everyone can emit the signal: FullSignal!(string, int) valueChanged; You see the mixin is just a feature, if you don't like it - don't use it :-) mixins / string mixins. And I think it's also quite important that a signal implementation works with all of D's callable types - especially functions and delegates but opCall is nice as well. It does work with all callable types: Delegates/functions and by means of delegates with any callable type. Best regards, Robert
Re: std.signals regressions
On Friday, 12 July 2013 at 22:35:06 UTC, David wrote:
Am 12.07.2013 23:47, schrieb Robert:
I just finished a new implementation, replacing the template
mixin with
a string mixin. I also changed the name from signals2 to
signal. You can
find it here:
https://github.com/phobos-x/phobosx/blob/master/source/phobosx/signal.d
All unittests pass, you don't need any patched compiler. I
still have to
add some more checks and do some polishing, I will also put it
in the
dub registry. But you seem to have an urgent need, so feel
free to try
it out immediately - Be my pre-alpha Tester :-)
Best regards,
Robert
Bad timing, just got our[1] own implementation.
If I have time, I'll play around with it! Thanks for the great
work,
maybe we can get something working into phobos...
[1]https://github.com/AndrejMitrovic/new_signals
Having tried this I can't get ref parameters to work as function
arguments, are they supported?
Example:
struct KeyEvent {}
public Signal!(ref KeyEvent) onKeyDown;
connect, emit ...
public void keyDownHandler(ref KeyEvent keyEvent) { }
Works fine without ref.
Re: std.signals regressions
On 7/14/13, Damian damian...@hotmail.co.uk wrote: Having tried this I can't get ref parameters to work as function arguments, are they supported? This is a language issue, you can't pass 'ref' as a template parameter.
Re: std.signals regressions
I just finished a new implementation, replacing the template mixin with a string mixin. I also changed the name from signals2 to signal. You can find it here: https://github.com/phobos-x/phobosx/blob/master/source/phobosx/signal.d All unittests pass, you don't need any patched compiler. I still have to add some more checks and do some polishing, I will also put it in the dub registry. But you seem to have an urgent need, so feel free to try it out immediately - Be my pre-alpha Tester :-) Best regards, Robert
Re: std.signals regressions
Am 12.07.2013 23:47, schrieb Robert: I just finished a new implementation, replacing the template mixin with a string mixin. I also changed the name from signals2 to signal. You can find it here: https://github.com/phobos-x/phobosx/blob/master/source/phobosx/signal.d All unittests pass, you don't need any patched compiler. I still have to add some more checks and do some polishing, I will also put it in the dub registry. But you seem to have an urgent need, so feel free to try it out immediately - Be my pre-alpha Tester :-) Best regards, Robert Bad timing, just got our[1] own implementation. If I have time, I'll play around with it! Thanks for the great work, maybe we can get something working into phobos... [1]https://github.com/AndrejMitrovic/new_signals
Re: std.signals regressions
On 7/13/13, David d...@dav1d.de wrote: Bad timing, just got our[1] own implementation. If I have time, I'll play around with it! Thanks for the great work, maybe we can get something working into phobos... [1]https://github.com/AndrejMitrovic/new_signals Disclaimer: This is the work of Johannes Pfau which I've just hacked and butchered a little bit (and also updated for 2.064) because we needed std.signals that doesn't crash that often.
Re: std.signals regressions - std.signals2
On Sun, 2013-06-16 at 15:25 +0200, David wrote: I looked into the new std.signals today: Thanks for having a look :-) https://github.com/eskimor/phobos/blob/new_signal/std/signals.d It is completly unusable, mixin Signal!() x is blocked by bug, it doesn't compile due to a wrong type (relativly easy fix), then when using -unittest it fails to compile, compiler also doesn't give any hints what exactly fails. Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally. Removing these unittests makes it at least compile, but the other unittests fail (assert). Removing also these unittests (hoping it still works), nope it doesn't signals aren't triggered at all. Are there any plans on improving this situation with std.signals or the new implementation, currently both are getting less useable every day... (e.g. signal.disconnect(my_handler) disconnects all, yay what a joy) There are plans absolutely, to be concrete: I plan to make it ready in July. There is one blocker currently: http://d.puremagic.com/issues/show_bug.cgi?id=8441 but there is already an aging pull request which fixes it: https://github.com/D-Programming-Language/dmd/pull/1660 (4 months old already) I hope it will be merged soon, then there is hopefully nothing that hinders my plans. The very least it should become ready during summer. I am hijacking this thread in hope someone will read that ;) Hopefully also someone of the new std.signals developers. I almost missed it ;-) Sorry about your bad experiences with the current version. Best regards, Robert
Re: std.signals regressions - std.signals2
https://github.com/eskimor/phobos/blob/new_signal/std/signals.d It is completly unusable, mixin Signal!() x is blocked by bug, it doesn't compile due to a wrong type (relativly easy fix), then when using -unittest it fails to compile, compiler also doesn't give any hints what exactly fails. Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally. Good to know, but that is already a few monce old, or? I remember seeing CAS in the code though Removing these unittests makes it at least compile, but the other unittests fail (assert). Removing also these unittests (hoping it still works), nope it doesn't signals aren't triggered at all. Are there any plans on improving this situation with std.signals or the new implementation, currently both are getting less useable every day... (e.g. signal.disconnect(my_handler) disconnects all, yay what a joy) There are plans absolutely, to be concrete: I plan to make it ready in July. There is one blocker currently: http://d.puremagic.com/issues/show_bug.cgi?id=8441 but there is already an aging pull request which fixes it: https://github.com/D-Programming-Language/dmd/pull/1660 (4 months old already) I hope it will be merged soon, then there is hopefully nothing that hinders my plans. The very least it should become ready during summer. This blocker wasn't a problem, I used the FullSignal, I can live with that, I don't like to have a private emit method anyways (in my opinion, private is always wrong) I am hijacking this thread in hope someone will read that ;) Hopefully also someone of the new std.signals developers. I almost missed it ;-) Sorry about your bad experiences with the current version. Hehe wouldn't say it is your fault, obviously it worked at some point (I guess)... I blame DMD, Ranges and the GC of course ;) - David
Re: std.signals regressions - std.signals2
Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally. Good to know, but that is already a few monce old, or? I remember seeing CAS in the code though It is yes. It is blocked and I got busy with other stuff, so I did put it on hold. Current master never worked, commits prior to (and including): 04c951e34623e9365a3874c89f43eb997a7b376c should work (if you drop the mixin part). The Heisenbug is still present though. (That's the reason for the CAS stuff) This blocker wasn't a problem, I used the FullSignal, I can live with that, I don't like to have a private emit method anyways (in my opinion, private is always wrong) Then you might want to try out an older version, see above. Hehe wouldn't say it is your fault, obviously it worked at some point (I guess)... I blame DMD, Ranges and the GC of course ;) Well master never worked (it is work in progress), but older versions did and should still. Best regards, Robert
Re: std.signals regressions - std.signals2
Am 17.06.2013 22:23, schrieb Robert: Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally. Good to know, but that is already a few monce old, or? I remember seeing CAS in the code though It is yes. It is blocked and I got busy with other stuff, so I did put it on hold. Current master never worked, commits prior to (and including): 04c951e34623e9365a3874c89f43eb997a7b376c should work (if you drop the mixin part). The Heisenbug is still present though. (That's the reason for the CAS stuff) Thanks, might try again, because for what I wanna do now, I really need strongConnect, so far it wasn't an issue but it's getting one...
Re: std.signals regressions - std.signals2
I looked into the new std.signals today: https://github.com/eskimor/phobos/blob/new_signal/std/signals.d It is completly unusable, mixin Signal!() x is blocked by bug, it doesn't compile due to a wrong type (relativly easy fix), then when using -unittest it fails to compile, compiler also doesn't give any hints what exactly fails. Removing these unittests makes it at least compile, but the other unittests fail (assert). Removing also these unittests (hoping it still works), nope it doesn't signals aren't triggered at all. Are there any plans on improving this situation with std.signals or the new implementation, currently both are getting less useable every day... (e.g. signal.disconnect(my_handler) disconnects all, yay what a joy) I am hijacking this thread in hope someone will read that ;) Hopefully also someone of the new std.signals developers.
Re: std.signals regressions
14.06.2013 18:31, David пишет:
This code currently fails with a RangeError (used to work in 2.062)
// http://dpaste.dzfl.pl/332a71ec
import std.stdio;
import std.signals;
struct X {
mixin Signal!();
}
class O {
void m() {}
}
void main() {
O o = new O();
X[string] aa;
aa[o] = X.init;
aa[o].connect(o.m);
/*{ // 20
X x = aa[o];
}*/
}
If you take the uncomment the block at line 20 you end up with a
segmentation fault, also worked in 2.062. Both times the problem is in
the __dtor (segfault happens in the call to _d_toObject(stor.ptr).
Changing struct to class and X.init to new X() it seems to work
as it should.
Is this worth a bugreport or was the old behaviour never intended?
This bug (I consider it one) broke quite a few lines of code... If the
old behaviour was never intended, why wasn't it documentated then... oh
well I am drifting into another D rant here...
http://dlang.org/phobos/std_signals.html#Signal
Mixin to create a signal within a class object.
So your `X` must be a class.
Also don't use std.signals - it's an incorrect and dangerous mess (see
bug tracker).
--
Денис В. Шеломовский
Denis V. Shelomovskij
Re: std.signals regressions
15.06.2013 14:19, David пишет: http://dlang.org/phobos/std_signals.html#Signal Mixin to create a signal within a class object. So your `X` must be a class. Oh right I have totally overseen that, still used to work really well with 2.063 It can work only by accident iff the struct isn't moved in memory no matter what compiler version you use. Also don't use std.signals - it's an incorrect and dangerous mess (see bug tracker). Provide me something better in phobos and so far it works pretty good (except the update breaking struct-signals) Currently there is no working signals implementation in Phobos. The worst thing about the current implementation is it will fail you program accidentally in rare cases. Don't use it. For issues, see: http://d.puremagic.com/issues/show_bug.cgi?id=4150 http://d.puremagic.com/issues/show_bug.cgi?id=9606 http://d.puremagic.com/issues/show_bug.cgi?id=9603 Also post to NG instead of emailing directly to allow others to follow the discussion. Also if you _do_ need working signals I can help a with implementation. -- Денис В. Шеломовский Denis V. Shelomovskij - Regarding E-mailing, sorry about that, I hate the thunderbird answer button It can work only by accident iff the struct isn't moved in memory no matter what compiler version you use. This worked for at least 3 compiler versions now I am aware of all of these bugs, none are critical if you know them 4150: use only class methods as event handlers 9606: emitting signals from a different thread would segfault in my application anayways (opengl code) 9603: basically same as 4150 Also wasn't there a std.signals2 somewhere...
std.signals regressions
This code currently fails with a RangeError (used to work in 2.062)
// http://dpaste.dzfl.pl/332a71ec
import std.stdio;
import std.signals;
struct X {
mixin Signal!();
}
class O {
void m() {}
}
void main() {
O o = new O();
X[string] aa;
aa[o] = X.init;
aa[o].connect(o.m);
/*{ // 20
X x = aa[o];
}*/
}
If you take the uncomment the block at line 20 you end up with a
segmentation fault, also worked in 2.062. Both times the problem is in
the __dtor (segfault happens in the call to _d_toObject(stor.ptr).
Changing struct to class and X.init to new X() it seems to work
as it should.
Is this worth a bugreport or was the old behaviour never intended?
This bug (I consider it one) broke quite a few lines of code... If the
old behaviour was never intended, why wasn't it documentated then... oh
well I am drifting into another D rant here...
