On 5/24/18 5:32 AM, Steven Schveighoffer wrote:
> On 5/23/18 7:11 PM, sarn wrote:

>> Here's an example of what I'm talking about:
>> https://github.com/dlang/phobos/blob/master/std/signals.d#L230
>>
>> It's running __dtor manually just to run some code that happens to be in the destructor.  It's obviously not meant to run any other destructors (at least, the documentation doesn't say "Use this mixin in your object and then calling disconnectAll() will destroy everything in your object.").
> 
> This is a bug. That module is not well-used and has not received a lot of attention, if you look at the development history, almost all changes are global style changes.
> 
>>
>> It's a broken design pattern, but existing code is doing it.  (As I said, I reviewed a lot of D packages, and I don't have time to file bug reports or PRs for each one.)
> 
> Understood. I'll file one for you on this one. It's one thing to have some random package using an incorrect pattern, it's another thing to have Phobos doing it.

Wow, so I learned a couple things reading that code.

1. Yes you are right, it's only meant to call the local mixin destructor, not any other destructors (it doesn't even call the destructor of the class it's mixed into). This "valid" use of __dtor has super-huge code smell.
2. I didn't realize that you could mixin extra pieces to a destructor!

I added the bug report here: https://issues.dlang.org/show_bug.cgi?id=18903. I outline exactly how to fix it, if anyone wants an easy PR opportunity.

-Steve

On Thursday, 24 May 2018 at 12:26:00 UTC, Steven Schveighoffer wrote:
> I added the bug report here: https://issues.dlang.org/show_bug.cgi?id=18903. I outline exactly how to fix it, if anyone wants an easy PR opportunity.
>
> -Steve

Thanks for digging into that.

On Wednesday, 23 May 2018 at 23:18:46 UTC, Manu wrote:
> On 23 May 2018 at 15:47, sarn via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>> On Wednesday, 23 May 2018 at 02:13:13 UTC, rikki cattermole wrote:
>>>
>>> I would consider the current state with classes a bug.
>>> So ticket please, it should not require a DIP to change (although Walter
>>> may disagree).
>>
>>
>> Unfortunately, the way __dtor and __xdtor work for classes can't be changed without the risk of breaking code.  (Even if the current behaviour is broken, users might be working around it.)
>
> In what way is the current behaviour *broken*? (as opposed to awkward, confusing, and poorly documented)

If c is a class instance, then c.__xdtor compiles and runs but can only be guaranteed to run the right destructors in special controlled cases where there's most likely a better way.

That std.signals code that Steven filed a bug report for is example of why we can't just fix the behaviour, though.  If we just fixed __dtor/__xdtor, any code that used std.signals would start having ugly bugs at run time.

I think that longer term we'll have to deprecate and remove these functions.

On Wednesday, 23 May 2018 at 19:05:38 UTC, Manu wrote:

> For my money it would be:
> destroy() <- just destroy
> reset() <- destroy and re-init
>
> Or something like that.
>
> Maybe:
> class.destruct(); <- destroy without init?
>
> Yeah that. I'll make a PR!

I had an in-depth discussion with Andrei about this at https://github.com/dlang/druntime/pull/2115

He ultimately gave up on trying to explain it to me and created his own PR here:  https://github.com/dlang/druntime/pull/2126

It's be good to review that discussion, and perhaps weigh in on #2126.

Mike

On Friday, 25 May 2018 at 23:47:33 UTC, sarn wrote:

> That std.signals code that Steven filed a bug report for is example of why we can't just fix the behaviour, though.  If we just fixed __dtor/__xdtor, any code that used std.signals would start having ugly bugs at run time.
>
> I think that longer term we'll have to deprecate and remove these functions.

I'm very much interested in doing something about these functions.  __xdtor is just one.  There are others at https://github.com/dlang/druntime/blob/54ab96e9977e0c6baa7ed9740810058fd4aec6ef/src/object.d#L1212-L1229.  __xtoHash is currently causing problems at https://github.com/dlang/dmd/pull/8222

TypeInfo has become my nemesis.  I've been trying to replace runtime hooks that depend on TypeInfo with templates that can get their information at compile-time, but I'm running into all sorts of problems.  e.g. Did you know array.length can be set in @safe nothrow pure code, but it lowers to runtime functions that are neither @safe, nothrow, nor pure?

Anyway, I'm getting better at modifying the compiler/runtime interface.  If we can come up with a solution to this mess, and I can understand it, I might be able to implement it.

Mike

On Sunday, 27 May 2018 at 09:55:56 UTC, Mike Franklin wrote:
> On Friday, 25 May 2018 at 23:47:33 UTC, sarn wrote:
>
>> [...]
>
> I'm very much interested in doing something about these functions.  __xdtor is just one.  There are others at https://github.com/dlang/druntime/blob/54ab96e9977e0c6baa7ed9740810058fd4aec6ef/src/object.d#L1212-L1229.  __xtoHash is currently causing problems at https://github.com/dlang/dmd/pull/8222
>
> TypeInfo has become my nemesis.  I've been trying to replace runtime hooks that depend on TypeInfo with templates that can get their information at compile-time, but I'm running into all sorts of problems.  e.g. Did you know array.length can be set in @safe nothrow pure code, but it lowers to runtime functions that are neither @safe, nothrow, nor pure?
>
> Anyway, I'm getting better at modifying the compiler/runtime interface.  If we can come up with a solution to this mess, and I can understand it, I might be able to implement it.
>
> Mike

