Issue 13074 - Old opCmp requirement for AA keys should be detected #3731
Conversation
| } | ||
| if (sd->xcmp && sd->xcmp != sd->xerrcmp) | ||
| { | ||
| error(loc, "%sAA key type %s is now required equality rather than comparison", |
There was a problem hiding this comment.
"%s AA key type %s now requires equality rather than comparison."
|
I tested this pull, it fixes the problem from what I see. I will play around a bit with some other possibilities. I'll also look at phobos failures. |
|
Phobos fix: dlang/phobos#2313 |
|
One more phobos fix: dlang/phobos#2314 |
| TEST_OUTPUT: | ||
| --- | ||
| fail_compilation/diag13074.d(29): Error: AA key type S is now required equality rather than comparison | ||
| fail_compilation/diag13074.d(29): Please define opEquals, or remove it to rely on default memberwise equality |
There was a problem hiding this comment.
Not sure whether it's critical to the test actually succeeding, but these messages should also be fixed.
|
Ready to merge. |
|
@yebblies Can you merge this? This is a blocker to prevent silent behavior change around AAs in 2.066. |
|
I haven't really been following the related pulls - why is this better than generating an opEquals that internally calls the user opCmp? |
|
In git-head, AA implementation is changed to use But, in 2.065 and earlier, AA had been used |
|
But can't an AA still function perfectly if a type defines opCmp and opHash, but not opEquals? Shouldn't we be able to make TypeInfo.equals call opCmp and preserve the old behavior? Or is there some reason opCmp can't be used to implement opEquals? |
|
I don't say that automatic opEquals generation from opCmp is a bad idea. But it is an operator overloading enhancement, that is essentially orthogonal with AA implementation changing. It should have another discussion. |
|
Ok, I'll pull this because it's currently a silent regression. Adding the opCmp -> opEquals rule would prevent the breakage completely. |
|
Auto-merge toggled on |
|
Thanks! |
I agree, and we can later remove the restriction for cases where we can generate the appropriate |
Issue 13074 - Old opCmp requirement for AA keys should be detected
Issue 13074 - Old opCmp requirement for AA keys should be detected
|
@9rnsr Just realized, classes also silently compile. Sorry for not noticing earlier! The case is a bit trickier, with runtime dispatch of the methods. The bug report: https://issues.dlang.org/show_bug.cgi?id=13114 |
https://issues.dlang.org/show_bug.cgi?id=13074