On Oct 16, 2014, at 3:42 PM, Martin Nowak via dmd-beta <[email protected]> wrote:
> On 10/16/2014 01:32 PM, Steven Schveighoffer via dmd-beta wrote: >> I am too actually. From your earlier messages, Martin, I thought you said we >> would add it as a warning? Has that been done for 2.066.1? I didn't see it >> in my github traffic. >> >> FWIW, I think a warning is sufficient, even if it only lasts a few versions. >> As long as silent behavior changes do not happen. > > Well I made a pull and we eventually agreed that even a warning would do more > harm than good, because one cannot explicitly use the compiler generated > toHash and opEquals. A opCmp that wouldn't comply with default equality also > required a custom toHash, which makes it even more unlikely that we will > break code. I think there is a misunderstanding, the issue is not that opCmp and opEquals are not consistent. The problem is this: 1. As of 2.065, AAs used opCmp and NOT opEquals. 2. Despite the spec's assertion that BOTH opEquals and opCmp should be defined for AA to work properly, the compiler allowed any combination, and let the implementation tell the story. 3. All that was required was to override opCmp and toHash to make AAs work. 4. In 2.066, we no longer use opCmp and use opEquals. 5. Without an error, any code that ONLY defined opCmp (and defined it differently than the default) despite the warnings from the spec will now silently compile, and do the incorrect thing. Basically, without an error, this is an on-purpose regression for code that has worked as long as AAs have been around (D1 included), and a silent one. I don't know why this critical error was removed. Yes, any code that relied on the AA implementation details was broken. But it will be viewed as a regression. The only problem with this change is that at some point prior, an ill-advised requirement for opCmp to be implemented, *even if the default was correct*, was added. This caused people who had valid AA keys with default opCmp to add an opCmp that re-implemented the same thing. Such a thing would automatically trigger the new error! So it is a messy situation. But I think the gravity of having a silent breaking regression on something that worked for about a decade should outweigh the PR problem of forcing people to add opCmp and then forcing them to remove it. I think we owe those developers the courtesy of identifying the problem before they spend lots of time trying to debug this. Now, if we wanted to add a warning, that is OK, at least it's not silent. But there should be some indication, IMO, that the implementation and requirements for AA keys have changed. -Steve _______________________________________________ dmd-beta mailing list [email protected] http://lists.puremagic.com/mailman/listinfo/dmd-beta
