www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Implementing SmartPtr - compiler bug?

reply Shachar Shemesh <shachar weka.io> writes:
For reasons I won't go into (but should be fairly obvious), I am trying 
to write code that does not rely on the garbage collector. As such, I'm 
using reference counting structs allocated on a pool.

To keep things sane, I'm trying to use RAII semantics, and to that end, 
a smart pointer that calls "incref" and "decref" when appropriate.

The SmartPtr struct implements this(T*), this(SmartPtr), this(this), 
~this(), opAssign(SmartPtr), opAssign(ref SmartPtr) and opAssign(T*). To 
the best of my understanding this should be enough to catch all relevant 
cases.

All works fine except one case:

Foo foo;

SmartPtr!Foo[4] array;

array[0] = foo;

assert(foo.refcount == 1);

array = array.init;

assert(foo.refcount == 0, "This assert fails");

I am compiling this on dmd v2.065

Switching the init line to "array[] = SmartPtr!Foo.init" makes the 
second assert pass.

To the best of my understanding, "array = array.init" should always be 
equivalent to "array[] = typeof(array[0]).init" for static arrays. Am I 
missing something? Is this a compiler bug?

Thanks,
Shachar
Oct 27 2014
next sibling parent ketmar via Digitalmars-d-learn <digitalmars-d-learn puremagic.com> writes:
just checked this with gdc and dmd head. interesting that

  a =3D a.init;

works the same in gdc and in dmd: just calls destructors for structs.
but doing

  a[] =3D S.init

works different. gdc does the same as in the first case: just calling
destructors for all array elements. but dmd head calls postblits
before destructing for all array elements (and then calls destructors
for this copied elements). i.e.

first case:

  destructor: bff51e2b
  destructor: bff51e2a
  destructor: bff51e29
  destructor: bff51e28

second case:

  postblit: bff9405c
  destructor: bff94028
  postblit: bff9405d
  destructor: bff94028
  postblit: bff9405e
  destructor: bff94028
  postblit: bff9405f
  destructor: bff94028
  destructor: bff9405f
  destructor: bff9405e
  destructor: bff9405d
  destructor: bff9405c

besides, dmd calls destructor for the first array element four times
(bff94028).

i think that this is definitely compiler bug. if dmd insists on
postbliting, it should at least call destructors for all array
elements, not only for the first one four times. ;-)
Oct 27 2014
prev sibling parent reply "Szymon Gatner" <noemail gmail.com> writes:
On Monday, 27 October 2014 at 07:31:34 UTC, Shachar Shemesh wrote:

 For reasons I won't go into (but should be fairly obvious), I 
 am trying to write code that does not rely on the garbage 
 collector. As such, I'm using reference counting structs 
 allocated on a pool.

 To keep things sane, I'm trying to use RAII semantics, and to 
 that end, a smart pointer that calls "incref" and "decref" when 
 appropriate.

 The SmartPtr struct implements this(T*), this(SmartPtr), 
 this(this), ~this(), opAssign(SmartPtr), opAssign(ref SmartPtr) 
 and opAssign(T*). To the best of my understanding this should 
 be enough to catch all relevant cases.

 All works fine except one case:

 Foo foo;

 SmartPtr!Foo[4] array;

 array[0] = foo;

 assert(foo.refcount == 1);

 array = array.init;

 assert(foo.refcount == 0, "This assert fails");

 I am compiling this on dmd v2.065

 Switching the init line to "array[] = SmartPtr!Foo.init" makes 
 the second assert pass.

 To the best of my understanding, "array = array.init" should 
 always be equivalent to "array[] = typeof(array[0]).init" for 
 static arrays. Am I missing something? Is this a compiler bug?

 Thanks,
 Shachar


You have created dynamic array of SmartPtrs. In that case they 
now live on GS's heap and destructor calls are no longer 
deterministic (and in fact are not guaranteed at all). That is 
the very first things that gets C++ programmers coming to D 
(happened to me too). RAII does not work in D. At least not how 
C++ would expect it to.
Oct 27 2014
parent reply ketmar via Digitalmars-d-learn <digitalmars-d-learn puremagic.com> writes:
On Mon, 27 Oct 2014 09:11:33 +0000
Szymon Gatner via Digitalmars-d-learn
<digitalmars-d-learn puremagic.com> wrote:


 You have created dynamic array of SmartPtrs.


nope. it's stack-allocated array.
Oct 27 2014
parent reply "Szymon Gatner" <noemail gmail.com> writes:
On Monday, 27 October 2014 at 09:21:14 UTC, ketmar via 
Digitalmars-d-learn wrote:

 On Mon, 27 Oct 2014 09:11:33 +0000
 Szymon Gatner via Digitalmars-d-learn
 <digitalmars-d-learn puremagic.com> wrote:


 You have created dynamic array of SmartPtrs.


 nope. it's stack-allocated array.


Right, sorry. Tho I admit I made assumptions since that was not 
the full code.
Oct 27 2014
parent reply Shachar Shemesh <shachar weka.io> writes:
On 27/10/14 11:31, Szymon Gatner wrote:


 Right, sorry. Tho I admit I made assumptions since that was not the full
 code.


I've opened a bug. It has a fully contained sample (that does not, in 
fact, implement smartptr).

https://issues.dlang.org/show_bug.cgi?id=13661
Oct 27 2014
parent reply "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Monday, 27 October 2014 at 12:40:17 UTC, Shachar Shemesh wrote:

 On 27/10/14 11:31, Szymon Gatner wrote:


 Right, sorry. Tho I admit I made assumptions since that was 
 not the full
 code.


 I've opened a bug. It has a fully contained sample (that does 
 not, in fact, implement smartptr).

 https://issues.dlang.org/show_bug.cgi?id=13661


Strictly speaking, your opAssign is wrong, because it doesn't 
swap source and destination, as it should. This doesn't matter 
here, but it would for a smartptr.

However, that opAssign isn't even called is certainly a bug.
Oct 27 2014
parent reply "Szymon Gatner" <noemail gmail.com> writes:
On Monday, 27 October 2014 at 14:04:53 UTC, Marc Schütz wrote:

 On Monday, 27 October 2014 at 12:40:17 UTC, Shachar Shemesh 
 wrote:

 On 27/10/14 11:31, Szymon Gatner wrote:


 Right, sorry. Tho I admit I made assumptions since that was 
 not the full
 code.


 I've opened a bug. It has a fully contained sample (that does 
 not, in fact, implement smartptr).

 https://issues.dlang.org/show_bug.cgi?id=13661


 Strictly speaking, your opAssign is wrong, because it doesn't 
 swap source and destination, as it should. This doesn't matter 
 here, but it would for a smartptr.

 However, that opAssign isn't even called is certainly a bug.


Why would opAssign() swap things?
Oct 27 2014
parent reply "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Monday, 27 October 2014 at 16:58:56 UTC, Szymon Gatner wrote:

 On Monday, 27 October 2014 at 14:04:53 UTC, Marc Schütz wrote:

 On Monday, 27 October 2014 at 12:40:17 UTC, Shachar Shemesh 
 wrote:

 On 27/10/14 11:31, Szymon Gatner wrote:


 Right, sorry. Tho I admit I made assumptions since that was 
 not the full
 code.


 I've opened a bug. It has a fully contained sample (that does 
 not, in fact, implement smartptr).

 https://issues.dlang.org/show_bug.cgi?id=13661


 Strictly speaking, your opAssign is wrong, because it doesn't 
 swap source and destination, as it should. This doesn't matter 
 here, but it would for a smartptr.

 However, that opAssign isn't even called is certainly a bug.


 Why would opAssign() swap things?


This is necessary to make assignment exception safe (in the 
general case). It's known as the copy-and-swap idiom in C++. Here 
it's described in the D spec:

http://dlang.org/struct#assign-overload

