www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - What's the correct opEquals signature for structs?

reply Johannes Pfau <nospam example.com> writes:
My std.uuid module doesn't compile with the latest dmd. I guess it's
because of a wrong opEquals signature, this is what I have now:

----------
 safe pure nothrow bool opEquals(ref const UUID s) const
{
    return s.data == this.data;
}
----------

and

----------
assert(UUID("00000000-0000-0000-0000-000000000000") == nilUUID);
----------
The complete code is here:
https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d

What's the best way to solve this issue?
Mar 13 2012
next sibling parent Johannes Pfau <nospam example.com> writes:
Am Tue, 13 Mar 2012 19:28:26 +0100
schrieb Johannes Pfau <nospam example.com>:

 My std.uuid module doesn't compile with the latest dmd. I guess it's
 because of a wrong opEquals signature, this is what I have now:
 
 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
     return s.data == this.data;
 }
 ----------
 
 and
 
 ----------
 assert(UUID("00000000-0000-0000-0000-000000000000") == nilUUID);
 ----------
 The complete code is here:
 https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d
 
 What's the best way to solve this issue?

sorry, forgot the error message: -------- Error: function std.uuid.UUID.opEquals (ref const(UUID) s) const is not callable using argument types (UUID) Error: UUID([cast(ubyte)138u,cast(ubyte)179u,cast(ubyte)6u,cast(ubyte)14u,cast(ubyte)44u,cast(ubyte)186u,cast(ubyte)79u,cast(ubyte)35u,cast(ubyte)183u,cast(ubyte)76u,cast(ubyte)181u,cast(ubyte)45u,cast(ubyte)179u,cast(ubyte)189u,cast(ubyte)251u,cast(ubyte)70u]) is not an lvalue --------
Mar 13 2012
prev sibling next sibling parent reply =?ISO-8859-1?Q?Alex_R=F8nne_Petersen?= <xtzgzorex gmail.com> writes:
On 13-03-2012 19:28, Johannes Pfau wrote:
 My std.uuid module doesn't compile with the latest dmd. I guess it's
 because of a wrong opEquals signature, this is what I have now:

 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
      return s.data == this.data;
 }
 ----------

 and

 ----------
 assert(UUID("00000000-0000-0000-0000-000000000000") == nilUUID);
 ----------
 The complete code is here:
 https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d

 What's the best way to solve this issue?

Welcome to opEquals/opCmp/toHash hell. ;) Try removing the ref on the parameter. (Stylistic note: use equals_t instead of bool.) -- - Alex
Mar 13 2012
next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Alex R. Petersen:

 (Stylistic note: use equals_t instead of bool.)

What is equals_t? See it's used here: http://dlang.org/phobos/object.html Bye, bearophile
Mar 13 2012
parent reply =?ISO-8859-1?Q?Alex_R=F8nne_Petersen?= <xtzgzorex gmail.com> writes:
On 13-03-2012 21:39, bearophile wrote:
 Alex R. Petersen:

 (Stylistic note: use equals_t instead of bool.)

What is equals_t? See it's used here: http://dlang.org/phobos/object.html Bye, bearophile

It's just aliased to bool right now. -- - Alex
Mar 13 2012
parent reply =?ISO-8859-1?Q?Alex_R=F8nne_Petersen?= <xtzgzorex gmail.com> writes:
On 13-03-2012 22:08, H. S. Teoh wrote:
 On Tue, Mar 13, 2012 at 10:02:19PM +0100, Alex Rønne Petersen wrote:
 On 13-03-2012 21:39, bearophile wrote:
 Alex R. Petersen:

 (Stylistic note: use equals_t instead of bool.)

What is equals_t? See it's used here: http://dlang.org/phobos/object.html Bye, bearophile

It's just aliased to bool right now.

What's the rationale for using equals_t instead of bool? To extend to a ternary comparison? I thought that case was already covered by opCmp()? T

It was an integer in the past (believe it or not). :) equals_t made the transition easier. -- - Alex
Mar 13 2012
parent reply bearophile <bearophileHUGS lycos.com> writes:
Alex R. Petersen:

 It was an integer in the past (believe it or not). :) equals_t made the 
 transition easier.

