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.

-- 
/Jacob Carlborg

On 7/25/14, 3:31 AM, Jonathan M Davis wrote:
> On Friday, 25 July 2014 at 04:50:38 UTC, Walter Bright wrote:
>> On 7/23/2014 9:45 AM, H. S. Teoh via Digitalmars-d wrote:
>>> https://issues.dlang.org/show_bug.cgi?id=13179
>>
>> Consider also:
>>
>> http://www.reddit.com/r/programming/comments/2bl51j/programming_in_d_a_great_online_book_for_learning/cj75gm9
>>
>>
>> The current scheme breaks existing code - code that was formerly
>> correct and working.
>>
>> AAs don't make sense if the notion of == on members is invalid. AAs
>> formerly required opCmp, and yes, opCmp could be constructed to give
>> different results for opCmp==0 than ==, but I would expect such an
>> object to not be used in an AA, again because it doesn't make sense.
>>
>> Using the default generated opEquals for AAs may break code, such as
>> the an AA of the structs in the parent post, but it seems unlikely
>> that that code was correct anyway in an AA (as it would give erratic
>> results).
>
> Exactly. The only reason that switching from using lhs.opCmp(rhs) == 0
> to opEquals would break code is if a type does not define them such that
> they're equivalent, which would mean that opEquals and/or opCmp was
> defined in a buggy manner. So, the only way that the change would break
> code is if it was broken in the first place. All it risks is making it
> so that the bug exhibits itself in an additional case.

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.

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?

-- 
/Jacob Carlborg

On 25 July 2014 22:06, Jacob Carlborg via Digitalmars-d < digitalmars-d@puremagic.com> wrote:

> On 25/07/14 12:39, Jonathan M Davis wrote:
>
>  But regardless of whether the efficiency cost is large, you're talking
>> about incurring it just to fix the code of folks who couldn't be bothered to make sure that opEquals and lhs.opCmp(rhs) == 0 were equivalent. You'd be punishing correct code (however slight that punishment may be) in order to fix the code of folks who didn't even properly test basic functionality. I see no reason to care about trying to help out folks who can't even be bothered to test opEquals and opCmp, especially when that help isn't free.
>>
>
> By Walter and Andrei's definition opCmp is not to be used for equivalent, therefor opCmp does never need to be equal to 0.


Yes it does, <= and >= are both things that you can type.

On Fri, Jul 25, 2014 at 09:46:55AM +0000, Jonathan M Davis via Digitalmars-d wrote:
> On Friday, 25 July 2014 at 08:21:26 UTC, Jacob Carlborg wrote:
> >By defining opEquals to be opCmp == 0 we're:
> >
> >1. We're not breaking code where it wasn't broken previously
> >2. We're fixing broken code. That is when opEqual and opCmp == 0 gave
> >different results
> 
> Code that worked perfectly fine before is now slower, because it's using opCmp for opEquals when it wasn't before.

I don't understand why you keep bringing up the point of being slower. I thought the whole point of D was to be safe first, then performant if you ask for it. In this case, sure there will be a (small!) performance hit, but then the solution is just to define opEquals yourself -- which you should have been doing in the first place! So this is really just prodding the programmer in the right direction.


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


> So, in order to avoid a performance hit on opEquals from defining opCmp, you now have to define toHash, which significantly increases the chances of bugs. And regardless of the increased risk of bugs, it's extra code that you shouldn't need to write anyway, because the normal, default opEquals and toHash worked just fine.
> 
> I honestly have no sympathy for anyone who defined opCmp to be different from the default opEquals but didn't define opEquals. Getting that right is simple, and it's trivial to test for you're unit testing like you should be.

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.


> I don't want to pay in my code just to make the compiler friendlier to someone who didn't even bother to do something so simple.
[...]

And you don't have to. You just define opEquals correctly as you have always done, and you pay *nothing*. The only time you pay is when you forgot to define opEquals -- in which case, which is worse, bad performance, or incorrect code? Perhaps you have different priorities, but I'd rather have bad performance than incorrect code, especially *subtly* wrong code that's very difficult to track down.


[...]
> I'd much rather be able to take advantage of the fast, default opEquals and correct toHash than be forced to define them just because I defined opCmp and didn't want a performance hit on opEquals.
[...]

So perhaps we should implement `bool opEquals = default;`.


T

-- 
My program has no bugs! Only unintentional features...

"H. S. Teoh via Digitalmars-d"  wrote in message news:mailman.336.1406295294.32463.digitalmars-d@puremagic.com...

> So perhaps we should implement `bool opEquals = default;`.

No new syntax. 

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. I argue that it's in the *common* case of linear orderings, where people are more liable to assume (incorrectly, it seems!) that opEquals should default to opCmp()==0 -- and that's the case we should be addressing. Let's not lose sight of the forest for the minority of the trees.


T

-- 
The peace of mind---from knowing that viruses which exploit Microsoft system vulnerabilities cannot touch Linux---is priceless. -- Frustrated system administrator.

On Fri, Jul 25, 2014 at 11:42:59PM +1000, Daniel Murphy via Digitalmars-d wrote:
> "H. S. Teoh via Digitalmars-d"  wrote in message news:mailman.336.1406295294.32463.digitalmars-d@puremagic.com...
> 
> >So perhaps we should implement `bool opEquals = default;`.
> 
> No new syntax.

*shrug* That's just what Jonathan suggested earlier.

At worst, you could just write:

	bool opEquals(T t) {
		return typeid(this).equals(&this, &t);
	}

It's a little more typing, but surely it's not *that* hard??


T

-- 
"Hi." "'Lo."

On Fri, Jul 25, 2014 at 02:08:55PM +0200, Jacob Carlborg via Digitalmars-d 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.
[...]

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.

The issue at hand is really more of easing the transition from the old buggy design so that we don't break old code where they used to work correctly.


T

-- 
Error: Keyboard not attached. Press F1 to continue. -- Yoon Ha Lee, CONLANG

On Fri, Jul 25, 2014 at 02:37:46AM -0700, Walter Bright via Digitalmars-d wrote:
> On 7/25/2014 2:12 AM, Jacob Carlborg wrote:
> >On 25/07/14 10:48, Walter Bright wrote:
> >>Putting it simply,
> >>
> >>1. == uses opEquals. If you don't supply opEquals, the compiler will make one for you.
> >>
> >>2. AAs use ==. See rule 1.
> >>
> >>
> >>Easy to understand, easy to explain, easy to document.
> >
> >It's very hard to use D when it constantly changes and breaks code. It's especially annoying reading your comments on reddit that we must stop break code. Then a few days later go an break code. I really hope no one gets false hopes from those comments.
> 
> 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.

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.

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

>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 -- and all your opCmp's are now useless 'cos that was wrong in the first place". From the perspective of the user, this seems like unreasonable code breakage -- first they *already* expected in the past that their code should've worked with opEquals, but they were told to use opCmp because AA's were buggy. Yet now we're going back on our word and saying that opCmp was wrong -- the very workaround we recommended in the past -- and that now they need to define opEquals instead, which didn't work before.  This gives users the perception that we're constantly going back on our word and breaking prior code and annulling previously recommended workarounds without any warning.

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.


T

-- 
Always remember that you are unique. Just like everybody else. -- despair.com