Remove "Chain#hashCode is consistent with List#hashCode"

[rossabaker]

It's no longer true as of Scala 2.13.16, and was probably overspecified in the first place.

Fixes #4716.

[armanbilge]

was probably overspecified in the first place.

Agree with this.

It's no longer true as of Scala 2.13.16

Isn't that because they fixed a hashing bug, that we may want to fix too?

• MurmurHash3 produces the same hash for lists of repeated values, regardless of list length scala/bug#12826

Edit (aha!):

• Backport hash improvements from Scala 2.13.16's MurmurHash3 #4719

[armanbilge]

FWIW, the over-specification did help us keep in sync with upstream changes, so maybe that's worth something, but IMO, not enough.

[rossabaker]

That's a fair point. I don't care about equality, but being alerted to the improved hash algorithm was nice. I'd also be willing to leave it with a comment so it's less of a chase for the next maintainer who hits this.

[satorg]

Honestly, I'm not sure I'm following how come it was assumed that

Chain#hashCode is consistent with List#hashCode

in the first place? What was the justification for that? I mean, Chain and List are different types, they don't have to have same hash codes even if they contain same elements, or do they?

[rossabaker]

They don't need the same hash codes unless they're equal, which is not a goal, shouldn't be a goal, and would be quite a challenge to do lawfully even if it were a goal.

[johnynek]

I think the original idea is that it is easy to mess up a hash and make it too weak. Also it would be easy to make a structural hash to cares about the construction order which we don't want. In this case we had a test that asserts the hashCode is the same as toList.hashCode which proves it can't be a terrible hash (it can't have a few of the failure modes).

So it's not a terrible idea but the weakness is obvious now: we don't control the thing we are asserting it matches and we don't actually need it to match just a sufficient condition for it to probably be a good hash.

[satorg]

@johnynek ,

Also it would be easy to make a structural hash to cares about the construction order which we don't want.

Oh, I see now, that is a valid point. But in order to check it properly, I guess the test should look somewhat like this:

forAll { (x: Chain[Int]) =>
  assert(x.hashCode === Chain.fromIterableOnce(x.iterator).hashCode)
}

– that way we make sure Chain#hashCode doesn't depend on its structure, but also it doesn't depend on the List implementation either. Wdyt?