Merge pull request #9331 from WalterBright/no-cmpsb

don't use memcmp() for struct equality, do memberwise comparisons per the spec

Author: WalterBright

changelog/no-cmpsb.dd

@@ -0,0 +1,26 @@
+`memcmp()` compares are no longer performed for struct equality tests, memberwise comparisons are done instead, per the spec
+
+The compiler would sometimes generate code for struct equality tests using
+`memcmp()` across the whole struct object. This assumed that:
+
+$(OL
+$(LI alignment holes were filled with 0, which is not always the case)
+$(LI there were no floating point NaN values, which should compare as not equal
+even if the bit patterns match)
+)
+
+The spec requires that the comparison be done in a memberwise manner.
+This brings the implementation in line with this.
+
+$(LINK2 https://dlang.org/spec/expression.html#equality_expressions, Equality Expressions)
+
+This can break existing code that relied on the former erroneous behavior.
+To correct such code, use one of the following:
+
+$(OL
+$(LI define an `opEquals()` operator overload to achieve the desired behavior)
+$(LI use `is` instead of `==`, as `is` will do a bit compare of the struct object.)
+)
+
+This new behavior is enabled with the `-preview=fieldwise` compiler switch. It
+will eventually become the default behavior.

src/dmd/cli.d

@@ -691,6 +691,7 @@ dmd -cov -unittest myprog.d
             "implement https://github.com/dlang/DIPs/blob/master/DIPs/DIP1000.md (Scoped Pointers)"),
         Feature("dip1008", "ehnogc",
             "implement https://github.com/dlang/DIPs/blob/master/DIPs/DIP1008.md (@nogc Throwable)"),
+        Feature("fieldwise", "fieldwise", "use fieldwise comparisons for struct equality"),
         Feature("markdown", "markdown", "enable Markdown replacements in Ddoc"),
         Feature("fixAliasThis", "fixAliasThis",
             "when a symbol is resolved, check alias this scope before going to upper scopes"),

src/dmd/globals.d

@@ -167,6 +167,7 @@ struct Param
     bool ehnogc;            // use @nogc exception handling
     bool dtorFields;        // destruct fields of partially constructed objects
                             // https://issues.dlang.org/show_bug.cgi?id=14246
+    bool fieldwise;         // do struct equality testing field-wise rather than by memcmp()
 
     CppStdRevision cplusplus = CppStdRevision.cpp98;    // version of C++ standard to support
 

src/dmd/globals.h

@@ -137,6 +137,7 @@ struct Param
     bool ehnogc;        // use @nogc exception handling
     bool dtorFields;        // destruct fields of partially constructed objects
                             // https://issues.dlang.org/show_bug.cgi?id=14246
+    bool fieldwise;         // do struct equality testing field-wise rather than by memcmp()
     unsigned cplusplus;     // version of C++ name mangling to support
     bool markdown;          // enable Markdown replacements in Ddoc
     bool vmarkdown;         // list instances of Markdown replacements in Ddoc

src/dmd/opover.d

@@ -1241,7 +1241,7 @@ Expression op_overload(Expression e, Scope* sc)
                     return;
 
                 import dmd.clone : needOpEquals;
-                if (!needOpEquals(sd))
+                if (!global.params.fieldwise && !needOpEquals(sd))
                 {
                     // Use bitwise equality.
                     auto op2 = e.op == TOK.equal ? TOK.identity : TOK.notIdentity;
@@ -1251,6 +1251,7 @@ Expression op_overload(Expression e, Scope* sc)
                 }
 
                 /* Do memberwise equality.
+                 * https://dlang.org/spec/expression.html#equality_expressions
                  * Rewrite:
                  *      e1 == e2
                  * as:

test/compilable/test16284.d

@@ -0,0 +1,27 @@
+/* REQUIRED_ARGS: -preview=fieldwise
+TEST_OUTPUT:
+---
+fail_compilation/test16284.d(24): Error: reinterpretation through overlapped field `s` is not allowed in CTFE
+fail_compilation/test16284.d(27):        called from here: `test()`
+fail_compilation/test16284.d(27):        while evaluating: `static assert(test())`
+---
+*/
+
+// https://issues.dlang.org/show_bug.cgi?id=16284
+
+struct S {}
+
+struct T
+{
+    union {int i; S s;}
+    this(uint dummy) { s = S.init; }
+}
+
+bool test()
+{
+    auto t1 = T(0);
+    auto t2 = T(0);
+    return t1 == t2;
+}
+
+static assert(test());

test/fail_compilation/test16284.d

@@ -1,5 +1,5 @@
 // PERMUTE_ARGS: -inline -O
-// REQUIRED_ARGS: -preview=dip25
+// REQUIRED_ARGS: -preview=dip25 -preview=fieldwise
 
 // Test operator overloading
 
@@ -728,7 +728,7 @@ bool test3789()
         auto ua2 = UnionA([1,2,3]);
         assert(ua1.u.x is ua2.u.x);
         assert(ua1.u.x != ua2.u.x);
-        assert(ua1 == ua2);
+        assert(ua1 != ua2);
         ua1.u.x = 1.0;
         ua2.u.x = 1.0;
         assert(ua1.u.x is ua2.u.x);
@@ -755,7 +755,7 @@ bool test3789()
         ub2.u.a = [1,2,3].dup;
         assert(ub1.u.a !is ub2.u.a);
         assert(ub1.u.a  == ub2.u.a);
-        assert(ub1 != ub2);
+        assert(ub1 == ub2);
         ub2.u.a = ub1.u.a;
         assert(ub1.u.a is ub2.u.a);
         assert(ub1.u.a == ub2.u.a);