For your example code it's not necessary, because there can never 
be exceptions, but depending on what the smart pointer does, it 
might throw in one of its members' opAssign, which could lead to 
a partially copied object.
Oct 27 2014
parent reply "Szymon Gatner" <noemail gmail.com> writes:
On Monday, 27 October 2014 at 18:42:11 UTC, Marc Schütz wrote:

 On Monday, 27 October 2014 at 16:58:56 UTC, Szymon Gatner wrote:

 On Monday, 27 October 2014 at 14:04:53 UTC, Marc Schütz wrote:

 On Monday, 27 October 2014 at 12:40:17 UTC, Shachar Shemesh 
 wrote:

 On 27/10/14 11:31, Szymon Gatner wrote:


 Right, sorry. Tho I admit I made assumptions since that was 
 not the full
 code.


 I've opened a bug. It has a fully contained sample (that 
 does not, in fact, implement smartptr).

 https://issues.dlang.org/show_bug.cgi?id=13661


 Strictly speaking, your opAssign is wrong, because it doesn't 
 swap source and destination, as it should. This doesn't 
 matter here, but it would for a smartptr.

 However, that opAssign isn't even called is certainly a bug.


 Why would opAssign() swap things?


 This is necessary to make assignment exception safe (in the 
 general case). It's known as the copy-and-swap idiom in C++. 
 Here it's described in the D spec:

 http://dlang.org/struct#assign-overload

 For your example code it's not necessary, because there can 
 never be exceptions, but depending on what the smart pointer 
 does, it might throw in one of its members' opAssign, which 
 could lead to a partially copied object.


Copy-and-swap in C++ as the name implies requires 1) copy first - 
a temporary copy-constructed object on created on the stack then 
2) trivial/nothrow swap with that temporary. Your suggestion 
sounded like source and destination of opAssign() were suppose to 
be swapped. In C++ this is sometimes used when move-assigning.
Oct 28 2014
parent "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Tuesday, 28 October 2014 at 08:36:07 UTC, Szymon Gatner wrote:

 On Monday, 27 October 2014 at 18:42:11 UTC, Marc Schütz wrote:

 On Monday, 27 October 2014 at 16:58:56 UTC, Szymon Gatner 
 wrote:

 On Monday, 27 October 2014 at 14:04:53 UTC, Marc Schütz wrote:

 On Monday, 27 October 2014 at 12:40:17 UTC, Shachar Shemesh 
 wrote:

 On 27/10/14 11:31, Szymon Gatner wrote:


 Right, sorry. Tho I admit I made assumptions since that 
 was not the full
 code.


 I've opened a bug. It has a fully contained sample (that 
 does not, in fact, implement smartptr).

 https://issues.dlang.org/show_bug.cgi?id=13661


 Strictly speaking, your opAssign is wrong, because it 
 doesn't swap source and destination, as it should. This 
 doesn't matter here, but it would for a smartptr.

 However, that opAssign isn't even called is certainly a bug.


 Why would opAssign() swap things?


 This is necessary to make assignment exception safe (in the 
 general case). It's known as the copy-and-swap idiom in C++. 
 Here it's described in the D spec:

 http://dlang.org/struct#assign-overload

 For your example code it's not necessary, because there can 
 never be exceptions, but depending on what the smart pointer 
 does, it might throw in one of its members' opAssign, which 
 could lead to a partially copied object.


 Copy-and-swap in C++ as the name implies requires 1) copy first 
 - a temporary copy-constructed object on created on the stack 
 then 2) trivial/nothrow swap with that temporary. Your 
 suggestion sounded like source and destination of opAssign() 
 were suppose to be swapped.


Sorry, this was not intended.


 In C++ this is sometimes used when move-assigning.


D does it automatically (and it always can, because structs need 
to be movable, e.g. contain no pointers to themselves), unless 
you define your own opAssign(), in which case your supposed to 
implement it in a way that has the same semantics.
Oct 28 2014