After implementing automatic reference counting for resource management and spending the last two days with trying to figure out where the crashes and leaks come from and write workarounds for the corresponding compiler bugs, I came to the conclusion that this approach is futile. The code base is too large, as is the number of patterns that lead to compiler bugs.

Since the number of issues unusually high, instead of posting one bug report for each (posted two), I compiled a small test suite with a number of bug patterns that I discovered (and some working patterns). The list is probably not complete and I know that some cases that are OK in the test suite now failed for me in real code.

Anyway, do you think it makes sense to post this test suite as a meta bug for reference counting (excluding phobos RefCounted), so that somebody can look into it as a whole?

I get the feeling that otherwise it might take a very long time until everything is stable in this area - and IMO this is an extremely important area for anyone who needs to (partially or fully) avoid the GC. And, last but not least, just like linker crashes and ICEs, it may cause a very fragile impression for anyone who encounters this.

Test suite: http://pastebin.com/niZwRKpc
(Run with "dmd -g -run" or "dmd -g -version=BAILOUT -run")

S. Ludwig:

> Since the number of issues unusually high, instead of posting one bug report for each (posted two), I compiled a small test suite with a number of bug patterns that I discovered (and some working patterns).

I suggest to also post every single bug in single Bugzilla entries. Fixing single bugs is already a not easy thing to do.

Bye,
bearophile

Am 18.02.2012 11:32, schrieb Sönke Ludwig:
> After implementing automatic reference counting for resource management
> and spending the last two days with trying to figure out where the
> crashes and leaks come from and write workarounds for the corresponding
> compiler bugs, I came to the conclusion that this approach is futile.
> The code base is too large, as is the number of patterns that lead to
> compiler bugs.
>
> Since the number of issues unusually high, instead of posting one bug
> report for each (posted two), I compiled a small test suite with a
> number of bug patterns that I discovered (and some working patterns).
> The list is probably not complete and I know that some cases that are OK
> in the test suite now failed for me in real code.
>
> Anyway, do you think it makes sense to post this test suite as a meta
> bug for reference counting (excluding phobos RefCounted), so that
> somebody can look into it as a whole?
>
> I get the feeling that otherwise it might take a very long time until
> everything is stable in this area - and IMO this is an extremely
> important area for anyone who needs to (partially or fully) avoid the
> GC. And, last but not least, just like linker crashes and ICEs, it may
> cause a very fragile impression for anyone who encounters this.
>
> Test suite: http://pastebin.com/niZwRKpc
> (Run with "dmd -g -run" or "dmd -g -version=BAILOUT -run")
>

You did not implement the assignment operator for your refcounted struct. If you do that it should heavily reduce the number of issues you have. I actually only run into 1 severe issue with refcounting and that is that array slice copying does not call any copy constructor / assignment operator

On 2012-02-18 13:25:54 +0000, Benjamin Thaut <code@benjamin-thaut.de> said:

> Am 18.02.2012 11:32, schrieb Sönke Ludwig:
>> After implementing automatic reference counting for resource management
>> and spending the last two days with trying to figure out where the
>> crashes and leaks come from and write workarounds for the corresponding
>> compiler bugs, I came to the conclusion that this approach is futile.
>> The code base is too large, as is the number of patterns that lead to
>> compiler bugs.
>> 
>> Since the number of issues unusually high, instead of posting one bug
>> report for each (posted two), I compiled a small test suite with a
>> number of bug patterns that I discovered (and some working patterns).
>> The list is probably not complete and I know that some cases that are OK
>> in the test suite now failed for me in real code.
>> 
>> Anyway, do you think it makes sense to post this test suite as a meta
>> bug for reference counting (excluding phobos RefCounted), so that
>> somebody can look into it as a whole?
>> 
>> I get the feeling that otherwise it might take a very long time until
>> everything is stable in this area - and IMO this is an extremely
>> important area for anyone who needs to (partially or fully) avoid the
>> GC. And, last but not least, just like linker crashes and ICEs, it may
>> cause a very fragile impression for anyone who encounters this.
>> 
>> Test suite: http://pastebin.com/niZwRKpc
>> (Run with "dmd -g -run" or "dmd -g -version=BAILOUT -run")
>> 
> 
> You did not implement the assignment operator for your refcounted struct. If you do that it should heavily reduce the number of issues you have. I actually only run into 1 severe issue with refcounting and that is that array slice copying does not call any copy constructor / assignment operator

