On Friday, 25 July 2014 at 13:44:54 UTC, H. S. Teoh via Digitalmars-d wrote:
> Exactly!! I don't understand why people keep bringing up non-linear
> partial orderings -- those only apply in a *minority* of cases!

Well, if <, <= etc are only to be used where you have a "natural" total order then I guess you are right, but then opCmp should be limited to those types.

Since comparison and boolean operators (&& etc) cannot be defined in a general manner that will work with all branches of mathematics, maybe they should be limited to total orders.

It is awfully limiting for DSL-style programming, though. As I've pointed out before, how D limits && and || prevents readable fuzzy logic and opCmp prevents useful fuzzy number operators. So, since the limits are there already, maybe forbidding orders on complex types, and vectors etc is sensible too… (Why limit some types and not others?)

> Since it's an
> uncommon use case, people would tend to be more careful when
> implementing it. I argue that it's in the *common* case of linear orderings, where people are more liable to assume

It is quite common to want an order on domain-specific objects without discriminating all cases, unless you do direct comparison for equality. Say if you have a colour type you might want an order on chromacity, but not on intensity. If you have a vector, you might want an order on magnitude, but not on direction. If you have a normal vector you might want an order on acute angles, e.g. define a<b === is_acute_angle(a,b).

If opCmp automatically defines equality, then you have to remember to undefine it.  Equality as opCmp would be slow and wrong in the case of "order by magnitude":

(a.x*a.x + a.y*a.y) == (b.x*b.x + b.y*b.y)

This can go undetected by the programmer if you use a mixin to add "standard operators" or if you don't care about equality in the actual type, but it is used for equality comparison in an aggregate (a struct that has the type as a field).

A more sensible approach from a correctness viewpoint is to require equality to be defined explicitly if you provide opCmp. I mean, if you want to argue for CORRECTNESS, take it all the way. Not 50% convinience, 50% correctness, and a dash flexibility, but not quite enough to cover the most pragmatic branches of mathematics…

On 2014-07-25 15:51, H. S. Teoh via Digitalmars-d wrote:

> AA's don't care about keys being orderable, all they care about is that
> keys should have a hash value, and be comparable. It was a mistake to
> use opCmp for AA keys in the first place. We're now fixing this mistake.

I'm responding to Jonathan's claims that types that defined opCmp but not opEquals are broken.

-- 
/Jacob Carlborg

On 2014-07-25 20:56, Jonathan M Davis wrote:

> opEquals will now be used for AA keys, not opCmp.

Well, yes. But that was not the case when the code was written. In that case it was to correct to defined 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.

That's what I'm saying. I don't understand what you're arguing for/against.

-- 
/Jacob Carlborg

On Fri, Jul 25, 2014 at 07:21:04PM +0000, via Digitalmars-d wrote:
> On Friday, 25 July 2014 at 13:44:54 UTC, H. S. Teoh via Digitalmars-d wrote:
> >Exactly!! I don't understand why people keep bringing up non-linear partial orderings -- those only apply in a *minority* of cases!
> 
> Well, if <, <= etc are only to be used where you have a "natural" total order then I guess you are right, but then opCmp should be limited to those types.

No it doesn't have to be. If you want it to work with non-linear orders, just define your own opEquals. We're talking about the *default* behaviour here. Linear orders should be default because they are what people want (and expect) in the majority of cases. Nobody said anything about making it *impossible* to define non-linear orderings.


T

-- 
It only takes one twig to burn down a forest.

On 7/25/2014 5:10 AM, Jacob Carlborg wrote:
> On 25/07/14 11:37, Walter Bright wrote:
>
>> We went through the likely code breakage from this in this thread, and
>> it's hard to see any non-broken code breaking. It will also fix
>> regression https://issues.dlang.org/show_bug.cgi?id=13179 and stop that
>> breakage.
>
> So opEquals will not be required to be defined if opCmp is defined if it's used
> as an AA key?

Right.

On Friday, 25 July 2014 at 19:03:16 UTC, Marc Schütz wrote:
> On Friday, 25 July 2014 at 18:45:30 UTC, Jonathan M Davis wrote:
>> 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.

No, if the opCmp is consistent with the default-generated opEquals, then there's no need to define either opEquals or opCmp. And if opCmp was inconsistent with the default opEquals, then toHash would have already had to have been defined to be consistent with opCmp, or the AA wouldn't have worked properly regardless.

