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.)

On Wednesday, 23 May 2018 at 19:28:28 UTC, Paul Backus wrote:
> On Wednesday, 23 May 2018 at 01:59:50 UTC, sarn wrote:
>> 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.
>
> There's this:
>
> https://dlang.org/spec/traits.html#isPOD

Damn, that's exactly what I wanted; I don't know how I missed it.

I guess that's why it's good to bounce ideas off others.

On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote:
> On 5/22/18 9:59 PM, sarn wrote:
>> * 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.
>
> Using __dtor is a very bad idea in almost all cases. Putting cleanup code into a normal function can have some of the same pitfalls, however (what if you forget to call the super version of the method?). The only *correct* way to destroy an object is to follow the runtime's example or call the function directly.
>
> The destructor also has the nice feature of being called when the struct goes out of scope.
>
> Best advice -- just use destroy on types to clean them up.

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.").

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.)

> But this is not necessarily the definition of POD. Generally this means it has no postblit, and some people may even be expecting such a thing to have no methods as well. So I'm not sure we want to add such a definition to the library.

The common case is that some data types can be blitted around as raw memory without worrying about destructors, postblits, or whatever is added to the language in future.  This is the thing that seems to matter.  (Have you seen any code that needs to care if a struct has methods?  It sounds like a very special case that can still be handled using current compile-time reflection anyway.)

__traits(isPOD) seems to do the job, and is a lot better than the ad hoc implementations I've seen.  We should encourage people to use it more often.

On Wednesday, 23 May 2018 at 15:43:31 UTC, Steven Schveighoffer wrote:
> Coincidentally, this JUST changed due to a different reason: https://github.com/dlang/druntime/pull/2178
>
>> Please file an enhancement request.
>
> I still think it could be better, so I added a further issue:
> https://issues.dlang.org/show_bug.cgi?id=18899
>
> -Steve

This is really good news!

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)

On Wednesday, 23 May 2018 at 22:47:21 UTC, sarn 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.)

With the recent poll that was taken, D users seem to be alright with fixing problems if it means breaking code. Especially if it is to fix something that is broken to begin with. Not fixing something that's broken because people might have workarounds implemented for it seems kind of backwards to me.

On 23 May 2018 at 16:34, Rubn via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> On Wednesday, 23 May 2018 at 22:47:21 UTC, sarn 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.)
>
>
> With the recent poll that was taken, D users seem to be alright with fixing problems if it means breaking code. Especially if it is to fix something that is broken to begin with. Not fixing something that's broken because people might have workarounds implemented for it seems kind of backwards to me.

I'm actively working on this stuff at the moment... if you guys can articulate what is *broken*, then I will look at it.

On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote:

> Hm.. that should be fixed. I don't see why we can't just do = T.init, we should at least be optimizing to this for small enough structs.

The problem I ran into with this is that if you use T.init directly you end up making copies (postblit) and then needing to destruct those copies since they are temporary.  That's why I had to lower it to simpler byte array primitive.

Mike

On 5/23/18 7:11 PM, sarn wrote:
> On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote:
>> On 5/22/18 9:59 PM, sarn wrote:
>>> * 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.
>>
>> Using __dtor is a very bad idea in almost all cases. Putting cleanup code into a normal function can have some of the same pitfalls, however (what if you forget to call the super version of the method?). The only *correct* way to destroy an object is to follow the runtime's example or call the function directly.
>>
>> The destructor also has the nice feature of being called when the struct goes out of scope.
>>
>> Best advice -- just use destroy on types to clean them up.
> 
> 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.

>> But this is not necessarily the definition of POD. Generally this means it has no postblit, and some people may even be expecting such a thing to have no methods as well. So I'm not sure we want to add such a definition to the library.
> 
> The common case is that some data types can be blitted around as raw memory without worrying about destructors, postblits, or whatever is added to the language in future.  This is the thing that seems to matter.  (Have you seen any code that needs to care if a struct has methods?  It sounds like a very special case that can still be handled using current compile-time reflection anyway.)


> __traits(isPOD) seems to do the job, and is a lot better than the ad hoc implementations I've seen.  We should encourage people to use it more often.

From the D spec, POD means a "struct that contains no hidden members, does not have virtual functions, does not inherit, has no destructor, and can be initialized and copied via simple bit copies"

https://dlang.org/glossary.html#pod

Which is what __traits(isPOD) requires.

This seems like what everyone should use when looking for POD, I wasn't aware we had a __traits for it.

-Steve

On 5/23/18 9:38 PM, Mike Franklin wrote:
> On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote:
> 
>> Hm.. that should be fixed. I don't see why we can't just do = T.init, we should at least be optimizing to this for small enough structs.
> 
> The problem I ran into with this is that if you use T.init directly you end up making copies (postblit) and then needing to destruct those copies since they are temporary.  That's why I had to lower it to simpler byte array primitive.

This is the fool-proof way, but this is a template function! We can deduce whether doing the T.init copy is going to call a postblit or destructor.

-Steve