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

          Issue ID: 15828
           Summary: DMD should refuse comparing unions
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: major
          Priority: P1
         Component: dmd
          Assignee: nob...@puremagic.com
          Reporter: johannesp...@gmail.com

Original GDC bug for reference:
http://bugzilla.gdcproject.org/show_bug.cgi?id=216

Consider this example:
------------------------------------------------------
union U
{
    T a;
    T2 b;
}
U u1, u2; assert (u1 == u2);
------------------------------------------------------

DMD currently happily compares these unions by a memcmp. This causes multiple
problems:

First of all, there are cases where this can not work: if T is a type with a
opEquals, DMD does currently not call the opEquals. In fact, it can't call the
opEquals: As the compiler can not know which union member is valid, there is no
way the compiler can call opEquals for the correct member.


If T and T2 are types of different sizes, T.sizeof > T2.sizeof, code like this
will fail:
------------------------------------------------------
U u1, u2;
u1.a = someValue;
u2.a = someOtherValue;
u1.b = 0;
u2.b = 0;
assert(u1 == u2);
------------------------------------------------------


For GDC, things are even worse. The GCC backend is sometimes 'clever' and
doesn't copy alignment holes padding when assigning structs. Because of this,
GDC does not always memcmp structs and uses a more complicated check instead.
But if these structs are members in a union, we can't know which member is
active and can't do the advanced comparison (e.g. field-by-field comparison).


I think the compiler should simply refuse to compile such code and ask the user
to implement an opEquals function. In theory for DMD it's enough to check if
any union members require non-trivial comparison, but considering the GDC
problem I'd prefer to simply refuse comparison of all unions without opEquals.


This actually affects std.json BTW: Comparison of JSONValues is actually broken
right now...

--

Reply via email to