Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash instances derived from Hash[Long] (e.g. Hash[List[Long]]) do not pass laws #2878

Closed
joroKr21 opened this issue Jun 4, 2019 · 4 comments

Comments

@joroKr21
Copy link
Member

joroKr21 commented Jun 4, 2019

The failing law is sameAsUniversalHash:

List(-1L).hashCode // -1698283412
Hash[List[Long]].hash(List(-1L)) // 1918838495
-1L.hashCode // 0
-1L.## // -1

Took me a while to realize this while working on Hash derivation over at kittens.

To be honest I'm not sure what's the point of that law.
Why would Hash exist if it's bound to be the same as hashCode?

@ctongfei
Copy link
Contributor

ctongfei commented Jun 10, 2019

I'm the author who wrote that law.

The law sameAsUniversalHash is supposed to guarantee for common types and case classes, the derived Hash instances behave in the same way as .hashCode (Not for user-defined Hash instance s).

The standard library's .hashCode implementation for Lists can be found at https://github.com/scala/scala/blob/2.12.x/src/library/scala/util/hashing/MurmurHash3.scala#L110. Note that they use .## instead of .hashCode. In the original PR for Hash (#1712), the community decided to use .hashCode instead of .## for performance reasons. So I would say that this behavior is expected.

So to move forward I think that sameAsUniversalHash should only be used internally to test Hash -- it should not be a requirement for any future Hash instances.

@joroKr21
Copy link
Member Author

Makes sense to me 👍

@ctongfei
Copy link
Contributor

@joroKr21 So for your work in kittens, you should just test the hashCompatibility property.

@joroKr21
Copy link
Member Author

joroKr21 commented Oct 24, 2022

This should be addressed by #4319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants