Copying, assigning to, and destroying a struct with a union that contains a non-POD struct should be @system

[dlangBugzillaToGithub]

Jonathan M Davis (@jmdavis) reported this on 2024-11-23T03:57:32Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=24874

Description

This code compiles just fine

---
void main() @safe
{
    Foo foo;
    auto foo2 = foo;
    foo = foo2;
}

struct Foo
{
    union
    {
        Bar b;
        int i;
    }
}

struct Bar
{
    this(this) {}
    void opAssign(Bar b) {}
    ~this() {}
}
---

This is in spite of the fact that the postblit constructor, opAssign, and destructor for b are all completely skipped, because the compiler doesn't generate one for Foo, because it doesn't know which member of the union is currently the valid one. However, skipping _any_ of those should be considered @system, because there's no telling what memory corruption might occur when those functions are skipped. And the situation is just going to get worse when we add move constructors, since _they_ would be skipped to.

Honestly, I'm inclined to argue that the compiler should be generating `@disable`d postblit / copy constructors, opAssigns, destructors - and move constructors once we get those - when _any_ of the member of a union has that corresponding function so that the programmer is forced to correctly handle the situation and define one that does the right thing (whatever that happens to be for their type), and the compiler doesn't just skip that function, since that's basically _never_ the correct behavior, and it could risk memory corruption depending on what those functions are doing (e.g. it could skip reference counting and result in memory being freed).

But even if we don't go that far, it's clearly wrong to treat this behavior as @safe, since it can definitely result in memory corruption. So, at minimum, if a copy / postblit constructor, opAssign, destructor, or move constructor is going to be skipped by a union, doing that operation with the containing struct or union should be considered @system.

A related issue is https://issues.dlang.org/show_bug.cgi?id=19916, and really, this bug stems from that one. Copying, assigning to, moving, or destroying a union involves accessing it, and accessing a member of a union simply isn't @safe except in situations where the types involved are so simple that you cannot possible have any memory safety issues if they get messed up (e.g. a union of an int and a long is probably @safe, because worst case, you get random numbers from them, but if more complex types are involved, then memory safety is simply not in the picture without the programmer going to the effort of making sure that only the correct member is accessed).