www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Compiler bug? (alias sth this; and std.signals)

reply "Joe" <l0calh05t gmx.net> writes:
Ok, i was trying out D and wanted to see if i could create 
something that behaves like a property but adds a change 
notification signal. Doing this, I encountered a hangup which 
might be a bug. So far I have the following:

-----------------------------------------------------------

import std.signals;
import std.stdio;

struct Property
{
	alias get this;
	
	int set(int v) { emit(); return data_ = v; }
	int get() { return data_; }
	
	int opAssign(int v) { return set(v); }
	
	mixin Signal!();

private:
	int data_ = 0;
}

struct Foo
{
	Property prop;
}

class Observer
{
	void watch()
	{
		writeln("Value change observed!");
	}
}

-----------------------------------------------------------

This works:
void main()
{
	Foo f;
	Observer o = new Observer;
	f.prop.connect(&o.watch);
	f.prop = 7;
}

This also works:
void main()
{
	Foo f;
	Observer o = new Observer;
	f.prop = 7;
	writeln(f.prop);
}

This never terminates:
void main()
{
	Foo f;
	Observer o = new Observer;
	f.prop.connect(&o.watch);
	f.prop = 7;
	writeln(f.prop);
}
Nov 12 2012
parent reply "Joe" <l0calh05t gmx.net> writes:
No one? Is this a bug, or am I overlooking something? (entirely 
possible since I didn't use D since trying it once in D1 times)

On Monday, 12 November 2012 at 11:59:50 UTC, Joe wrote:

 Ok, i was trying out D and wanted to see if i could create 
 something that behaves like a property but adds a change 
 notification signal. Doing this, I encountered a hangup which 
 might be a bug. So far I have the following:

 -----------------------------------------------------------

 import std.signals;
 import std.stdio;

 struct Property
 {
 	alias get this;
 	
 	int set(int v) { emit(); return data_ = v; }
 	int get() { return data_; }
 	
 	int opAssign(int v) { return set(v); }
 	
 	mixin Signal!();

 private:
 	int data_ = 0;
 }

 struct Foo
 {
 	Property prop;
 }

 class Observer
 {
 	void watch()
 	{
 		writeln("Value change observed!");
 	}
 }

 -----------------------------------------------------------

 This works:
 void main()
 {
 	Foo f;
 	Observer o = new Observer;
 	f.prop.connect(&o.watch);
 	f.prop = 7;
 }

 This also works:
 void main()
 {
 	Foo f;
 	Observer o = new Observer;
 	f.prop = 7;
 	writeln(f.prop);
 }

 This never terminates:
 void main()
 {
 	Foo f;
 	Observer o = new Observer;
 	f.prop.connect(&o.watch);
 	f.prop = 7;
 	writeln(f.prop);
 }
Nov 13 2012
next sibling parent "Maxim Fomin" <maxim maxim-fomin.ru> writes:
The third version using dmd 2.060 linux crashes. Valgrind shows 
lots of errors (other two are clean) relating to corrupted heap 
caused likely here 
(https://github.com/D-Programming-Language/phobos/blob/master
std/signals.d#L248) 
which is not surprising judging by semi-manual memory management 
in __dtor. I guess either you are abusing library feature or 
found a bug. Consider posting this to Bugzilla.
Nov 13 2012
prev sibling next sibling parent reply eskimo <jfanatiker gmx.at> writes:
Not a compiler bug. A bug in the implementation of std.signals.

 	writeln(f.prop);




Property is a struct and thus it is passed by value, which means that
the signal is copied. Signal does not define a postblit constructor,
because it was intended to be used in classes, so a bitwise copy is
done. Meaning that the internal allocated storage is freed twice.

If used in a class there is no issue with copying a signal, because it
is a mixin and does not exist on its own and an object has reference
semantics.

I am already working on a replacement for std.signals which fixes some
of its shortcomings. It will also fix this one.

Best regards,

Robert
Nov 13 2012
parent reply "Joe" <l0calh05t gmx.net> writes:
On Tuesday, 13 November 2012 at 22:55:38 UTC, eskimo wrote:

 Property is a struct and thus it is passed by value, which 
 means that
 the signal is copied.


But wait! Due to "alias get this;", f.prop shouldn't copy prop
but call get (which it does in the working - second - case)!

How to check? Remove the alias and writeln(f.prop) prints
"Property(7)", with the alias writeln(f.prop) prints "7"
Nov 14 2012
parent reply eskimo <jfanatiker gmx.at> writes:
 
 But wait! Due to "alias get this;", f.prop shouldn't copy prop
 but call get (which it does in the working - second - case)!


-> Also here the struct is copied, but the signal has no content yet and
thus no memory allocation yet occurred. (So no double free)

 
 How to check? Remove the alias and writeln(f.prop) prints
 "Property(7)", with the alias writeln(f.prop) prints "7"


It should copy f.prop and it does. Your check results have another
explanation: writefln is a complete generic templated function, it has
no way of knowing that you want to pass the contained integer instead of
the struct. So by default it takes the struct (it has no requirements
that would lead to trigger the alias this). Eventually it will call some
function taking concrete arguments for printing the data. This concrete
function is not overloaded for your struct so the alias this to int
triggers now and the overload for int is being called. 

But first it is copied to every generic function that might be called on
the way.
Nov 14 2012
parent reply "Joe" <l0calh05t gmx.net> writes:
On Wednesday, 14 November 2012 at 09:31:47 UTC, eskimo wrote:

 But first it is copied to every generic function that might be 
 called on
 the way.


Ok, I guess it just doesn't do what I understood it to do (which 
is too bad, but to be expected with a new language). In any case 
you would appear to be correct, as

void main()
{
	Foo f;
	Observer o = new Observer;
	f.prop.connect(&o.watch);
	f.prop = 7;
	writeln(f.prop.get);
}

works. It just doesn't look like intended.
Nov 15 2012
parent reply eskimo <jfanatiker gmx.at> writes:
On Thu, 2012-11-15 at 09:53 +0100, Joe wrote:

 On Wednesday, 14 November 2012 at 09:31:47 UTC, eskimo wrote:

 But first it is copied to every generic function that might be 
 called on
 the way.


 
 Ok, I guess it just doesn't do what I understood it to do (which 
 is too bad, but to be expected with a new language). In any case 
 you would appear to be correct, as
 
 void main()
 {
 	Foo f;
 	Observer o = new Observer;
 	f.prop.connect(&o.watch);
 	f.prop = 7;
 	writeln(f.prop.get);
 }
 
 works. It just doesn't look like intended.


