www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 5519] New: Saner struct equality

reply d-bugmail puremagic.com writes:
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: -------
Feb 02 2011
next sibling parent d-bugmail puremagic.com writes:
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: -------
Feb 03 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Feb 03 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Feb 03 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Feb 03 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Feb 04 2011
prev sibling parent d-bugmail puremagic.com writes:
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: -------
Feb 04 2011