Skip to content

Fixup hash(::Real) broken by #49996 #50067

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 6, 2023
Merged

Fixup hash(::Real) broken by #49996 #50067

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f276e1a
fixup for 49996 and add test from 50065
Jun 5, 2023
bee9e21
generalize test additions, remove misleading comment, and adjust coll…
Jun 5, 2023
a6866d8
debug statement
Jun 5, 2023
e5626f9
remove debug print statement and admit that hashing BigInts is broken…
Jun 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions base/float.jl
View file
Open in desktop
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
3 changes: 3 additions & 0 deletions test/hashing.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,6 @@ struct AUnionParam{T<:Union{Nothing,Float32,Float64}} end
@test Type{AUnionParam{<:Union{Nothing,Float32,Float64}}} === Type{AUnionParam}
@test Type{AUnionParam.body}.hash == 0
@test Type{Base.Broadcast.Broadcasted}.hash != 0

# Issue #50065
@test hash(1.0) == hash(BigFloat(1.0))
Copy link
Member

@oscardssmith oscardssmith Jun 5, 2023

Choose a reason for hiding this comment

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

add 1//1 and big(1) for good measure.

Copy link
Member Author

@LilithHafner LilithHafner Jun 5, 2023

Choose a reason for hiding this comment

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

I think it makes sense to add BigFloat and BigInt to the massive cross-product at the top instead.

Copy link
Member

@oscardssmith oscardssmith Jun 5, 2023

Choose a reason for hiding this comment

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

well I'm sure glad I suggested testing BigInt :)