[Issue 24874] New: Copying, assigning to, and destroying a struct with a union that contains a non-POD struct should be @system

[d-bugmail at puremagic.com]

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

          Issue ID: 24874
           Summary: Copying, assigning to, and destroying a struct with a
                    union that contains a non-POD struct should be @system
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: major
          Priority: P1
         Component: dmd
          Assignee: nobody at puremagic.com
          Reporter: issues.dlang at jmdavisProg.com

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

--