Skip to content

Commit

Permalink
Fixup hash(::Real) broken by #49996 (#50067)
Browse files Browse the repository at this point in the history
* fixup for 49996 and add test from 50065

---------

Co-authored-by: Lilith Hafner <[email protected]>
  • Loading branch information
LilithHafner and Lilith Hafner authored Jun 6, 2023
1 parent 1bb3d26 commit 53bcb39
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
13 changes: 7 additions & 6 deletions base/float.jl
Original file line number Diff line number Diff line change
Expand Up @@ -675,27 +675,28 @@ function hash(x::Real, h::UInt)
den = -den
end
num_z = trailing_zeros(num)
num >>= num_z
den_z = trailing_zeros(den)
den >>= den_z
pow += num_z - den_z

# handle values representable as Int64, UInt64, Float64
if den == 1
left = top_set_bit(abs(num)) - den_z
right = pow
left = top_set_bit(abs(num)) + pow
right = pow + den_z
if -1074 <= right
if 0 <= right
left <= 63 && return hash(Int64(num) << Int(pow-num_z), h)
left <= 64 && !signbit(num) && return hash(UInt64(num) << Int(pow-num_z), h)
left <= 63 && return hash(Int64(num) << Int(pow), h)
left <= 64 && !signbit(num) && return hash(UInt64(num) << Int(pow), h)
end # typemin(Int64) handled by Float64 case
left <= 1024 && left - right <= 53 && return hash(ldexp(Float64(num), pow-num_z), h)
left <= 1024 && left - right <= 53 && return hash(ldexp(Float64(num), pow), h)
end
end

# handle generic rational values
h = hash_integer(den, h)
h = hash_integer(pow, h)
h = hash_integer(num >> num_z, h)
h = hash_integer(num, h)
return h
end

Expand Down
11 changes: 8 additions & 3 deletions test/hashing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@ types = Any[
Bool,
Int8, UInt8, Int16, UInt16, Int32, UInt32, Int64, UInt64, Float32, Float64,
Rational{Int8}, Rational{UInt8}, Rational{Int16}, Rational{UInt16},
Rational{Int32}, Rational{UInt32}, Rational{Int64}, Rational{UInt64}
Rational{Int32}, Rational{UInt32}, Rational{Int64}, Rational{UInt64},
BigFloat, #BigInt, # TODO: BigInt hashing is broken on 32-bit systems
]
if Int === Int64
push!(types, BigInt)
else
@test_broken hash(12345678901234) == hash(big(12345678901234))
end
vals = vcat(
typemin(Int64),
-Int64(maxintfloat(Float64)) .+ Int64[-4:1;],
Expand Down Expand Up @@ -51,8 +57,7 @@ let collides = 0
collides += eq
end
end
# each pair of types has one collision for these values
@test collides <= (length(types) - 1)^2
@test collides <= 452
end
@test hash(0.0) != hash(-0.0)

Expand Down

7 comments on commit 53bcb39

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@vtjnash

This comment was marked as off-topic.

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 53bcb39 Jun 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible issues were detected.
A full report can be found here.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a 5-10% slowdown across the board, but no commits look like they could be impactful, and I thought #49901 is off by default (@topolarity)? So apparently just a noisy day?

Please sign in to comment.