Bug 24864 – hasElaborateDestructor incorrectly true for structs with anonymous unions

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
druntime
Product
D
Version
D2
Platform
All
OS
All
Creation time
2024-11-18T09:18:39Z
Last change time
2024-11-18T14:35:17Z
Keywords
pull
Assigned to
No Owner
Creator
Jonathan M Davis

Comments

Comment #0 by issues.dlang — 2024-11-18T09:18:39Z
This code --- void main() { import std.traits; static struct Member { ~this() {} } static struct S { union { Member member; int i; } } static assert(!hasElaborateDestructor!S); } --- fails to compile, because the static assertion fails. However, S does not have a destructor. Member does, and member is a member variable of S, but it's within a union, so S itself doesn't get a destructor, and member's destructor is not supposed to be called - and if you do something like --- void main() { static struct Member { ~this() { assert(false); } } static struct S { union { Member member; int i; } } S s; } --- the assertion does not fail, because S does not have a destructor, and therefore, member's destructor is not called. Of course, well-written code would keep track of which union member is the valid one and call destroy on member when appropriate from inside a user-defined destructor on S, but that's irrelevant to the question of whether S as presented here has elaborate destruction. It doesn't, and as such, hasElaborateDestructor shouldn't claim that it does.
Comment #1 by issues.dlang — 2024-11-18T10:00:53Z
hasElaborateAssign used to have this same problem, but the fix for https://issues.dlang.org/show_bug.cgi?id=24835 inadvertently fixed it.
Comment #2 by dlang-bot — 2024-11-18T12:00:16Z
@jmdavis updated dlang/dmd pull request #17075 "Fix bugzilla issue# 24864: hasElaborateDestructor wrong with anonymous unions" fixing this issue: - Fix bugzilla issue 24864: hasElaborateDestructor wrong with anonymous unions hasElaborateDestructor should only be true if the type actually has a destructor which is called when the object leaves scope. Anonymous unions do not give a destructor to their containing type even if one or more of their members has one (the same with copy constructors, postblit constructors, and assignment operators). For that sort of thing to work properly, such functions need to be added manually to the struct such that they call the functions appropriately on whichever member of the union is the valid one. As such, hasElaborateDestructor should not be true based on the member of a union, and there is no need to check the fields of a struct at all, because the ultimate question is whether the struct itself has a destructor. So, the fact that the code has been checking the struct's members is unnecessary in general and wrong in the case of anonymous unions. The change to checking for __xdtor is because __dtor is an explicitly declared destructor, whereas __xdtor is generated by the compiler (either because the struct has a destructor or because it has at least one member variable which does - and which isn't in a union). So, the check for whether the member variables had a __dtor member was probably to try to catch the cases where the struct hadn't declared an explicit destructor but had had one generated because of its member variables. However, simply checking for __xdtor catches that along with explicit destructors, and there's no need to instantiate any additional templates to check the member variables. In addition to fixing this issue with hasElaborateDestructor, I've improved the tests for hasElaborateCopyConstructor and hasElaborateAssign to catch the same issue for them, though they don't currently have the bug. https://github.com/dlang/dmd/pull/17075
Comment #3 by dlang-bot — 2024-11-18T14:35:17Z
dlang/dmd pull request #17075 "Fix bugzilla issue# 24864: hasElaborateDestructor wrong with anonymous unions" was merged into master: - 74e630d98547ca21eb0ce983c27141d641e46908 by Jonathan M Davis: Fix bugzilla issue 24864: hasElaborateDestructor wrong with anonymous unions hasElaborateDestructor should only be true if the type actually has a destructor which is called when the object leaves scope. Anonymous unions do not give a destructor to their containing type even if one or more of their members has one (the same with copy constructors, postblit constructors, and assignment operators). For that sort of thing to work properly, such functions need to be added manually to the struct such that they call the functions appropriately on whichever member of the union is the valid one. As such, hasElaborateDestructor should not be true based on the member of a union, and there is no need to check the fields of a struct at all, because the ultimate question is whether the struct itself has a destructor. So, the fact that the code has been checking the struct's members is unnecessary in general and wrong in the case of anonymous unions. The change to checking for __xdtor is because __dtor is an explicitly declared destructor, whereas __xdtor is generated by the compiler (either because the struct has a destructor or because it has at least one member variable which does - and which isn't in a union). So, the check for whether the member variables had a __dtor member was probably to try to catch the cases where the struct hadn't declared an explicit destructor but had had one generated because of its member variables. However, simply checking for __xdtor catches that along with explicit destructors, and there's no need to instantiate any additional templates to check the member variables. In addition to fixing this issue with hasElaborateDestructor, I've improved the tests for hasElaborateCopyConstructor and hasElaborateAssign to catch the same issue for them, though they don't currently have the bug. https://github.com/dlang/dmd/pull/17075