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