Thank you for your answers, now I understand. Using an int makes sense for opEquals, because if opEquals doesn't get inlined then using int is sometimes able to give you a bit more efficiency (there is no need to convert values different from 0 and 1 to 1). I don't know how much this saves you on modern CPUs (probably no more than few CPU cycles). Bye, bearophile
Mar 13 2012
parent reply =?ISO-8859-1?Q?Alex_R=F8nne_Petersen?= <xtzgzorex gmail.com> writes:
On 13-03-2012 23:35, bearophile wrote:
 Alex R. Petersen:

 It was an integer in the past (believe it or not). :) equals_t made the
 transition easier.

Thank you for your answers, now I understand. Using an int makes sense for opEquals, because if opEquals doesn't get inlined then using int is sometimes able to give you a bit more efficiency (there is no need to convert values different from 0 and 1 to 1). I don't know how much this saves you on modern CPUs (probably no more than few CPU cycles). Bye, bearophile

Most compilers implement booleans as native integers and narrow/expand them when storing/loading to/from memory, so it's unlikely to matter at all. -- - Alex
Mar 13 2012
parent bearophile <bearophileHUGS lycos.com> writes:
Alex R. Petersen:

 Most compilers implement booleans as native integers and narrow/expand 
 them when storing/loading to/from memory, so it's unlikely to matter at all.

If you have the int value 25, this is a true value, in C you are free to use it for its zero/nonzero quality. But if opEquals is required to return a bool, the value 25 has to become 1, this is not just a narrowing. This conversion requires one instruction, I think. Bye, bearophile
Mar 13 2012
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Mar 13, 2012 at 10:02:19PM +0100, Alex Rønne Petersen wrote:
 On 13-03-2012 21:39, bearophile wrote:
Alex R. Petersen:

(Stylistic note: use equals_t instead of bool.)

What is equals_t? See it's used here: http://dlang.org/phobos/object.html Bye, bearophile

It's just aliased to bool right now.

What's the rationale for using equals_t instead of bool? To extend to a ternary comparison? I thought that case was already covered by opCmp()? T -- Democracy: The triumph of popularity over principle. -- C.Bond
Mar 13 2012
prev sibling next sibling parent reply "Jesse Phillips" <Jessekphillips+D gmail.com> writes:
On Tuesday, 13 March 2012 at 18:28:27 UTC, Johannes Pfau wrote:
 My std.uuid module doesn't compile with the latest dmd. I guess 
 it's
 because of a wrong opEquals signature, this is what I have now:

 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
     return s.data == this.data;
 }
 ----------

I think Alex is right, does auto ref work for parameters? safe pure nothrow equals_t opEquals(auto ref const UUID s) const
Mar 13 2012
parent =?UTF-8?B?QWxleCBSw7hubmUgUGV0ZXJzZW4=?= <xtzgzorex gmail.com> writes:
On 13-03-2012 19:43, Jesse Phillips wrote:
 On Tuesday, 13 March 2012 at 18:28:27 UTC, Johannes Pfau wrote:
 My std.uuid module doesn't compile with the latest dmd. I guess it's
 because of a wrong opEquals signature, this is what I have now:

 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
 return s.data == this.data;
 }
 ----------

I think Alex is right, does auto ref work for parameters? safe pure nothrow equals_t opEquals(auto ref const UUID s) const

I think auto ref will only work if it is templatized (unfortunately). But that just might work for structs, since it's not an overridden function... i.e.: safe pure nothrow equals_t opEquals()(auto ref const UUID s) const -- - Alex
Mar 13 2012
prev sibling next sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Tuesday, March 13, 2012 19:28:26 Johannes Pfau wrote:
 My std.uuid module doesn't compile with the latest dmd. I guess it's
 because of a wrong opEquals signature, this is what I have now:
 
 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
 return s.data == this.data;
 }
 ----------
 
 and
 
 ----------
 assert(UUID("00000000-0000-0000-0000-000000000000") == nilUUID);
 ----------
 The complete code is here:
 https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d
 
 What's the best way to solve this issue?