Well if signal had a proper postblit constructor your original way of
doing it would work. It is just not as efficient, but this is a price
you have to pay. The compiler has no way of knowing that you intended to
pass the contained int from the beginning, when you are actually passing
the containing struct.

But, considering that the alias this triggers only when you are issuing
an operation not supported by the struct itself, it is pretty reasonable
behaviour and everything else would be pretty surprising.

Best regards,

Robert
Nov 15 2012
parent reply "Joe" <l0calh05t gmx.net> writes:
On Thursday, 15 November 2012 at 09:37:55 UTC, eskimo wrote:

 But, considering that the alias this triggers only when you are 
 issuing
 an operation not supported by the struct itself, it is pretty 
 reasonable
 behaviour and everything else would be pretty surprising.

 Best regards,

 Robert


I wonder though why it works at all then, because without the
alias the string conversion *is* supported and produces
"Property(7)".
Nov 15 2012
parent reply eskimo <jfanatiker gmx.at> writes:
:-) Indeed, that is the only thing that surprised me too (but not as
much as in another language, because of D's capabilities). The solution
I think is this overload found in std.format of formatValue:

void formatValue(Writer, T, Char)(Writer w, auto ref T val, ref
FormatSpec!Char f)
if ((is(T == struct) || is(T == union)) && (hasToString!(T, Char) || !
isBuiltinType!T) && !is(T == enum))

-> Its body implements the generic print for structs. It either calls
the structs toString() method if available or if it is a range it uses
formatRange() otherwise it prints its type name with its contained
values. 
But as you can see the templates requirement states !isBuiltinType!T, so
in case of your alias this to an int, it won't be used. So the
implementer of this method most likely had taken into account the
possibility of an alias this to a built in type.

Btw., I love D's readability, it was really easy to find this and to
understand what it does.

Best regards,

Robert

On Thu, 2012-11-15 at 15:11 +0100, Joe wrote:


 I wonder though why it works at all then, because without the
 alias the string conversion *is* supported and produces
 "Property(7)".
Nov 15 2012
parent reply "Joe" <l0calh05t gmx.net> writes:
Why would is(T == struct) be true but !isBuiltinType!T be false?
This seems highly inconsistent. If T is a struct, it is not a
builtin type, and if T is int (also making the condition false),
then Property should never have been passed as a struct, but as
the int gotten via the alias.

On Thursday, 15 November 2012 at 14:52:41 UTC, eskimo wrote:

 :-) Indeed, that is the only thing that surprised me too (but 
 not as
 much as in another language, because of D's capabilities). The 
 solution
 I think is this overload found in std.format of formatValue:

 void formatValue(Writer, T, Char)(Writer w, auto ref T val, ref
 FormatSpec!Char f)
 if ((is(T == struct) || is(T == union)) && (hasToString!(T, 
 Char) || !
 isBuiltinType!T) && !is(T == enum))

 -> Its body implements the generic print for structs. It either 
 calls
 the structs toString() method if available or if it is a range 
 it uses
 formatRange() otherwise it prints its type name with its 
 contained
 values.
 But as you can see the templates requirement states 
 !isBuiltinType!T, so
 in case of your alias this to an int, it won't be used. So the
 implementer of this method most likely had taken into account 
 the
 possibility of an alias this to a built in type.

 Btw., I love D's readability, it was really easy to find this 
 and to
 understand what it does.

 Best regards,

 Robert
Nov 16 2012
parent eskimo <jfanatiker gmx.at> writes:
 Why would is(T == struct) be true but !isBuiltinType!T be false?


Exactly because of alias this. Your type is a struct and an integer at
the same time, so to speak. At least it behaves that way and that's what
its all about. If if looks like a duck, quacks like a duck, ... it is a
duck ;-)

 This seems highly inconsistent. If T is a struct, it is not a
 builtin type, and if T is int (also making the condition false),
 then Property should never have been passed as a struct, but as
 the int gotten via the alias.


Your struct is both at the same time, it stops being both as soon as it
is copied to a function argument expecting an int, but not any sooner.
It is always passed as the struct, that's also what you are doing: You
pass the struct to writefln. It only allows being treated as an int,
which also means it can be copied to an int argument of a function. But
this happens even later in the call chain, the above mentioned function
is also a template, so it sees the struct not the naked int, but it does
the expected thing because of the seemingly contradictory checks. So
another overload is used, which expects builtin types.

With your alias this your struct is a struct and a builtin at the same
time. Without the alias this feature, you would be right that the check
makes no sense.
Nov 16 2012
prev sibling parent eskimo <jfanatiker gmx.at> writes:
In your particular case, which this definitely is a bug and if only for
missing documentation that you should only use it from classes, you
might regardless be better of with writeln(f.prop.get) because you avoid
the needless copy with memory allocation/deallocation (if done
correctly) of the signal.

Passing f.prop directly should be possible for any function that takes
an int, but not for generic template functions because then you pass the
struct directly.

Best regards,

Robert
Nov 14 2012