It'd be nice though if the default assignment operator for structs would be generated like this:

	if (&other != &this)
	{
		~this(); // destruct
		memcpy(&this, &other, other.sizeof); // blit
		this(this); // postblit
	}

because this is the correct thing to do on assignment 99% of the time. So you'd have less boilerplate code to write.


-- 
Michel Fortin
michel.fortin@michelf.com
http://michelf.com/

Am 18.02.2012 14:25, schrieb Benjamin Thaut:
> Am 18.02.2012 11:32, schrieb Sönke Ludwig:
>> After implementing automatic reference counting for resource management
>> and spending the last two days with trying to figure out where the
>> crashes and leaks come from and write workarounds for the corresponding
>> compiler bugs, I came to the conclusion that this approach is futile.
>> The code base is too large, as is the number of patterns that lead to
>> compiler bugs.
>>
>> Since the number of issues unusually high, instead of posting one bug
>> report for each (posted two), I compiled a small test suite with a
>> number of bug patterns that I discovered (and some working patterns).
>> The list is probably not complete and I know that some cases that are OK
>> in the test suite now failed for me in real code.
>>
>> Anyway, do you think it makes sense to post this test suite as a meta
>> bug for reference counting (excluding phobos RefCounted), so that
>> somebody can look into it as a whole?
>>
>> I get the feeling that otherwise it might take a very long time until
>> everything is stable in this area - and IMO this is an extremely
>> important area for anyone who needs to (partially or fully) avoid the
>> GC. And, last but not least, just like linker crashes and ICEs, it may
>> cause a very fragile impression for anyone who encounters this.
>>
>> Test suite: http://pastebin.com/niZwRKpc
>> (Run with "dmd -g -run" or "dmd -g -version=BAILOUT -run")
>>
>
> You did not implement the assignment operator for your refcounted
> struct. If you do that it should heavily reduce the number of issues you
> have. I actually only run into 1 severe issue with refcounting and that
> is that array slice copying does not call any copy constructor /
> assignment operator

Can you show your assignment operator?

For me it just causes 3 cases to crash, which were simply failing before.

But apart from that, if the assignment operator is defined differently than what Michel suggested that's of course one of the bugs.

Am 18.02.2012 14:15, schrieb bearophile:
> S. Ludwig:
>
>> Since the number of issues unusually high, instead of posting one bug
>> report for each (posted two), I compiled a small test suite with a
>> number of bug patterns that I discovered (and some working patterns).
>
> I suggest to also post every single bug in single Bugzilla entries. Fixing single bugs is already a not easy thing to do.
>
> Bye,
> bearophile

Some cases are for existing bugs, but many of them are difficult to tell apart. I would just like to avoid useless overhead if possible.

Updated test with two different opAssign versions:

http://pastebin.com/3EcZf4DS

On 2/18/12 7:59 AM, Michel Fortin wrote:
> It'd be nice though if the default assignment operator for structs would
> be generated like this:
>
> if (&other != &this)
> {
> ~this(); // destruct
> memcpy(&this, &other, other.sizeof); // blit
> this(this); // postblit
> }
>
> because this is the correct thing to do on assignment 99% of the time.
> So you'd have less boilerplate code to write.

Heh, we discussed this years ago but it was forgotten.

Andrei

On Sat, 18 Feb 2012 11:32:07 +0100, Sönke Ludwig <ludwig@informatik.uni-luebeck.de> wrote:

> After implementing automatic reference counting for resource management and spending the last two days with trying to figure out where the crashes and leaks come from and write workarounds for the corresponding compiler bugs, I came to the conclusion that this approach is futile. The code base is too large, as is the number of patterns that lead to compiler bugs.
>
> Since the number of issues unusually high, instead of posting one bug report for each (posted two), I compiled a small test suite with a number of bug patterns that I discovered (and some working patterns). The list is probably not complete and I know that some cases that are OK in the test suite now failed for me in real code.
>
> Anyway, do you think it makes sense to post this test suite as a meta bug for reference counting (excluding phobos RefCounted), so that somebody can look into it as a whole?
>
> I get the feeling that otherwise it might take a very long time until everything is stable in this area - and IMO this is an extremely important area for anyone who needs to (partially or fully) avoid the GC. And, last but not least, just like linker crashes and ICEs, it may cause a very fragile impression for anyone who encounters this.
>
> Test suite: http://pastebin.com/niZwRKpc
> (Run with "dmd -g -run" or "dmd -g -version=BAILOUT -run")
>

How about integrating it into dmd's testsuite with
failing parts deactivated based on the bug number.