So, if we let opEquals and toHash continue to be generated by the compiler when opCmp is defined, the only folks who would now have to define opEquals or toHash who didn't before would be the folks who should have been defining them previously to be consistent with opCmp but didn't. Whereas with the current git master, they'd all have to define opEquals and toHash. With H.S. Teoh's suggestion, opEquals wouldn't have to be defined but toHash would (since the compiler has no way of knowing that lhs.opCmp(rhs) == 0 is equivalent to the default opEquals that the default toHash is consistent with).

The current option breaks all AA-related code that didn't define opEquals and toHash already (which a lot of code didn't have to do), and H.S. Teoh's option breaks all AA-related code that didn't define toHash already. So, the option that causes the least code breakage is to let the compiler continue to define opEquals and toHash as it has, even if opCmp has been defined. The only risk is if opCmp wasn't consistent with the default opEquals, but if that's the case, the code was already broken anyway.

- Jonathan M Davis

On 7/25/2014 7:08 AM, H. S. Teoh via Digitalmars-d wrote:
> You're missing the fact that:
>
> 1) Before we fixed the use of typeinfo.compare in aaA.d, users were
> *required* to define opCmp in order for their types to be usable as AA
> keys -- because that's what aaA.d used to compare keys. This applies
> even if the user type has no meaningful partial ordering -- you still
> had to define opCmp because otherwise it just plain won't work properly.

I'm not missing that at all. I wrote that requirement.


> 2) Because of (1), there's now code out there that defines opCmp for
> user types just so that they can be used as AA keys. But we're now
> breaking that by saying "nyah nyah we're now using opEquals for AA keys,
> so your opCmp don't work no more, sux to be you!".

No, we're not breaking code, unless the user wrote an opCmp that doesn't produce the same result as ==. This kind of struct would not make sense to use in an AA, and that code is most likely very rare and quite broken.


> From the perspective of the user, this can be extremely frustrating:
> prior to 2.066, they were told "you must define opCmp otherwise your AA
> keys won't work properly". So like obedient little lambs they went ahead
> and did that. And now in 2.066 we're saying "you must define opEquals
> otherwise your AA keys won't work properly

Nope. If the user doesn't write opEquals, the compiler will generate one for him, using the same method as it did for ==.


> 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.

As I explained to Jacob, such a workaround will introduce subtle problems that are going to be hard to live with.

On Friday, 25 July 2014 at 20:07:08 UTC, Jacob Carlborg wrote:
> On 2014-07-25 20:56, Jonathan M Davis wrote:
>
>> opEquals will now be used for AA keys, not opCmp.
>
> Well, yes. But that was not the case when the code was written. In that case it was to correct to defined opCmp.

Yes, but opCmp always had to be consistent with opEquals, or the code was broken. If the code was "lucky," and the key type was only ever used with opCmp and never opEquals, then the bug wouldn't have manifested itself, but the odds of that are probably low, and it's a bug regardless.

>> 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.
>
> That's what I'm saying. I don't understand what you're arguing for/against.

I'm arguing for _not_ have the compiler complain if you defined opCmp but not opEquals but to do exactly what it did with 2.065 - which is to automatically define opEquals and toHash for you if you didn't define them.

H.S. Teoh is arguing for changing it so that the generated opEquals uses opCmp if it's present, which generally makes the performance of the generated opEquals worse if opCmp was consistent with the default opEquals (which it should have been, since it didn't define its own), and worse, it forces all types used as keys in AAs to define toHash, because then the generated opEquals is no longer consistent with the generated toHash.

What I'm arguing for would only break code which failed to define opEquals to be consistent with opCmp - so only code which is already broken. No other code would break. However, the current situation with git master and H.S. Teoh's suggestion would both break existing code.

- Jonathan M Davis

On 7/25/2014 5:08 AM, Jacob Carlborg wrote:
> I see no reason why I should define opEquals when opCmp was used for AA keys.
> You keep ignoring that argument.

Allow me to repeat:

"The thing is, either this suffers from == behaving differently than AAs, or you've made opEquals superfluous by defining it to be opCmp==0. The latter is a mistake, as Andrei has pointed out, as opCmp may not have a concept of equality, and opEquals may not have a concept of ordering."

This is the crux of the matter.

On 7/25/2014 5:10 AM, Ary Borenszweig wrote:
> Not at all.
>
> If you have a type that has partial ordering (only cares about opCmp, not about
> opEquals), but still keeps the default opEquals, then this would silently break
> someone's code by changing their opEquals semantic.
>
> THIS is the breaking change.

Yes. A subtle but extremely important point. Comparison and Equality are fundamentally different operations. Defining opEquals to be the equivalent of opCmp==0 is utterly breaking that.