You are replacing runtime typeinfo with compiletime templates. Unless you can guarantee that the type information won't change during runtime, you are going to have a hard time.

Alex

On Sunday, 27 May 2018 at 16:06:21 UTC, 12345swordy wrote:

> You are replacing runtime typeinfo with compiletime templates. Unless you can guarantee that the type information won't change during runtime, you are going to have a hard time.

Read this thread for context https://forum.dlang.org/post/mr7a65$2hc$1@digitalmars.com

See this PR for an example https://github.com/dlang/dmd/pull/7225

And see this talk for a demonstration of the benefits https://www.youtube.com/watch?v=endKC3fDxqs

Mike

On Sunday, 27 May 2018 at 18:55:41 UTC, Mike Franklin wrote:
> On Sunday, 27 May 2018 at 16:06:21 UTC, 12345swordy wrote:
>
>> You are replacing runtime typeinfo with compiletime templates. Unless you can guarantee that the type information won't change during runtime, you are going to have a hard time.
>
> Read this thread for context https://forum.dlang.org/post/mr7a65$2hc$1@digitalmars.com
>
> See this PR for an example https://github.com/dlang/dmd/pull/7225
>
> And see this talk for a demonstration of the benefits https://www.youtube.com/watch?v=endKC3fDxqs
>
> Mike

Can you actually reply to me instead of saying "read/watch this"? I'm not going to watch an hour, just to see how you going to achieve this.

On Sunday, 27 May 2018 at 21:11:42 UTC, 12345swordy wrote:
> On Sunday, 27 May 2018 at 18:55:41 UTC, Mike Franklin wrote:
>> And see this talk for a demonstration of the benefits https://www.youtube.com/watch?v=endKC3fDxqs
>>
>> Mike
>
> Can you actually reply to me instead of saying "read/watch this"? I'm not going to watch an hour, just to see how you going to achieve this.

tl;dw: there's precedent.  Historically typeinfo has been overused in the runtime library (I think it's because a lot of the code/architecture predates today's templates).  For example, if you compared an int[] to an int[], the compiler would generate a function that looked up a comparator for int using RTTI, and would iterate over the arrays calling it on each element.  That video is about replacing code like that with smarter templated code that's not only faster but results in *less* code bloat.

On Sunday, 27 May 2018 at 09:55:56 UTC, Mike Franklin wrote:
> TypeInfo has become my nemesis.  I've been trying to replace runtime hooks that depend on TypeInfo with templates that can get their information at compile-time, but I'm running into all sorts of problems.  e.g. Did you know array.length can be set in @safe nothrow pure code, but it lowers to runtime functions that are neither @safe, nothrow, nor pure?

Ouch.

> Anyway, I'm getting better at modifying the compiler/runtime interface.  If we can come up with a solution to this mess, and I can understand it, I might be able to implement it.

I've been meaning to learn more about how the compiler/runtime interface works myself but still haven't got around it.  I'm probably going to learn a lot by looking at your PRs.

I've been thinking this through a bit, and here's what I've got so far:

At first I obviously wanted an all-round "just works" low-level interface for destroying objects.  But after looking at how people are using __dtor/__xdtor in real code, and looking at the PRs for destroy(), it's obvious that there's a lot of disagreement about what that means.  Let's leave that topic for now.

If we just focus on fixing classes, we can add, say, __vdtor (I really don't care much about the name, BTW).  This needs to be a normal virtual function in the vtable (preferably in a way that's compatible with C++ ABIs) that runs the user-defined destructor code, then recursively calls destructors for members and base classes.

The generation of __vdtor also needs a special case to make attributes work.  Something like "an empty destructor has attributes inferred like templated functions are, restricted by any of its bases".  For example, this needs to work:

extern(C++)
{

class Base
{
    // Virtual
    ~this() @nogc
    {
    }
}

class Derived
{
    // Not marked pure but is still @nogc
    // (NB: explicitly marking @nogc isn't needed in current language)
    override ~this() @nogc
    {
    }
}

class Derived2 : Derived
{
    override ~this() @nogc pure
    {
        // Marked pure, but still recurses to empty destructors in Derived and Base
    }
}

}

Right now __vdtor would need to be implemented with a thunk that wraps around __xdtor (like I implemented here: https://theartofmachinery.com/2018/05/27/cpp_classes_in_betterc.html).  But if __vdtor is implemented, the D runtime code can be simplified to use __vdtor for classes, then hopefully we can deprecate and remove __dtor and __xdtor for classes, and then __vdtor can become the native destructor implementation.

On Sunday, 27 May 2018 at 22:27:52 UTC, sarn wrote:
> I've been thinking this through a bit, and here's what I've got so far:

Here's a tweak that should be implementable without any language changes:

Instead of trying to detect an empty destructor, we use a UDA on the class --- call it BaseObject or something.  A BaseObject-marked class is meant to be something like Andre's ProtoObject, or a custom alternative base.  It must not define a destructor or include members with destructors (could relax this in future but works for now), and must only inherit from other BaseObject-marked classes.

With that, __vdtor could be implemented using a template mixin.  For a BaseObject class, that would generate an empty virtual __vdtor.  For other classes, it would call __xdtor and then (non-virtually) call __vdtor for the superclass as long as it's not a BaseObject class.

Can anyone see something I've missed?  I think it works with the current type system, makes Andre's ProtoObject possible while supporting subclassing with @nogc or whatever, and gives us safe class destructors that could be compatible with C++.