Wow you're fast!

you're assuming that the fix is actually correct :) I am mostly just guessing.

Hmm, yeah. On 32-bit systems the pointer in a BigInt is a UInt32 pointer, so creating a UInt64 pointer and unsafe_loading from it may be broken when x.alloc == 1.

Yeah. I don't think this is workable. Let's eval it out then.

grr. The collision test fails. Any thoughts?

The (length(types) - 1) ^ 2 upper bound in the collision test is cute, but afaict, the comment explaining why it works is wrong (or maybe its too clever for me cc @vtjnash #38031). My unmerged approach when adding BigFloat tests is to replace it with a hardcoded bound, which is also not good.

The best approach would be to adjust BigInt and BigFloat hashing to reduce collisions, but we shouldn't hold these bugfixes for that.

Was this broken only since 1.9? Do we know what is broken in the current code for 32 bits systems?

The following is merely speculation:

Was this broken only since 1.9?

Unlikely

Do we know what is broken in the current code for 32 bits systems?

I believe Int64 gets hashed in one go while BigInt gets hashed one UInt32 chunk at a time, so for numbers in the range typemax(Int32) < x < typemax(Int64), BigInts and Int64s are hashed differently.

The hash range used to fold back on itself for certain types, so it is not a comment about the required guarantee but simply a statement about why it was not zero anymore and why the resulting collisions are still rare enough (and not e.g. clustered)

This has been broken since at least #38031 (it goes further back, but git blame is hard to track then).

This has been broken since at least #38031 (it goes further back, but git blame is hard to track then).

Testing a few old 32 bit arm builds that I have, it worked in v1.4.2 and was broken by v1.5.3.