At present, I believe that the correct solution is to have two opEquals. That's what Kenji did when he fixed the various structs in Phobos recently. For instane, std.datetime.SysTime's opEquals now looks like this: bool opEquals(const SysTime rhs) const pure nothrow { return opEquals(rhs); } /// ditto bool opEquals(const ref SysTime rhs) const pure nothrow { return _stdTime == rhs._stdTime; } Ideally, auto ref would work, but it currently only works with templates. I believe that that's supposed to be changed, but it hasn't been yet. - Jonathan M Davis
Mar 13 2012
parent reply =?UTF-8?B?QWxleCBSw7hubmUgUGV0ZXJzZW4=?= <xtzgzorex gmail.com> writes:
On 13-03-2012 21:55, Jonathan M Davis wrote:
 On Tuesday, March 13, 2012 19:28:26 Johannes Pfau wrote:
 My std.uuid module doesn't compile with the latest dmd. I guess it's
 because of a wrong opEquals signature, this is what I have now:

 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
 return s.data == this.data;
 }
 ----------

 and

 ----------
 assert(UUID("00000000-0000-0000-0000-000000000000") == nilUUID);
 ----------
 The complete code is here:
 https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d

 What's the best way to solve this issue?

At present, I believe that the correct solution is to have two opEquals. That's what Kenji did when he fixed the various structs in Phobos recently. For instane, std.datetime.SysTime's opEquals now looks like this: bool opEquals(const SysTime rhs) const pure nothrow { return opEquals(rhs); } /// ditto bool opEquals(const ref SysTime rhs) const pure nothrow { return _stdTime == rhs._stdTime; } Ideally, auto ref would work, but it currently only works with templates. I believe that that's supposed to be changed, but it hasn't been yet. - Jonathan M Davis

Did you see my other post? Maybe we could do something like this: equals_t opEquals()(const auto ref SysTime rhs) const pure nothrow -- - Alex
Mar 13 2012
parent reply =?UTF-8?B?QWxleCBSw7hubmUgUGV0ZXJzZW4=?= <xtzgzorex gmail.com> writes:
On 14-03-2012 01:10, Jonathan M Davis wrote:
 On Tuesday, March 13, 2012 22:03:45 Alex Rønne Petersen wrote:
 Did you see my other post? Maybe we could do something like this:

 equals_t opEquals()(const auto ref SysTime rhs) const pure nothrow

