Skip to content
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.

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

check overflow in hex2num (fix #22031) #22039

Closed
wants to merge 4 commits into from
Closed

check overflow in hex2num (fix #22031) #22039

wants to merge 4 commits into from

Conversation

rfourquet
Copy link
Member

@rfourquet rfourquet commented May 23, 2017

This also creates hex2num(::Type, str) to get a type-stable function, and fix (what I think to be) a bug, where num2hex(-1) == "-0000000000000001" instead of "ffffffffffffffff", i.e. num2hex is supposed to give the bit-pattern in hex form (similar to bits).

Edit: this is independant of the question on whether hex2num should be moved to parse...

@rfourquet rfourquet added the kind:bugfix This change fixes an existing bug label May 23, 2017
@rfourquet
Copy link
Member Author

I need here a function, say f, which takes a bits type and returns the corresponding Unsigned type with the same bit-width, so that e.g. f(Int) = UInt and f(Float16) = UInt16, to be used together with reinterpret. I first thougth of unsigned, but I feel unsigned(T::Type) should return the return-type of unsigned(x::T), i.e. unsigned(Float16(1)) == UInt(1) so unsigned(Float16) == UInt. I now chose reinterpret(Unsigned, T) == f(T) (e.g. reinterpret(Unsigned, Float16) == UInt16)) which does what is needed, but I'm not conviced... Any ideas for a better name for f?

base/intfuncs.jl Outdated

julia> num2hex(2.2)
"400199999999999a"

Copy link
Contributor

Choose a reason for hiding this comment

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

excess blank line here

I think "a hexadecimal" is more common than "an hexadecimal"

base/intfuncs.jl Outdated

An hexadecimal string of the binary representation of a number.
See also the [`bits`](@ref) function, which is similar but gives
a binary string, and [`hex2num`] which does the opposite conversion.
Copy link
Member

Choose a reason for hiding this comment

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

[`hex2num`](@ref) maybe.

unsigned without checking for negative values.
See also [`signed`](@ref).

unsigned(T::Type) -> UnsignedType
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be a separate docstring for the separate signature?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know, but will do.

base/intfuncs.jl Outdated
# this construction is not available when put in int.jl and running in Core
for (S,U,F) in zip(BitSigned_types, BitUnsigned_types,
(nothing, Float16, Float32, Float64, nothing))
@eval begin
Copy link
Contributor

Choose a reason for hiding this comment

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

the body of this for loop is indented one level too deep

@rfourquet
Copy link
Member Author

Thanks @tkelman and @fredrikekre, comments addressed.

@fredrikekre
Copy link
Member

Given #22088, is this still relevant?

@JeffBezanson
Copy link
Sponsor Member

It looks like this would still be useful if the hex2num-specific parts were removed.

@JeffBezanson
Copy link
Sponsor Member

Replaced by #22203

@fredrikekre fredrikekre deleted the rf/hex2num branch August 11, 2017 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants