View mode: basic / threaded / horizontal-split · Log in · Help
March 26, 2012
Issue 3789, stucts equality
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
March 26, 2012
Re: Issue 3789, stucts equality
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.
March 27, 2012
Re: Issue 3789, stucts equality
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
March 27, 2012
Re: Issue 3789, stucts equality
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
March 29, 2012
Re: Issue 3789, stucts equality
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.
March 29, 2012
Re: Issue 3789, stucts equality
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.
March 29, 2012
Re: Issue 3789, stucts equality
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;
}
March 29, 2012
Re: Issue 3789, stucts equality
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;
   }
}
March 29, 2012
Re: Issue 3789, stucts equality
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
March 29, 2012
Re: Issue 3789, stucts equality
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
« First   ‹ Prev
1 2
Top | Discussion index | About this forum | D home