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.
