On Friday, 25 July 2014 at 13:44:54 UTC, H. S. Teoh via Digitalmars-d wrote:
> On Fri, Jul 25, 2014 at 12:10:31PM +0100, Regan Heath via Digitalmars-d wrote:
>> On Fri, 25 Jul 2014 09:39:11 +0100, Walter Bright
>> <newshound2@digitalmars.com> wrote:
>> 
>> >On 7/25/2014 1:02 AM, Jacob Carlborg wrote:
>> >>3. If opCmp is defined but no opEquals, lhs == rhs will be lowered
>> >>to lhs.opCmp(rhs) == 0
>> >
>> >This is the sticking point. opCmp and opEquals are separate on
>> >purpose, see Andrei's posts.
>> 
>> Sure, Andrei makes a valid point .. for a minority of cases.  The
>> majority case will be that opEquals and opCmp==0 will agree.  In those
>> minority cases where they are intended to disagree the user will have
>> intentionally defined both, to be different.  I cannot think of any
>> case where a user will intend for these to be different, then not
>> define both to ensure it.
> [...]
>
> Exactly!! I don't understand why people keep bringing up non-linear
> partial orderings -- those only apply in a *minority* of cases! (Raise
> your hands if your code depends on non-linear partial orderings. How
> many of you *require* this more often than linear orderings? Yeah, I
> thought so.) Why are we sacrificing *common* case -- where opCmp defines
> a linear ordering -- for the minority case?
>
> And it's not like we're making it impossible in the minority case -- if
> you want a non-linear partial ordering, wouldn't you make sure to define
> both opCmp and opEquals so that they do the right thing? Since it's an
> uncommon use case, people would tend to be more careful when
> implementing it.

Do I miss something or wouldn't an non-linear ordering imply, that x.opCmp(y) != 0 for all x,y ∈ T and thus automatically generating opEquals to opCmd() == 0 would automatically do the right thing in this case?

So the amount of people that require a different opEquals are even smaller
and defining opEquals and opCmp for two different orderings is a code smell squared.

>> And it's not like we're making it impossible in the minority case -- if
>> you want a non-linear partial ordering, wouldn't you make sure to define
>> both opCmp and opEquals so that they do the right thing? Since it's an
>> uncommon use case, people would tend to be more careful when
>> implementing it.
>
> Do I miss something or wouldn't an non-linear ordering imply, that x.opCmp(y) != 0 for all x,y ∈ T and thus automatically generating opEquals to opCmd() == 0 would automatically do the right thing in this case?
>
> So the amount of people that require a different opEquals are even smaller
> and defining opEquals and opCmp for two different orderings is a code smell squared.

A nevermind, got my hands on a coffee now.

On Friday, 25 July 2014 at 14:10:11 UTC, H. S. Teoh via Digitalmars-d wrote:
> The whole reason the opCmp()==0 thing was brought up, was to eliminate
> this frustration -- give users a nice way to transition into the correct
> AA design of using opEquals for their keys, instead of just outright
> breaking past recommendations in their face with no warning.

Not just that, it's also the right thing to do, independently from AAs. I'm astonished that it doesn't work like that already. When I first read the operator overloading docs, I really liked that in D I don't need to define all the individual comparison operators, but only opCmp. I totally expected this to include opEquals, too, which I thought was just an option that could be used to enhance performance, but which could be safely ignored otherwise. It's really bad if this isn't the case, because then there is nothing telling you that you need an opEquals, it will just silently compile and appear to be working.

On Friday, 25 July 2014 at 13:34:55 UTC, H. S. Teoh via Digitalmars-d wrote:
> On Fri, Jul 25, 2014 at 09:46:55AM +0000, Jonathan M Davis via Digitalmars-d wrote:
>> Even worse, if you define opEquals, you're then forced to define
>> toHash, which is much harder to get right.
>
> If you're redefining opCmp and opEquals, I seriously question whether
> the default toHash actually produces the correct result. If it did, it
> begs the question, what's the point of redefining opCmp?

Except that with the current git master, you're forced to define opEquals just because you define opCmp, which would then force you to define opCmp. And with your suggested  fix of making opEquals equivalent to lhs.opCmp(rhs) == 0, then _every_ type with opCmp will have to define toHash, because the default toHash is for the default opEquals, not for a user-defined opCmp.

