What is going on here?

[w0rp via Digitalmars-d]

On Sunday, 8 March 2015 at 07:02:48 UTC, Shachar Shemesh wrote:
> On 07/03/15 23:44, w0rp wrote:
>> Why not handle the scope(failure) cases in the constructor 
>> itself?
>>
> Because this breaks encapsulation.
>
> struct SomeStruct {
> 	SomeOtherStruct a;
> 	MoreStruct b;
>
> 	this(int c) {
> 		this.a = SomeOtherStruct(c);
> 		this.b = MoreStruct(c+1);
> 	}
> }
>
> This code, as written, reasonable though it may seem, does not 
> work with D's current implementation. If MoreStruct's 
> constructor throws, a will be left with dangling resources. Or 
> not. It really depends on how SomeOtherStruct is implemented.
>
> Which is exactly my point. For things to be kept sane, each 
> struct must be able to completely control its own resources. 
> Once we accept that, however, a simple look at your example 
> shows there is a much cleaner way of doing it:
>
>> struct SomeStruct {
>>     SmartPtr ptr;
>>     SmartPtr ptr2;
>>
>>     this() {
>>         ptr = malloc(...);
>>         // No longer necessary: scope(failure) free(ptr);
>>
>>         /* Whatever follows, possibly throwing. */
>>
>>         ptr2 = malloc(...);
>>         // No longer necessary: scope(failure) free(ptr2);
>>     }
>>
>>     // No longer necessary: there is nothing left for us to 
>> destruct: ~this()
>> }
>
> This also avoids the bugs your implementation has, where you 
> failed to override this(this) and opAssign, and thus you both 
> are leaking some memory AND double freeing another. If SmartPtr 
> is implemented correctly, the above, as is, is bug free.
>
> Which is precisely why RAII are so great. They reduce the 
> amount of code you need to write in order to be bug-free. You 
> contain the tough corner cases in the RAII implementation, 
> leaving everyone else to just write their code in peace.
>
> Which is why I think problems such as this are a real setback.
>
> Shachar

I only wrote enough to show the really important part. You 
probably want a resource to not be copyable at all, only 
moveable, so I'd probably write @disable this(this) and probably 
also @disable this(). As for a struct with a throwing desctructor 
creating other structs with throwing destructors, my two rules 
apply exactly as before. If the struct's constructor throws, 
release the allocated resources manually with scope(failure) 
cases. Sure encapsulation will be broken, but at the moment you 
can do no better.

The only way we can improve on this is to have the destructors of 
each field called, but not the destructor itself, exactly as C++ 
does it. One complication, as others have mentioned, is that 
construction of each field cannot be stated explictly away from 
the actual constructor, so that might cause some issues.

I'll note that following the rules with non-copyable objects, you 
won't ever double-free any resources. If a constructor of some 
object in the constructor throws, its destructor won't be called 
ever, as you won't hit the following explicit scope(failure) 
case. You won't have a dangling resource, because the previous 
objects with scope(failure) cases will be destroyed, but again 
not double-freed because the struct's destructor won't be run.

So if you want something to work as of right now, do that.