Issue 3789 is an enhancement request, I think it fixes one small but quite important problem in D design. The situation is shown by this simple code:


struct String {
    char[] data;
}
void main () {
    auto foo = String("foo".dup);
    auto bar = String("foo".dup);
    assert(bar !is foo, "structs aren't the same bit-wise");
    assert(bar == foo, "oops structs aren't equal");
}



The D Zen says D is designed to be safe on default and to perform unsafe (and faster) things on request. Not comparing the strings as strings in the following code breaks the Principle of least astonishment, so breaks that rule.

An acceptable alternative to fixing Bug 3789 is statically disallowing the equal operator (==) in such cases (or even in all cases).

D has the "is" for the situations where you want to perform bitwise comparison, for structs too. For the other situations where I use "==" among struts, I want it do the right thing, like comparing its contained strings correctly instead of arbitrarily deciding to use bitwise comparison of the sub-struct that represents the string.

There is already a patch for this, from the extra-good Kenji Hara: https://github.com/D-Programming-Language/dmd/pull/387

Making "==" work as "is" for structs means using an operator for the purpose of the other operator, and it has caused some bugs in my code. And it will cause bugs in D code to come.


Another example, reduced/modified from a real bug in a program of mine:


import std.stdio;
struct Foo {
    int x;
    string s;
}
void main () {
    int[Foo] aa;
    aa[Foo(10, "hello")] = 1;
    string hel = "hel";
    aa[Foo(10, hel ~ "lo")] = 2;
    writeln(aa);
}


Here D defines a hashing for the Foo struct, but it uses the standard "==" to compare the struct keys. So the output is this, that I believe is what almost no one will ever want:

[Foo(10, "hello"):1, Foo(10, "hello"):2]

Bye,
bearophile

El 26/03/2012 13:58, bearophile escribió:
> Issue 3789 is an enhancement request, I think it fixes one small but quite important problem in D design. The situation is shown by this simple code:
>
>
> struct String {
>      char[] data;
> }
> void main () {
>      auto foo = String("foo".dup);
>      auto bar = String("foo".dup);
>      assert(bar !is foo, "structs aren't the same bit-wise");
>      assert(bar == foo, "oops structs aren't equal");
> }
>
>
>
> The D Zen says D is designed to be safe on default and to perform unsafe (and faster) things on request. Not comparing the strings as strings in the following code breaks the Principle of least astonishment, so breaks that rule.
>


Maybe it makes more sense that struct==struct applies == to each of its fields. It would be the same as bitwise comparison for simple primitive types, but would be more useful with other types such as strings.

On Monday, 26 March 2012 at 21:25:06 UTC, Alvaro wrote:
> Maybe it makes more sense that struct==struct applies == to each of its fields. It would be the same as bitwise comparison for simple primitive types, but would be more useful with other types such as strings.

+1

On Mon, 26 Mar 2012 17:25:07 -0400, Alvaro <alvaroDotSegura@gmail.com>
wrote:

> Maybe it makes more sense that struct==struct applies == to each of its fields. It would be the same as bitwise comparison for simple primitive types, but would be more useful with other types such as strings.

That's exactly what bug 3789 advocates for.  Please vote up!

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

-Steve

On 3/27/12, Steven Schveighoffer <schveiguy@yahoo.com> wrote:
> That's exactly what bug 3789 advocates for.  Please vote up!
>
> http://d.puremagic.com/issues/show_bug.cgi?id=3789

Unrelated, but I don't understand bugzilla's stupid 10-vote limitation. As if I can only care about 10 bugs. I agree with 3789, == should do the safe thing by default.

Speaking of which, does anyone have some sort of template to autogenerate an opEquals that compares all fields of a struct via '=='? I could write it (maybe via traits & getAllMembers) but surely someone has done it before.

On 3/29/12, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
> Speaking of which, does anyone have some sort of template to autogenerate an opEquals that compares all fields of a struct via '=='?

Well that took 2 seconds.

import std.range;