That would probably work (though I wouldn't use equals_t, since it seems like an utterly pointless alias to me). Still, it shouldn't have to be templated to work with auto ref. - Jonathan M Davis

That's arguable... The thing is, auto ref, when used on class methods cannot work. The reason is simple: Inheritance. An overriding method can't magically take both a value and a reference. Obviously we can special-case auto ref on structs, but... is this really desirable? -- - Alex
Mar 13 2012
next sibling parent =?UTF-8?B?QWxleCBSw7hubmUgUGV0ZXJzZW4=?= <xtzgzorex gmail.com> writes:
On 14-03-2012 01:18, Jonathan M Davis wrote:
 On Wednesday, March 14, 2012 01:14:04 Alex Rønne Petersen wrote:
 On 14-03-2012 01:10, Jonathan M Davis wrote:
 On Tuesday, March 13, 2012 22:03:45 Alex Rønne Petersen wrote:
 Did you see my other post? Maybe we could do something like this:

 equals_t opEquals()(const auto ref SysTime rhs) const pure nothrow

That would probably work (though I wouldn't use equals_t, since it seems like an utterly pointless alias to me). Still, it shouldn't have to be templated to work with auto ref. - Jonathan M Davis

That's arguable... The thing is, auto ref, when used on class methods cannot work. The reason is simple: Inheritance. An overriding method can't magically take both a value and a reference. Obviously we can special-case auto ref on structs, but... is this really desirable?

As I understand it, auto ref is supposed to work with _any_ function. The _compiler_ decides whether it's best to use a ref or a value. That may mean that you actually get two of the same function. I don't know. I do know that Walter misunderstood what Andrei meant and made it a template thing when it wasn't supposed to be. Supposedly, he's going to fix it, but he hasn't yet. - Jonathan M Davis

How would it ever work? One entry in a vtable can't point to two functions. -- - Alex
Mar 13 2012
prev sibling parent =?ISO-8859-1?Q?Alex_R=F8nne_Petersen?= <xtzgzorex gmail.com> writes:
On 14-03-2012 01:43, Andrej Mitrovic wrote:
 On 3/14/12, Jonathan M Davis<jmdavisProg gmx.com>  wrote:
 As I understand it, auto ref is supposed to work with _any_ function. The
 _compiler_ decides whether it's best to use a ref or a value.

I never really understood the need for 'const ref' with structures. If the compiler knows the size of a structure shouldn't it be able to automatically figure out if it's faster to pass a struct by value or by pointer? But maybe there's more to it than that?

struct S { int i; } void foo(S s) // compiler decides to pass by ref { s = S(2); } S s = S(1); foo(s); assert(s.i == 1); // fails - wat? -- - Alex
Mar 13 2012
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Tuesday, March 13, 2012 22:03:45 Alex Rønne Petersen wrote:
 Did you see my other post? Maybe we could do something like this:
 
 equals_t opEquals()(const auto ref SysTime rhs) const pure nothrow

That would probably work (though I wouldn't use equals_t, since it seems like an utterly pointless alias to me). Still, it shouldn't have to be templated to work with auto ref. - Jonathan M Davis
Mar 13 2012
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, March 14, 2012 01:14:04 Alex Rønne Petersen wrote:
 On 14-03-2012 01:10, Jonathan M Davis wrote:
 On Tuesday, March 13, 2012 22:03:45 Alex Rønne Petersen wrote:
 Did you see my other post? Maybe we could do something like this:
 
 equals_t opEquals()(const auto ref SysTime rhs) const pure nothrow

That would probably work (though I wouldn't use equals_t, since it seems like an utterly pointless alias to me). Still, it shouldn't have to be templated to work with auto ref. - Jonathan M Davis

That's arguable... The thing is, auto ref, when used on class methods cannot work. The reason is simple: Inheritance. An overriding method can't magically take both a value and a reference. Obviously we can special-case auto ref on structs, but... is this really desirable?

As I understand it, auto ref is supposed to work with _any_ function. The _compiler_ decides whether it's best to use a ref or a value. That may mean that you actually get two of the same function. I don't know. I do know that Walter misunderstood what Andrei meant and made it a template thing when it wasn't supposed to be. Supposedly, he's going to fix it, but he hasn't yet. - Jonathan M Davis
Mar 13 2012
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 3/14/12, Jonathan M Davis <jmdavisProg gmx.com> wrote:
 As I understand it, auto ref is supposed to work with _any_ function. The
 _compiler_ decides whether it's best to use a ref or a value.

I never really understood the need for 'const ref' with structures. If the compiler knows the size of a structure shouldn't it be able to automatically figure out if it's faster to pass a struct by value or by pointer? But maybe there's more to it than that?
Mar 13 2012
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, March 14, 2012 01:43:54 Andrej Mitrovic wrote:
 On 3/14/12, Jonathan M Davis <jmdavisProg gmx.com> wrote:
 As I understand it, auto ref is supposed to work with _any_ function. The
 _compiler_ decides whether it's best to use a ref or a value.

I never really understood the need for 'const ref' with structures. If the compiler knows the size of a structure shouldn't it be able to automatically figure out if it's faster to pass a struct by value or by pointer? But maybe there's more to it than that?

In C++, the compiler _never_ decides that sort of thing. It always passes stuff the way that you tell it to. Before auto ref, D was exactly the same way. If you tell function's parameter is ref or const ref, then it takes it be reference, otherwise it takes it by value. That works great in C++, because const& in C++ will take rvalues, but D doesn't do that (if nothing else, because Andrei is completely against it on the grounds that it makes it so that a function can't determine whether it's dealing with an rvalue or an lvalue when given a const&; I don't understand why this is a problem - in fact I think that most people don't - but he's quite adamant about it, and he may very well be right). So, D ends up with ref and const ref, just like C++'s & and const& except that const& doesn't take rvalues, which is where all the pain starts. Ultimately, that resulted in the suggestion of auto ref, which _does_ allow the compiler to decide - unlike every other parameter type. So, the reasons for const ref really have nothing to do with auto ref, and I'd tend to argue that const ref is pretty pointless at this point. If you want the struct to be passed by value, then don't use auto ref or ref. If you want it to be passed by ref, then pass by ref. And if you don't care, then use auto ref. const ref just isn't needed. But since it precedes auto ref, we have it. And until auto ref works with non-templated functions, const ref is still needed. - Jonathan M Davis
Mar 13 2012
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, March 14, 2012 01:36:40 Alex Rønne Petersen wrote:
 On 14-03-2012 01:18, Jonathan M Davis wrote:
 On Wednesday, March 14, 2012 01:14:04 Alex Rønne Petersen wrote:
 On 14-03-2012 01:10, Jonathan M Davis wrote:
 On Tuesday, March 13, 2012 22:03:45 Alex Rønne Petersen wrote:
 Did you see my other post? Maybe we could do something like this:
 
 equals_t opEquals()(const auto ref SysTime rhs) const pure nothrow

That would probably work (though I wouldn't use equals_t, since it seems like an utterly pointless alias to me). Still, it shouldn't have to be templated to work with auto ref. - Jonathan M Davis

That's arguable... The thing is, auto ref, when used on class methods cannot work. The reason is simple: Inheritance. An overriding method can't magically take both a value and a reference. Obviously we can special-case auto ref on structs, but... is this really desirable?

As I understand it, auto ref is supposed to work with _any_ function. The _compiler_ decides whether it's best to use a ref or a value. That may mean that you actually get two of the same function. I don't know. I do know that Walter misunderstood what Andrei meant and made it a template thing when it wasn't supposed to be. Supposedly, he's going to fix it, but he hasn't yet. - Jonathan M Davis

How would it ever work? One entry in a vtable can't point to two functions.

I don't know. I believe that Timon had an explanation for how auto ref is supposed to work that he posted in a discussion semi-recently, but I don't remember the details. Regardless, as I understand it, it _is_ possible to have a non-templated function which can take an argument by either ref or value at the compiler's discretion and that that's what auto ref is supposed to do. But I don't know the details. - Jonathan M Davis
Mar 13 2012
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 3/14/12, Alex R=F8nne Petersen <xtzgzorex gmail.com> wrote:
 void foo(S s) // compiler decides to pass by ref
 {
      s =3D S(2);
 }

Well in this case it wouldn't pass by ref since it sees an assignment. But I can see how this would become tricky business (e.g. "why is my code slow all of a sudden"). const ref has it's uses then. :p
Mar 13 2012
prev sibling parent Johannes Pfau <nospam example.com> writes:
Am Tue, 13 Mar 2012 19:31:45 +0100
schrieb Alex R=C3=B8nne Petersen <xtzgzorex gmail.com>:

 On 13-03-2012 19:28, Johannes Pfau wrote:
 My std.uuid module doesn't compile with the latest dmd. I guess it's
 because of a wrong opEquals signature, this is what I have now:

 ----------
  safe pure nothrow bool opEquals(ref const UUID s) const
 {
      return s.data =3D=3D this.data;
 }
 ----------

 and

 ----------
 assert(UUID("00000000-0000-0000-0000-000000000000") =3D=3D nilUUID);
 ----------
 The complete code is here:
 https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d

 What's the best way to solve this issue?

Welcome to opEquals/opCmp/toHash hell. ;) =20 Try removing the ref on the parameter. =20 (Stylistic note: use equals_t instead of bool.) =20

Thanks all for your answers! Removing the ref / adding an overload without the ref indeed fixed this problem. I didn't change bool to equals_t yet, that's probably a question for the review.
Mar 14 2012