digitalmars.D - D's Destructors are What Scott Meyers Warned Us About
- sarn (87/87) May 22 2018 (I'm referring to Scott's 2014 DConf talk:
- rikki cattermole (4/98) May 22 2018 I would consider the current state with classes a bug.
- sarn (4/7) May 23 2018 Unfortunately, the way __dtor and __xdtor work for classes can't
- Manu (4/12) May 23 2018 In what way is the current behaviour *broken*? (as opposed to awkward,
- sarn (13/34) May 25 2018 Thanks for digging into that.
- Mike Franklin (14/20) May 27 2018 I'm very much interested in doing something about these
- 12345swordy (5/20) May 27 2018 You are replacing runtime typeinfo with compiletime templates.
- Mike Franklin (7/10) May 27 2018 Read this thread for context
- 12345swordy (4/15) May 27 2018 Can you actually reply to me instead of saying "read/watch this"?
- sarn (9/17) May 27 2018 tl;dw: there's precedent. Historically typeinfo has been
- sarn (53/62) May 27 2018 I've been meaning to learn more about how the compiler/runtime
- sarn (19/21) May 27 2018 Here's a tweak that should be implementable without any language
- 12345swordy (6/27) May 28 2018 How is __vdtor is going to be called, via destroy or via
- sarn (24/30) May 28 2018 Code using destroy() can still use destroy(). Otherwise, __vdtor
- 12345swordy (7/13) May 28 2018 Interesting... You don't mind me asking your assistance on
- sarn (9/15) May 30 2018 I want to write a draft of a DIP related to attribute inference
- Rubn (6/15) May 23 2018 With the recent poll that was taken, D users seem to be alright
- Manu (4/21) May 23 2018 I'm actively working on this stuff at the moment... if you guys can
- Manu (15/92) May 22 2018 All of these things!
- 12345swordy (6/9) May 22 2018 Why? That seems restrictive as there is possibility that the
- Manu (3/10) May 22 2018 ... what?
- 12345swordy (7/20) May 23 2018 Why? That seems restrictive as there is possibility that the
- Steven Schveighoffer (3/7) May 23 2018 Isn't this just classInstance.destroy() ?
- Manu (9/17) May 23 2018 Is everything in object.d globally available? You don't have to import
- Steven Schveighoffer (3/24) May 23 2018 I think it's a good idea. Like unsafeDestroy.
- Manu (13/41) May 23 2018 I hate the name.
- Mike Franklin (8/15) May 27 2018 I had an in-depth discussion with Andrei about this at
- Steven Schveighoffer (3/5) May 23 2018 Oh, didn't answer this, yes object.d is always imported.
- Guillaume Piolat (3/6) May 23 2018 Keep up the good work of explaining surprising things!
- Steven Schveighoffer (39/74) May 23 2018 This is advice you need to follow. Using the underlying __functions are
- Steven Schveighoffer (6/13) May 23 2018 Coincidentally, this JUST changed due to a different reason:
- 12345swordy (8/23) May 23 2018 The destroy function for class/structs badly need an overhaul.
- Jonathan M Davis (5/32) May 23 2018 Regardless of what issues destroy may currently have, the entire reason ...
- 12345swordy (5/34) May 23 2018 Here is the thing though, when you call destroy on the object,
- Jonathan M Davis (8/45) May 23 2018 The documentation is quite clear that it does not free the memory, and i...
- 12345swordy (6/36) May 23 2018 My point being is that current objects rely on the external
- sarn (3/9) May 23 2018 This is really good news!
- sarn (22/38) May 23 2018 Here's an example of what I'm talking about:
- Steven Schveighoffer (15/57) May 24 2018 This is a bug. That module is not well-used and has not received a lot
- Steven Schveighoffer (11/33) May 24 2018 Wow, so I learned a couple things reading that code.
- Mike Franklin (7/10) May 23 2018 The problem I ran into with this is that if you use T.init
- Steven Schveighoffer (5/14) May 24 2018 This is the fool-proof way, but this is a template function! We can
- Paul Backus (3/8) May 23 2018 There's this:
- sarn (3/11) May 23 2018 Damn, that's exactly what I wanted; I don't know how I missed it.
- Mike Franklin (4/21) Jul 08 2018 In the context of your blog post at
- sarn (5/8) Jul 08 2018 I don't know yet, but, yeah, that work looks interesting and I
- 12345swordy (5/14) Jul 08 2018 I was thinking of rewriting my current DIP into introducing null
(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(); } ... } __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. 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. 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. 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. 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.
May 22 2018
On 23/05/2018 1:59 PM, sarn 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(); } ... } __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. 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. 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. 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. 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.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).
May 22 2018
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.)
May 23 2018
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:In what way is the current behaviour *broken*? (as opposed to awkward, confusing, and poorly documented)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.)
May 23 2018
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. -SteveThanks 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: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 02:13:13 UTC, rikki cattermole wrote:In what way is the current behaviour *broken*? (as opposed to awkward, confusing, and poorly documented)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.)
May 25 2018
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/o ject.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
May 27 2018
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: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[...]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/o ject.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
May 27 2018
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
May 27 2018
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: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.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
May 27 2018
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: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.And see this talk for a demonstration of the benefits https://www.youtube.com/watch?v=endKC3fDxqs MikeCan 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.
May 27 2018
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.
May 27 2018
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++.
May 27 2018
On Monday, 28 May 2018 at 04:26:02 UTC, sarn wrote:On Sunday, 27 May 2018 at 22:27:52 UTC, sarn wrote:How is __vdtor is going to be called, via destroy or via directly? The issue that I see that your going to create a "BaseObject" for every attribute or combination of said attributes. Which creates way too much code bloat. Even more so with the possibility of adding more attributes in the future.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++.
May 28 2018
On Monday, 28 May 2018 at 20:13:47 UTC, 12345swordy wrote:How is __vdtor is going to be called, via destroy or via directly?Code using destroy() can still use destroy(). Otherwise, __vdtor would be callable in the same way that __dtor and __xdtor are. The plan is to have a better, safer, less problematic (with attributes) solution to what __dtor and __xdtor do with classes, so that longer term we can deprecate and remove the old way.The issue that I see that your going to create a "BaseObject" for every attribute or combination of said attributes. Which creates way too much code bloat. Even more so with the possibility of adding more attributes in the future.So, I won't say the problem doesn't exist, but it's nowhere near as bad as that. The idea isn't to have a BaseObject for every possible combination of attributes. The BaseObject UDA is just a solution to the problem destructors have with inheritance+attributes. Once you have a base class marked BaseObject, you can derive from it and add whatever combination of attributes you like. The BaseObject-marked class can have as much functionality as needed, except it can't have a non-trivial destructor. That's okay for the immediate problems of implementing ProtoObject and __vdtor, and making safe containers, and improving C++ interop and -betterC code. If needed in future, destructor support could be added in the existing language by exploiting attribute inference, but maybe by that time it would be better to make language changes and turn BaseObject into a no-op. It doesn't matter. I just think it's better to have a viable migration plan that doesn't start with "let's make all these language changes first".
May 28 2018
On Monday, 28 May 2018 at 22:37:01 UTC, sarn wrote:On Monday, 28 May 2018 at 20:13:47 UTC, 12345swordy wrote:Interesting... You don't mind me asking your assistance on writing DIP on this? I have one set up already, and I needed help as 1.) This is my first time writing a DIP 2.) I don't know what main course of action to take regarding this issue.[...]Code using destroy() can still use destroy(). Otherwise, __vdtor would be callable in the same way that __dtor and __xdtor are. [...]
May 28 2018
On Monday, 28 May 2018 at 22:53:03 UTC, 12345swordy wrote:Interesting... You don't mind me asking your assistance on writing DIP on this? I have one set up already, and I needed help as 1.) This is my first time writing a DIP 2.) I don't know what main course of action to take regarding this issue.I want to write a draft of a DIP related to attribute inference next chance I get (possibly this weekend). It's related to this, so I'll post the draft here. You're very welcome to comment (or send PRs). BTW, I'll just be following these guidelines and using accepted DIPs as examples: https://github.com/dlang/DIPs/blob/master/GUIDELINES.md https://github.com/dlang/DIPs/tree/master/DIPs/accepted
May 30 2018
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: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 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.)
May 23 2018
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: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 02:13:13 UTC, rikki cattermole wrote: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 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.)
May 23 2018
On 22 May 2018 at 18:59, sarn via Digitalmars-d <digitalmars-d 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(); } ... } __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. 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. 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. 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. 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)
May 22 2018
On Wednesday, 23 May 2018 at 02:15:25 UTC, Manu wrote:It should recurse the ClassInfo calling the dtors there to perform a full virtual destruction.Why? That seems restrictive as there is possibility that the parent class have an empty destruction with no attributes which your suggestion to be attribute unfriendly.(Unless the attributes are determine at compile time) Alexander
May 22 2018
On 22 May 2018 at 19:39, 12345swordy via Digitalmars-d <digitalmars-d puremagic.com> wrote:On Wednesday, 23 May 2018 at 02:15:25 UTC, Manu wrote:... what?It should recurse the ClassInfo calling the dtors there to perform a full virtual destruction.Why? That seems restrictive as there is possibility that the parent class have an empty destruction with no attributes which your suggestion to be attribute unfriendly.(Unless the attributes are determine at compile time)
May 22 2018
On Wednesday, 23 May 2018 at 03:45:39 UTC, Manu wrote:On 22 May 2018 at 19:39, 12345swordy via Digitalmars-d <digitalmars-d puremagic.com> wrote:Why? That seems restrictive as there is possibility that the parent class have an empty destruction with no attributes which causes your classInstance.~this() suggestion to be attribute unfriendly.(Unless the parent class attributes are determine at compile time) *There I fixed it! Really wish this is a proper web forum.On Wednesday, 23 May 2018 at 02:15:25 UTC, Manu wrote:... what?It should recurse the ClassInfo calling the dtors there to perform a full virtual destruction.Why? That seems restrictive as there is possibility that the parent class have an empty destruction with no attributes which your suggestion to be attribute unfriendly.(Unless the attributes are determine at compile time)
May 23 2018
On 5/22/18 10:15 PM, Manu wrote:If you're in the mood to prepare a DIP, I think you should prepare this dip: Support the syntax: classInstance.~this();Isn't this just classInstance.destroy() ? -Steve
May 23 2018
On 23 May 2018 at 05:38, Steven Schveighoffer via Digitalmars-d <digitalmars-d puremagic.com> wrote:On 5/22/18 10:15 PM, Manu wrote:Is everything in object.d globally available? You don't have to import `destroy()` right? My nit-pick is that destroy resets to .init after destruction, which makes it feel like more than a destroy, and that is almost certainly the reason people reach for __xdtor() when they do (ie, they JUST want to destruct). We should probably address that, and then publish a strong recommendation.If you're in the mood to prepare a DIP, I think you should prepare this dip: Support the syntax: classInstance.~this();Isn't this just classInstance.destroy() ?
May 23 2018
On 5/23/18 1:41 PM, Manu wrote:On 23 May 2018 at 05:38, Steven Schveighoffer via Digitalmars-d <digitalmars-d puremagic.com> wrote:I think it's a good idea. Like unsafeDestroy. -SteveOn 5/22/18 10:15 PM, Manu wrote:Is everything in object.d globally available? You don't have to import `destroy()` right? My nit-pick is that destroy resets to .init after destruction, which makes it feel like more than a destroy, and that is almost certainly the reason people reach for __xdtor() when they do (ie, they JUST want to destruct). We should probably address that, and then publish a strong recommendation.If you're in the mood to prepare a DIP, I think you should prepare this dip: Support the syntax: classInstance.~this();Isn't this just classInstance.destroy() ?
May 23 2018
On 23 May 2018 at 10:59, Steven Schveighoffer via Digitalmars-d <digitalmars-d puremagic.com> wrote:On 5/23/18 1:41 PM, Manu wrote:I hate the name. People will constantly reach for the wrong thing. Such a long name says to me "this isn't the function you're looking for" in terms of intuition. 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!On 23 May 2018 at 05:38, Steven Schveighoffer via Digitalmars-d <digitalmars-d puremagic.com> wrote:I think it's a good idea. Like unsafeDestroy.On 5/22/18 10:15 PM, Manu wrote:Is everything in object.d globally available? You don't have to import `destroy()` right? My nit-pick is that destroy resets to .init after destruction, which makes it feel like more than a destroy, and that is almost certainly the reason people reach for __xdtor() when they do (ie, they JUST want to destruct). We should probably address that, and then publish a strong recommendation.If you're in the mood to prepare a DIP, I think you should prepare this dip: Support the syntax: classInstance.~this();Isn't this just classInstance.destroy() ?
May 23 2018
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 Mike
May 27 2018
On 5/23/18 1:41 PM, Manu wrote:Is everything in object.d globally available? You don't have to import `destroy()` right?Oh, didn't answer this, yes object.d is always imported. -Steve
May 23 2018
On Wednesday, 23 May 2018 at 01:59:50 UTC, sarn wrote: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:Keep up the good work of explaining surprising things! Someone has to do Scott's job for D.
May 23 2018
On 5/22/18 9:59 PM, sarn 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 think it's very good to bring attention to these pitfalls, thanks!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.This is advice you need to follow. Using the underlying __functions are error prone, and subject to weird errors as you have found. The other thing to do is to follow the example of what Phobos functions do when dealing with these low-level functions. In terms of classes, the RTTI walk is callable via the function _rt_finalize (an extern(C) function, you can prototype it anywhere). I highly recommend just calling destroy as it will do this.(Unfortunately destroy() currently isn't zero-overhead for plain old data structs because it's based on RTTI, but at least it works.)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. Please file an enhancement request.* 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.Ideally, destroy should do the most efficient thing. So this is good advice as well.* 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.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.isPod as you have described it is not difficult: enum isPod(T) = is(T == struct) && !hasElaborateDestructor!T; 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. Clearly destroy is preferred to checking or doing anything else, but maybe in some cases we want a more efficient destruction that cuts corners. For instance, there's no reason to blit the init value back to the object when its memory is being reclaimed. That building block is somewhat inextricable from destroy at the moment. Ideally, you shouldn't have to worry about it as a normal user -- some genius library author should take care of these details. Unfortunately, the only place where this happens is on the stack or in the GC. All other memory allocation schemes require you to get your hands dirty with destruction. -Steve
May 23 2018
On 5/23/18 9:12 AM, Steven Schveighoffer wrote:On 5/22/18 9:59 PM, sarn wrote:Coincidentally, this JUST changed due to a different reason: https://github.com/dlang/druntime/pull/2178(Unfortunately destroy() currently isn't zero-overhead for plain old data structs because it's based on RTTI, but at least it works.)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.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
May 23 2018
On Wednesday, 23 May 2018 at 15:43:31 UTC, Steven Schveighoffer wrote:On 5/23/18 9:12 AM, Steven Schveighoffer wrote:The destroy function for class/structs badly need an overhaul. Even more so for class, as (IMO) it should be responsibility of the class designer when it comes to deinitializing (The exception being that creating the class exclusively for the GC to use of course), as finalize function which destroy calls is external, in which information such as attributes is lostOn 5/22/18 9:59 PM, sarn wrote:Coincidentally, this JUST changed due to a different reason: https://github.com/dlang/druntime/pull/2178(Unfortunately destroy() currently isn't zero-overhead for plain old data structs because it's based on RTTI, but at least it works.)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.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
May 23 2018
On Wednesday, May 23, 2018 17:29:11 12345swordy via Digitalmars-d wrote:On Wednesday, 23 May 2018 at 15:43:31 UTC, Steven Schveighoffer wrote:Regardless of what issues destroy may currently have, the entire reason that it exists is to destroy objects - and class objects in particular. So, if it doesn't currently work properly for that purpose, it needs to be fixed. - Jonathan M DavisOn 5/23/18 9:12 AM, Steven Schveighoffer wrote:The destroy function for class/structs badly need an overhaul. Even more so for class, as (IMO) it should be responsibility of the class designer when it comes to deinitializing (The exception being that creating the class exclusively for the GC to use of course), as finalize function which destroy calls is external, in which information such as attributes is lostOn 5/22/18 9:59 PM, sarn wrote:Coincidentally, this JUST changed due to a different reason: https://github.com/dlang/druntime/pull/2178(Unfortunately destroy() currently isn't zero-overhead for plain old data structs because it's based on RTTI, but at least it works.)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.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
May 23 2018
On Wednesday, 23 May 2018 at 17:47:12 UTC, Jonathan M Davis wrote:On Wednesday, May 23, 2018 17:29:11 12345swordy via Digitalmars-d wrote:Here is the thing though, when you call destroy on the object, you would assume by the name that the memory occupied by it will be deallocated which is NOT always the case. At this point I rather rename it as .deint, as it is less misleading here.On Wednesday, 23 May 2018 at 15:43:31 UTC, Steven Schveighoffer wrote:Regardless of what issues destroy may currently have, the entire reason that it exists is to destroy objects - and class objects in particular. So, if it doesn't currently work properly for that purpose, it needs to be fixed. - Jonathan M DavisOn 5/23/18 9:12 AM, Steven Schveighoffer wrote:The destroy function for class/structs badly need an overhaul. Even more so for class, as (IMO) it should be responsibility of the class designer when it comes to deinitializing (The exception being that creating the class exclusively for the GC to use of course), as finalize function which destroy calls is external, in which information such as attributes is lost[...]Coincidentally, this JUST changed due to a different reason: https://github.com/dlang/druntime/pull/2178[...]I still think it could be better, so I added a further issue: https://issues.dlang.org/show_bug.cgi?id=18899 -Steve
May 23 2018
On Wednesday, May 23, 2018 18:04:28 12345swordy via Digitalmars-d wrote:On Wednesday, 23 May 2018 at 17:47:12 UTC, Jonathan M Davis wrote:The documentation is quite clear that it does not free the memory, and it's never been the case that it was supposed to. And we already renamed it once from clear because of the problems that it caused with containers. At this point, I think that it would be overkill to rename it yet again, especially since we've been trying to not rename stuff unless we really need to, because it causes code breakage for what it usually minimal benefit. - Jonathan M DavisOn Wednesday, May 23, 2018 17:29:11 12345swordy via Digitalmars-d wrote:Here is the thing though, when you call destroy on the object, you would assume by the name that the memory occupied by it will be deallocated which is NOT always the case. At this point I rather rename it as .deint, as it is less misleading here.On Wednesday, 23 May 2018 at 15:43:31 UTC, Steven Schveighoffer wrote:Regardless of what issues destroy may currently have, the entire reason that it exists is to destroy objects - and class objects in particular. So, if it doesn't currently work properly for that purpose, it needs to be fixed. - Jonathan M DavisOn 5/23/18 9:12 AM, Steven Schveighoffer wrote:The destroy function for class/structs badly need an overhaul. Even more so for class, as (IMO) it should be responsibility of the class designer when it comes to deinitializing (The exception being that creating the class exclusively for the GC to use of course), as finalize function which destroy calls is external, in which information such as attributes is lost[...]Coincidentally, this JUST changed due to a different reason: https://github.com/dlang/druntime/pull/2178[...]I still think it could be better, so I added a further issue: https://issues.dlang.org/show_bug.cgi?id=18899 -Steve
May 23 2018
On Wednesday, 23 May 2018 at 18:11:45 UTC, Jonathan M Davis wrote:On Wednesday, May 23, 2018 18:04:28 12345swordy via Digitalmars-d wrote:My point being is that current objects rely on the external finalize function for de-inialtalzation, which is not ideal if you wanted to call destroy in the scope of system default attributes like safe or nogc. How do you think that the finalize function should behave in these contexts?On Wednesday, 23 May 2018 at 17:47:12 UTC, Jonathan M Davis wrote:The documentation is quite clear that it does not free the memory, and it's never been the case that it was supposed to. And we already renamed it once from clear because of the problems that it caused with containers. At this point, I think that it would be overkill to rename it yet again, especially since we've been trying to not rename stuff unless we really need to, because it causes code breakage for what it usually minimal benefit. - Jonathan M DavisOn Wednesday, May 23, 2018 17:29:11 12345swordy via Digitalmars-d wrote:Here is the thing though, when you call destroy on the object, you would assume by the name that the memory occupied by it will be deallocated which is NOT always the case. At this point I rather rename it as .deint, as it is less misleading here.[...]Regardless of what issues destroy may currently have, the entire reason that it exists is to destroy objects - and class objects in particular. So, if it doesn't currently work properly for that purpose, it needs to be fixed. - Jonathan M Davis
May 23 2018
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/2178This is really good news!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
May 23 2018
On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote:On 5/22/18 9:59 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."). 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.)* 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.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.
May 23 2018
On 5/23/18 7:11 PM, sarn wrote:On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote: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.On 5/22/18 9:59 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.").* 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.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
May 24 2018
On 5/24/18 5:32 AM, Steven Schveighoffer wrote:On 5/23/18 7:11 PM, sarn wrote: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. -SteveHere'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.
May 24 2018
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
May 23 2018
On 5/23/18 9:38 PM, Mike Franklin wrote:On Wednesday, 23 May 2018 at 13:12:57 UTC, Steven Schveighoffer wrote: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. -SteveHm.. 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.
May 24 2018
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
May 23 2018
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: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.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
May 23 2018
On Wednesday, 23 May 2018 at 01:59:50 UTC, sarn 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:In the context of your blog post at https://theartofmachinery.com/2018/05/27/cpp_classes_in_betterc.html, I'm wondering if the changes in 2.081 help matters at all: I'm wondering if any of the changes in 2.081 improves the situation here: e.g. https://dlang.org/changelog/2.081.0.html#cpp_destroy Mike
Jul 08 2018
On Monday, 9 July 2018 at 01:19:28 UTC, Mike Franklin wrote:In the context of your blog post at https://theartofmachinery.com/2018/05/27/cpp_classes_in_betterc.html, I'm wondering if the changes in 2.081 help matters at all: I'm wondering if any of the changes in 2.081 improves the situation here: e.g. https://dlang.org/changelog/2.081.0.html#cpp_destroy MikeI don't know yet, but, yeah, that work looks interesting and I definitely have to do some more testing. BTW, I'm still interested in writing DIPs. I've just been working a lot on another project lately.
Jul 08 2018
On Monday, 9 July 2018 at 02:10:27 UTC, sarn wrote:On Monday, 9 July 2018 at 01:19:28 UTC, Mike Franklin wrote:I was thinking of rewriting my current DIP into introducing null function and recursive_destruct that can call on classes only if certain rules are checked. -AlexanderIn the context of your blog post at https://theartofmachinery.com/2018/05/27/cpp_classes_in_betterc.html, I'm wondering if the changes in 2.081 help matters at all: I'm wondering if any of the changes in 2.081 improves the situation here: e.g. https://dlang.org/changelog/2.081.0.html#cpp_destroy MikeI don't know yet, but, yeah, that work looks interesting and I definitely have to do some more testing. BTW, I'm still interested in writing DIPs. I've just been working a lot on another project lately.
Jul 08 2018