http://d.puremagic.com/issues/show_bug.cgi?id=5519

           Summary: Saner struct equality
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2011-02-02 15:07:06 PST ---
Performing a comparison between two structs is a very common operation. Often structs contain strings and other things. Currently (DMD 2.051) the struct equality ignores the contents of strings contained inside structs, and this behaviour is unacceptably bug-prone in a language like D that's otherwise oriented toward code safety. So in the following four programs I'd like the assertions to pass.

----------------

struct Foo { string s; }
void main() {
    string s1 = "he";
    string s2 = "llo";
    string s3 = "hel";
    string s4 = "lo";
    auto f1 = Foo(s1 ~ s2);
    auto f2 = Foo(s3 ~ s4);
    assert((s1 ~ s2) == (s3 ~ s4));
    assert(f1 == f2); // this asserts
}

----------------

struct Foo { int[] a; }
void main() {
    auto a1 = [1, 2];
    auto a2 = [3, 4, 5];
    auto a3 = [1, 2, 3];
    auto a4 = [4, 5];
    auto f1 = Foo(a1 ~ a2);
    auto f2 = Foo(a3 ~ a4);
    assert((a1 ~ a2) == (a3 ~ a4));
    assert(f1 == f2); // this asserts
}

----------------

class Bar {
    int x;
    this(int x_) { x = x_; }
    bool opEquals(Object o) {
        return x == (cast(Bar)o).x;
    }
}
struct Foo { Bar a; }
void main() {
    auto f1 = Foo(new Bar(1));
    auto f2 = Foo(new Bar(1));
    assert(f1 == f2); // this asserts
}

----------------

struct Foo { int[int] aa; }
void main() {
    auto f1 = Foo([1:0, 2:0]);
    auto f2 = Foo([1:0, 2:0]);
    assert(f1.aa == f2.aa);
    assert(f1 == f2); // this asserts
}

----------------

The title of this enhancement request is "Saner struct equality" instead of "Sane struct equality" because (it seems) D struct equality isn't meant/expected to be fully correct. This example shows a struct comparison problem this enhancement request doesn't cover:


import core.stdc.string: memset;
struct Foo { long l; byte b; }
void main() {
    Foo f1 = Foo(10, 20);
    Foo f2;
    memset(&f1, 'X', Foo.sizeof);
    f2.l = 10;
    f2.b = 20;
    assert(f1 == f2); // this asserts
}

----------------

Surprisingly this works (DMD 2.051):


struct Bar {
    int x;
    const bool opEquals(ref const(Bar) o) {
        return x == o.x || x == -o.x;
    }
}
struct Foo { Bar a; }
void main() {
    auto f1 = Foo(Bar(1));
    auto f2 = Foo(Bar(-1));
    assert(f1 == f2);  // this doesn't assert
}

----------------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5519


Denis Derman <denis.spir@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |denis.spir@gmail.com


--- Comment #1 from Denis Derman <denis.spir@gmail.com> 2011-02-03 03:34:36 PST ---
(In reply to comment #0)
> Performing a comparison between two structs is a very common operation. Often structs contain strings and other things. Currently (DMD 2.051) the struct equality ignores the contents of strings contained inside structs, [...]
> 
> ----------------
> 
> struct Foo { string s; }
> void main() {
>     string s1 = "he";
>     string s2 = "llo";
>     string s3 = "hel";
>     string s4 = "lo";
>     auto f1 = Foo(s1 ~ s2);
>     auto f2 = Foo(s3 ~ s4);
>     assert((s1 ~ s2) == (s3 ~ s4));
>     assert(f1 == f2); // this asserts
> }
> 
> ----------------

This issue is partially masked by the fact sring /literals/ are interned in D (or is it an implementation optimisation of dmd?). Very annoying in interaction with the present issue: since string interning makes comparison of structs holding string sometimes behave as expected, other cases have an increased risk of being bug-prone. Below another example showing this (all asserts pass). Note that idup does not help & restore correct behaviour observed in case of literals:

struct S {string s;}
unittest {
    // literals
    string s01 = "hello"; string s02 = "hello";
    assert ( S(s01) == S(s02) );
    // concat
    string s1 = "he"; string s2 = "llo";
    string s3 = "hel"; string s4 = "lo";
    assert ( S(s1 ~ s2) != S(s3 ~ s4) );
    // slice
    string s = "hello";
    assert ( S(s[1..$-1]) != S("ell") );
    // idup'ed
    assert ( S(s[1..$-1].idup) != S("ell") );
    s5 = s[1..$-1].idup;
    assert ( S(s5) != S("ell") );
}

Denis

> ----------------
> 
> Surprisingly this works (DMD 2.051):
> 
> 
> struct Bar {
>     int x;
>     const bool opEquals(ref const(Bar) o) {
>         return x == o.x || x == -o.x;
>     }
> }
> struct Foo { Bar a; }
> void main() {
>     auto f1 = Foo(Bar(1));
>     auto f2 = Foo(Bar(-1));
>     assert(f1 == f2);  // this doesn't assert
> }
> 
> ----------------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5519



--- Comment #2 from Denis Derman <denis.spir@gmail.com> 2011-02-03 03:40:07 PST ---
(In reply to comment #0)

> Surprisingly this works (DMD 2.051):
> 
> 
> struct Bar {
>     int x;
>     const bool opEquals(ref const(Bar) o) {
>         return x == o.x || x == -o.x;
>     }
> }
> struct Foo { Bar a; }
> void main() {
>     auto f1 = Foo(Bar(1));
>     auto f2 = Foo(Bar(-1));
>     assert(f1 == f2);  // this doesn't assert
> }

You probably meant in your comment: "this doesn't throw" didn't you? The assertion passes, meaning the code behaves with expected semantics.

Denis

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5519



--- Comment #3 from Denis Derman <denis.spir@gmail.com> 2011-02-03 03:55:37 PST ---
The core issue, I guess, is that '==' implicitely means comparison "by value
equality". This sense is even more obvious in D which has a dedicated operator
'is' for comparison "by ref identity".
Current behaviour comparing structs using by ref for their string & array
members may not be wrong in itself, but it conflicts with expected semantics of
'=='. And the case of (interned) string literals makes it inconsistent.

Denis

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5519



--- Comment #4 from Denis Derman <denis.spir@gmail.com> 2011-02-03 04:30:32 PST ---
One (hopefully last) more point:

A situation where one may constantly need to compare structs for equality (by
value!) is unittests:

unittest {
    ...
    assert (result == S(a, b));
}

This is actually how I stepped on the present issue.

The general workaround is indeed to write custom opEquals methods which just compare member per member. Thus, I ended up adding such opEquals to all structs:

struct Lexeme {
    string tag;
    string slice;
    Ordinal index;          // for parser match error messages
    string toString () {
        return format(`Lexeme("%s","%s",%s)`, tag,slice,index);
    }
    // workaround for bug in default struct '=='
    const bool opEquals (ref const(Lexeme) l) {
        return (
               this.tag == l.tag
            && this.slice == l.slice
            && this.index == l.index
        );
    }
}

Denis

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5519



--- Comment #5 from bearophile_hugs@eml.cc 2011-02-04 12:18:18 PST ---
This issue is essentially a dupe of bug 3789

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5519


Simen Kjaeraas <simen.kjaras@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |simen.kjaras@gmail.com
         Resolution|                            |DUPLICATE


--- Comment #6 from Simen Kjaeraas <simen.kjaras@gmail.com> 2011-02-04 13:52:43 PST ---
*** This issue has been marked as a duplicate of issue 3789 ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------