And remember that a lot of types have opCmp just to work with AAs, so all of a sudden, _every_ user-defined type which is used as an AA key will have to define toHash.

> Frankly, I find this rather incongruous. First you say that requiring
> programmers to define toHash themselves is too high an expectation, then
> you say that you have no sympathy on these same programmers 'cos they
> can't get their opEquals code right. If it's too much to expect them to
> write toHash properly, why would we expect them to write opEquals
> correctly either? But if they *are* expected to get opEquals right, then
> why is it a problem for them to also get toHash right? I'm honestly
> baffled at what your point is.

opEquals is trivial. toHash is much harder to get right, especially if you want a hash function that's even halfway decent.

- Jonathan M Davis

Am 25.07.2014 20:45, schrieb Jonathan M Davis:
> On Friday, 25 July 2014 at 13:34:55 UTC, H. S. Teoh via Digitalmars-d
> wrote:
>> On Fri, Jul 25, 2014 at 09:46:55AM +0000, Jonathan M Davis via
>> Digitalmars-d wrote:
>>> Even worse, if you define opEquals, you're then forced to define
>>> toHash, which is much harder to get right.
>>
>> If you're redefining opCmp and opEquals, I seriously question whether
>> the default toHash actually produces the correct result. If it did, it
>> begs the question, what's the point of redefining opCmp?
>
> Except that with the current git master, you're forced to define
> opEquals just because you define opCmp, which would then force you to
> define opCmp.

That sentence doesn't make much sense, did you mean "opHash just because you define opEquals" or something similar?

>
> opEquals is trivial. toHash is much harder to get right, especially if
> you want a hash function that's even halfway decent.
>

As written before somewhere else, a toHash that is  as decent as the current default, but limited to the fields you actually want, should be really easy, if phobos exposed a function like
  hash_t createHash(T...)(T args)
that does to args what D currently does to all members to create the default hash.

(I kinda tend towards "whatever, maybe not defaulting opEquals to a.opCmp(b) == 0 is acceptable, people should stumble upon this when first reading the documentation on how to overload operators" now, though)

Cheers,
Daniel

Am 25.07.2014 18:11, schrieb "Marc Schütz" <schuetzm@gmx.net>":
> I'm astonished that it doesn't work like that already. When I first read
> the operator overloading docs, I really liked that in D I don't need to
> define all the individual comparison operators, but only opCmp. I

Well, to be fair the documentation, is pretty explicit about it, the headings are "Overloading == and !=" and "Overloading <, <=, <, and <=".
The D1 documentation even had a rationale why there's both opEquals and opCmp, no idea why that was dropped for D2.

However, I read about opCmp at some time and in the meantime forgot about the "not for ==" part - but this is probably a problem with my brain (or the long timespan) and not with the documentation.

Cheers,
Daniel

On Friday, 25 July 2014 at 12:08:55 UTC, Jacob Carlborg wrote:
> On 25/07/14 12:09, Jonathan M Davis wrote:
>
>> The _only_ code that would break would be code that's _already_ broken -
>> code that defines opCmp in a way that's inconsistent with the default
>> opEquals and then doesn't define opEquals. I see no reason to worry
>> about making sure that we don't break code that's already broken.
>
> I see no reason why I should define opEquals when opCmp was used for AA keys. You keep ignoring that argument.

opEquals will now be used for AA keys, not opCmp. That's why git master generates errors when you have a struct which defines opCmp and not opEquals, and you try and use it as an AA key. It was done on the theory that your opEquals and opCmp might not match (which would be buggy code to begin with, so it would be forcing everyone to change their code just because someone might have gotten their opEquals and opCmp wrong).

If we keep the same behavior as 2.065 but still change the AAs to use opEquals, then there's no need to define opEquals unless the type was buggy and defined opCmp in a way that was inconsistent with the default opEquals and then didn't define one which was consistent. The code will continue to work.

H.S. Teoh wants to change the default-generated opEquals to be equivalent to lhs.opCmp(rhs) == 0 in the case where opCmp is defined in order to avoid further breaking the code of folks whose code is broken and didn't define opEquals when opCmp didn't match the default.

