Bug 15828 – DMD should refuse comparing unions

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

Small correction: std.json has already been fixed in upstream phobos, so I can just backport that fix for GDC.

(In reply to Johannes Pfau from comment #1) > Small correction: std.json has already been fixed in upstream phobos, so I > can just backport that fix for GDC. Excellent, less work for us to do. :-)

I agree that implicitly comparing unions should be a compiler error. The spec doesn't mention == for unions at all: https://dlang.org/spec/expression.html#compare_expressions The failure to call opEquals on a class reference in the union caused https://issues.dlang.org/show_bug.cgi?id=18615 Rebindable!A doesn't use class A's opEquals (returns a is b instead). I've submitted a PR for Phobos to fix 18615 by explicitly comparing the class references in Rebindable's union: https://github.com/dlang/phobos/pull/6370 I hope that such removal of implicit union comparision eases the pain of fixing this 15828. :-)

@teodutu created dlang/dmd pull request #12937 "[DSSv3] Fix Issue 15828 - DMD should refuse comparing unions" fixing this issue: - Fix Issue 15828 - DMD should refuse comparing unions Changes: - Add check in `dmd/opover.d` to verify if the compared structs are unions and output an error if the check passes - Verify the error by `tests/fail_compilation/test15828.d` - Add `changelog/union-comparison.dd` to explain these changes https://github.com/dlang/dmd/pull/12937

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19107 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB