D's Destructors are What Scott Meyers Warned Us About

[Manu]

On 22 May 2018 at 18:59, sarn via Digitalmars-d
<digitalmars-d at puremagic.com> wrote:
> (I'm referring to Scott's 2014 DConf talk:
> https://www.youtube.com/watch?v=KAWA1DuvCnQ)
>
> I was actually preparing a DIP about this when Manu posted to the forums
> about his own related problems with C++ interop.
>
> I traced a bug in some of my D code to my own misunderstanding of how D's
> destructors actually work.  So I did some research and discovered a bunch of
> edge cases with using __dtor, __xdtor and hasElaborateDestructor.  I tried
> reviewing the packages on code.dlang.org and some elsewhere (thankfully only
> about 1% of D packages use these functions directly) and it turns out I'm
> not the only one confused.  I don't have time to file bug reports for every
> package, so, if you're responsible for code that handles destructors
> manually, please do a review.  There's a *lot* of buggy code out there.
>
> I'm starting this thread to talk about ways to make things better, but first
> the bad news.  Let's use this idiomatic code as an example of typical bugs:
>
> void foo(T)(auto ref T t)
> {
>     ...
>     static if (hasElaborateDestructor!T)
>     {
>         t.__dtor();
>     }
>     ...
> }
>
> Gotcha #1:
> __dtor calls the destructor defined by T, but not the destructor defined by
> any members of T (if T is a struct or class).  I know, some of you might be
> thinking, "Silly sarn, that's what __xdtor is for, of course!"  Turns out
> this isn't that widely known or understood (__dtor is used in examples in
> the spec --- https://dlang.org/spec/struct.html#assign-overload).  A lot of
> code is only __dtor-aware, and there's at least some code out there that
> checks for both __dtor and __xdtor and mistakenly prefers __dtor.  __xdtor
> only solves the specific problem of also destroying members.
>
> Gotcha #2:
> The destructor will never be run for classes because hasElaborateDestructor
> is only ever true for structs.  This is actually per the documentation, but
> it's also not well known "on the ground" (e.g., a lot of code has
> meaningless is(T == class) or is(T == struct) around
> hasElaborateDestructor).  The code example is obviously a leak if t was
> emplace()d in non-GC memory, but even for GCed classes, it's important for
> containers to be explicit about whether or not they own reference types.
>
> Gotcha #3:
> Even if __dtor is run on a class instance, it generally won't run the
> correct destructor because it's not virtual.  (I.e., if T is a base class,
> no destructors for derived classes will be called.)  The spec says that D
> destructors are virtual, but this is emulated using runtime type information
> rather than by using the normal virtual function implementation.  __xdtor is
> the same.
>
> Gotcha #4:
> Even if the right derived class __dtor is run, it won't run the destructors
> for any base classes.  The spec says that destructors automatically recurse
> to base classes, but, again, this is handled manually by walking RTTI, not
> by making the destructor function itself recurse.
>
> Gotcha #5:
> The idiom of checking if something needs destruction before destroying it is
> often implemented incorrectly.  As before, hasElaborateDestructor works for
> structs, but doesn't always work as expected for classes.  hasMember!(T,
> "__dtor") seems to work for classes, but doesn't work for a struct that
> doesn't define a destructor, but requires destruction for its members (a
> case that's easy to miss in testing).
>
> It looks like most D programmers think that D destructors work like they
> typically do in C++, just like I did.
>
> Here are some recommendations:
> * Really try to just use destroy().  Manually working with __dtor/__xdtor is
> a minefield, and I haven't seen any code that actually reimplements the RTTI
> walk that the runtime library does.  (Unfortunately destroy() currently
> isn't zero-overhead for plain old data structs because it's based on RTTI,
> but at least it works.)
> * Avoid trying to figure out if something needs destruction.  Just destroy
> everything, or make it clear you don't own classes at all, or be totally
> sure you're working with plain old data structs.
> * Some code uses __dtor as a way to manually run cleanup code on an object
> that will be used again.  Putting this cleanup code into a normal method
> will cause fewer headaches.
>
> The one other usage of these low-level destructor facilities is checking if
> a type is a plain old data struct.  This is an important special case for
> some code, but everyone seems to do the check a different way.  Maybe a
> special isPod trait is needed.
>

All of these things!
I learned a lot of them over the last few days while trying to
implement destructor linkage and virtual destructors for extern(C++).

If you're in the mood to prepare a DIP, I think you should prepare this dip:

Support the syntax:
  classInstance.~this();

Which **does the right thing**. That is, aggregate of all the gotchas above!
It should recurse the ClassInfo calling the dtors there to perform a
full virtual destruction.
It should also work for structs.
It should also work for extern(C++), which is going to behave yet
differently to the cases you've described (since it needs to match C++
semantics)