Hash[Map[K, V]] has unnecessary constraint

[travisbrown]

Similar to #3038. Now we have:

  implicit def catsKernelStdHashForMap[K: Hash, V: Hash]: Hash[Map[K, V]] =
    new MapHash[K, V]

But the K: Hash isn't used in the implementation.

[vasiliybondarenko]

I'll create PR if nobody minds.

[travisbrown]

@vasiliybondarenko Sounds good to me, although the problem is that I don't think it's possible to fix it without breaking bincompat, which means the PR will be have to be parked for a long time.

[kailuowang]

as soon as #3051 gets merged we can fix this right without breaking BC: deprecate the current one and remove the implicit keyword and add a new one without the constraint.

[travisbrown]

@kailuowang Oh, I guess that's right—the erasure will be different so the names won't collide.

[vasiliybondarenko]

@kailuowang could you take a look at my PR?

[ctongfei]

I'm the author of that function.

Hash[V] is actually used in the implementation: to hash a map, we need to have the hash code for each (K, V) pair, which, in turn, requires an implicit Hash[V].

See the implementation here: https://github.com/typelevel/cats/blob/master/kernel/src/main/scala/cats/kernel/instances/MapInstances.scala#L33

[vasiliybondarenko]

@ctongfei that's right. But we still don't need Hash[K], as key uses Scala's hashCode

[ctongfei]

@vasiliybondarenko That is right. Scala's Map uses universal hash code, so Hash[K] is not needed. Thanks for the clarification!