[Issue 13179] AA key type TagIndex now requires equality rather than comparison (page 4)

https://issues.dlang.org/show_bug.cgi?id=13179 --- Comment #29 from github-bugzilla@puremagic.com --- Commit pushed to 2.066 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/1b244ba48a50fa0cb0961b08cfc3c3cb4bba2f35 Merge pull request #3813 from WalterBright/revert3731 [reg] fix Issue 13179 - AA key type TagIndex now requires equality rather than... --

https://issues.dlang.org/show_bug.cgi?id=13179 --- Comment #30 from github-bugzilla@puremagic.com --- Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/1b244ba48a50fa0cb0961b08cfc3c3cb4bba2f35 Merge pull request #3813 from WalterBright/revert3731 --

September 15, 2014

[Issue 13179] AA key type TagIndex now requires equality rather than comparison

Posted by Steven Schveighoffer

Permalink

Steven Schveighoffer

Permalink

https://issues.dlang.org/show_bug.cgi?id=13179

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--- Comment #31 from Steven Schveighoffer <schveiguy@yahoo.com> ---
You realize this now SILENTLY breaks such code? Anyone who defined opCmp and toHash to be other than the default, left opEquals alone, will have their AA keys compile silently, and then do the completely wrong thing?

This was not a regression. It is an unfortunate step REQUIRED because of the state of D before this change. Before the change, regardless of what the spec said, only opCmp was used. opEquals was not used. If one defined opCmp, but not opEquals, no error was raised, and it worked as expected. Now that we switched to using opEquals instead of opCmp, even though the spec has warned such users not to depend on implementation details such as this, it will break code.

In reverting this change, we have introduced a silent breaking regression.

I'll also note that the equivalent class requirements are still in there. In other words, change TagIndex to a class, and you still get the error.

There was a lot of work and thought put into this before the release, I'm surprised it was killed so quickly. I respectfully ask that it be reinstated. If we all decide that such users should just be screwed for not reading the fine print of the spec, then I guess that's what's been decided. At least, remove the class error as well.

--

https://issues.dlang.org/show_bug.cgi?id=13179 Martin Nowak <code@dawg.eu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |code@dawg.eu --- Comment #32 from Martin Nowak <code@dawg.eu> --- > opCmp was added because of a regression in 2.065.0 and now it's changed again. What are you referring to here? --

https://issues.dlang.org/show_bug.cgi?id=13179 --- Comment #33 from Martin Nowak <code@dawg.eu> --- (In reply to hsteoh from comment #22) > Instead, Jonathan's solution is the best: get rid of the compile error that forces you to declare opEquals just because you declared opCmp. Code that defines opCmp that's inconsistent with opEquals is already broken, and we aren't making things worse. Assuming that the user-defined opCmp is consistent with opEquals, then the AA code change that switched from using typeinfo.compare to typeinfo.equals should still work correctly on existing code. So this sounds like the best way to go. So is this the bottom line of the discussion? And are we OK that this change might silently break code that was previously incorrect but did work? --

https://issues.dlang.org/show_bug.cgi?id=13179 --- Comment #35 from Martin Nowak <code@dawg.eu> --- > See: http://forum.dlang.org/post/lqv16p$1cr0$1@digitalmars.com OK, problem solved. I'd also like to amend that if a struct defines opCmp we'll still use the compiler generated xToHash, i.e. compute the hash over all fields. So it's even more unlikely that anyone successfully used a struct with only opCmp as hash key. --

Forums