template safeOpEquals(T)
{
    bool opEquals(T t)
    {
        foreach (lhsField, rhsField; lockstep(this.tupleof, t.tupleof))
        {
            if (lhsField != rhsField)
                return false;
        }

        return true;
    }
}

struct Foo
{
    int[] arr;
    mixin safeOpEquals!Foo;
}

void main()
{
    Foo a, b;
    a.arr = [1, 2, 3].dup;
    b.arr = [1, 2, 3].dup;
    assert(a == b);
}

D makes things so damn easy.

On 3/29/12, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
> D makes things so damn easy.

Correction, makes things easier. No need to pass the type of 'this':

template safeOpEquals()
{
    bool opEquals(typeof(this) t)
    {
        foreach (lhsField, rhsField; lockstep(this.tupleof, t.tupleof)) {
            if (lhsField != rhsField)
                return false;
        }
        return true;
    }
}
struct Foo
{
    int[] arr;
    mixin safeOpEquals;
}

 3/29/12, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
> snip

Crap, lockstep doesn't work on different types, it was iterating over the arrays. That went over my head. A slightly complicated version that actually works:

import std.stdio;
import std.conv;

string tupleCompare(int len)()
{
    string result;
    foreach (i; 0 .. len)
    {
        result ~= "
            if (this.tupleof[" ~ to!string(i) ~ "] != t.tupleof[" ~
to!string(i) ~ "])
                return false;
        ";
    }
    return result;
}

template safeOpEquals()
{
    bool opEquals(typeof(this) t)
    {
        enum len = typeof(this).tupleof.length;
        mixin(tupleCompare!len);
        return true;
    }
}

On Thu, 29 Mar 2012 07:49:28 -0400, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:

> On 3/27/12, Steven Schveighoffer <schveiguy@yahoo.com> wrote:
>> That's exactly what bug 3789 advocates for.  Please vote up!
>>
>> http://d.puremagic.com/issues/show_bug.cgi?id=3789
>
> Unrelated, but I don't understand bugzilla's stupid 10-vote
> limitation. As if I can only care about 10 bugs. I agree with 3789, ==
> should do the safe thing by default.

The reasoning behind it was that we didn't want everyone voting for every bug.  10 votes may be too restrictive, but some restriction should be in place IMO.  The 10 votes was an arbitrary choice.  When you have unlimited votes, you are not as responsible with them.

I personally have never run out of votes, because every time I go to vote for a bug, I have some voted-for bugs which have been fixed.

The voting system was put in place a few years ago, to see how it might help focus attention on more severe bugs.  I'm not sure it had the dramatic effect we were hoping, but some bugs did get more attention because they had more votes.  It at least gives you a good compelling argument if you want to argue that a high-vote bug should be fixed :)

-Steve

On 03/29/12 14:07, Andrej Mitrovic wrote:
>  3/29/12, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
>> snip
> 
> Crap, lockstep doesn't work on different types, it was iterating over the arrays. That went over my head. A slightly complicated version that actually works:
> 
> import std.stdio;
> import std.conv;
> 
> string tupleCompare(int len)()
> {
>     string result;
>     foreach (i; 0 .. len)
>     {
>         result ~= "
>             if (this.tupleof[" ~ to!string(i) ~ "] != t.tupleof[" ~
> to!string(i) ~ "])
>                 return false;
>         ";
>     }
>     return result;
> }
> 
> template safeOpEquals()
> {
>     bool opEquals(typeof(this) t)
>     {
>         enum len = typeof(this).tupleof.length;
>         mixin(tupleCompare!len);
>         return true;
>     }
> }

// D does make things easy:
template safeOpEquals()
{
   bool opEquals(T)(T t) {
      foreach (i, v; this.tupleof )
         if (v != t.tupleof[i])
            return 0;
      return 1;
   }
}

struct Foo
{
    int[] arr;
    string s;
    mixin safeOpEquals;
}


You may also want to include something like

>   static if (!is(typeof(this)==T))
>      return 0;
>   else
>      foreach etc...

to make it return false, instead of failing at compiletime when comparing different types.

artur