So, if we remove the new check for a user-defined opEquals when opCmp is defined, then you don't have to define opEquals. If we do what H.S. Teoh suggests, then you'll have to define it if you want to avoid the additional checks that opCmp would be doing that opEquals wouldn't do, but if you didn't care, then you wouldn't. If we leave it as it is in git master, then you'd always have to define it if you defined opCmp and wanted to use it as an AA key, and since opCmp was used for AA keys before, that means that _every_ type which didn't define opEquals but was used as an AA key will suddenly have to define opEquals and toHash and will thus now be broken. So, the current situation in git master is the worst all around.

- Jonathan M Davis

On Friday, 25 July 2014 at 18:54:07 UTC, Daniel Gibson wrote:
> Am 25.07.2014 20:45, schrieb Jonathan M Davis:
>> On Friday, 25 July 2014 at 13:34:55 UTC, H. S. Teoh via Digitalmars-d
>> wrote:
>>> On Fri, Jul 25, 2014 at 09:46:55AM +0000, Jonathan M Davis via
>>> Digitalmars-d wrote:
>>>> Even worse, if you define opEquals, you're then forced to define
>>>> toHash, which is much harder to get right.
>>>
>>> If you're redefining opCmp and opEquals, I seriously question whether
>>> the default toHash actually produces the correct result. If it did, it
>>> begs the question, what's the point of redefining opCmp?
>>
>> Except that with the current git master, you're forced to define
>> opEquals just because you define opCmp, which would then force you to
>> define opCmp.
>
> That sentence doesn't make much sense, did you mean "opHash just because you define opEquals" or something similar?

You're right. I meant that you would then be forced to define toHash.

- Jonathan M Davis

On Friday, 25 July 2014 at 18:45:30 UTC, Jonathan M Davis wrote:
> On Friday, 25 July 2014 at 13:34:55 UTC, H. S. Teoh via Digitalmars-d wrote:
>> On Fri, Jul 25, 2014 at 09:46:55AM +0000, Jonathan M Davis via Digitalmars-d wrote:
>>> Even worse, if you define opEquals, you're then forced to define
>>> toHash, which is much harder to get right.
>>
>> If you're redefining opCmp and opEquals, I seriously question whether
>> the default toHash actually produces the correct result. If it did, it
>> begs the question, what's the point of redefining opCmp?
>
> Except that with the current git master, you're forced to define opEquals just because you define opCmp, which would then force you to define opCmp. And with your suggested  fix of
(assuming you mean "toHash")
> making opEquals equivalent to lhs.opCmp(rhs) == 0, then _every_ type with opCmp will have to define toHash, because the default toHash is for the default opEquals, not for a user-defined opCmp.

No, only those types that define opCmp _and_ are going to be used as AA keys, and that's sensible. All others don't need toHash.

>
> And remember that a lot of types have opCmp just to work with AAs, so all of a sudden, _every_ user-defined type which is used as an AA key will have to define toHash.

No, if a type had only defined opCmp because of the previous AA (mis)implementation, it needs to be changed with any of the suggested solutions: If opEquals is not going to be auto-generated, the user needs to add it, if it is, the user has the choice between adding toHash, or (more likely, as opCmp usually isn't necessary) changing opCmp into opEquals.

On Friday, 25 July 2014 at 18:54:15 UTC, Daniel Gibson wrote:
> Am 25.07.2014 18:11, schrieb "Marc Schütz" <schuetzm@gmx.net>":
>> I'm astonished that it doesn't work like that already. When I first read
>> the operator overloading docs, I really liked that in D I don't need to
>> define all the individual comparison operators, but only opCmp. I
>
> Well, to be fair the documentation, is pretty explicit about it, the headings are "Overloading == and !=" and "Overloading <, <=, <, and <=".

Whatever the outcome of the discussion will be, it needs to be documented much better. The current documentation doesn't say anything about whether or not, and how opEquals and opCmp relate. I doesn't even mention that they are supposed to be consistent.

I'm just afraid that it will not be noticed, because it will be "hidden" in the documentation. If the status quo is kept, you just won't know you've written wrong code, even though the compiler has all the means to tell you.

> The D1 documentation even had a rationale why there's both opEquals and opCmp, no idea why that was dropped for D2.
>
> However, I read about opCmp at some time and in the meantime forgot about the "not for ==" part - but this is probably a problem with my brain (or the long timespan) and not with the documentation.

Well, you're not the only one :-(