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

WIP: checked integer conversions #8420

Merged
merged 17 commits into from
Sep 29, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
effff39
check integer truncation (#5413) and make more operators follow T+T =…
JeffBezanson Sep 17, 2014
1d5050b
get numbers test passing with checked integer conversions and type-st…
JeffBezanson Sep 17, 2014
8495944
make a couple things safe for checked signed<->unsigned conversion
JeffBezanson Sep 17, 2014
e1690fc
check signed<->unsigned conversions
JeffBezanson Sep 18, 2014
c67837c
get dates tests passing with checked signed<->unsigned conversion
JeffBezanson Sep 19, 2014
06241fe
Merge branch 'master' of github.com:JuliaLang/julia into jb/checked_i…
JeffBezanson Sep 19, 2014
ec607da
get a few more things working with checked integer conversions
JeffBezanson Sep 19, 2014
50f1d10
Merge branch 'master' of github.com:JuliaLang/julia into jb/checked_i…
JeffBezanson Sep 25, 2014
16c2330
make signed() and unsigned() unchecked. check only in convert()
JeffBezanson Sep 25, 2014
9f0ef0d
add a bit of widening to reductions, to make up for the new
JeffBezanson Sep 25, 2014
b3e30f9
Merge branch 'master' of github.com:JuliaLang/julia into jb/checked_i…
JeffBezanson Sep 26, 2014
3336fbc
fix more cases of widening in reductions
JeffBezanson Sep 26, 2014
4d1c138
fix more issues with integer conversion in random number generation
JeffBezanson Sep 26, 2014
000f41e
Merge branch 'master' of github.com:JuliaLang/julia into jb/checked_i…
JeffBezanson Sep 26, 2014
8c39ad2
Merge branch 'master' of github.com:JuliaLang/julia into jb/checked_i…
JeffBezanson Sep 26, 2014
8321a9e
doc and test updates for checked integer conversions
JeffBezanson Sep 28, 2014
53b6dee
Merge branch 'master' of github.com:JuliaLang/julia into jb/checked_i…
JeffBezanson Sep 28, 2014
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
Prev Previous commit
Next Next commit
get a few more things working with checked integer conversions
  • Loading branch information
JeffBezanson committed Sep 19, 2014
commit ec607da4d3b25663487dd8c5e773d661b6bb6054
2 changes: 1 addition & 1 deletion base/hashing2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ end
function hash(s::IntSet, h::Uint)
h += uint(0x88989f1fc7dea67d)
h += hash(s.fill1s)
filln = s.fill1s ? uint32(-1) : uint32(0)
filln = s.fill1s ? ~zero(eltype(s.bits)) : zero(eltype(s.bits))
for x in s.bits
if x != filln
h = hash(x, h)
Expand Down
3 changes: 3 additions & 0 deletions base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ abs(x::Signed) = flipsign(x,x)
~(n::Integer) = -n-1

asunsigned(x::Integer) = reinterpret(typeof(unsigned(zero(x))), x)
asunsigned(x::Bool) = unsigned(x)
asunsigned(x) = unsigned(x)
assigned(x::Integer) = reinterpret(typeof(signed(zero(x))), x)
assigned(x) = signed(x)
Copy link
Member

Choose a reason for hiding this comment

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

since assigned is a word, this seems like a poor method name

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right; I realized that afterwards. It would be better to get rid of both of these, but I don't have a better idea so far.

Copy link
Member

Choose a reason for hiding this comment

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

i didn't have a suggestion either, but now i'm leaning towards coerce(Unsigned,x) being a good option

Copy link
Member

Choose a reason for hiding this comment

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

Why not convert(Unsigned, x)?

Copy link
Member

Choose a reason for hiding this comment

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

convert would throw an error if x < 0

Expand Down Expand Up @@ -210,6 +211,7 @@ convert(::Type{Signed}, x::Uint128) = convert(Int128,x)
convert(::Type{Signed}, x::Float32) = convert(Int,x)
convert(::Type{Signed}, x::Float64) = convert(Int,x)
convert(::Type{Signed}, x::Char) = convert(Int,x)
convert(::Type{Signed}, x::Bool) = convert(Int,x)

convert(::Type{Unsigned}, x::Int8 ) = convert(Uint8,x)
convert(::Type{Unsigned}, x::Int16 ) = convert(Uint16,x)
Expand All @@ -219,6 +221,7 @@ convert(::Type{Unsigned}, x::Int128 ) = convert(Uint128,x)
convert(::Type{Unsigned}, x::Float32) = convert(Uint,x)
convert(::Type{Unsigned}, x::Float64) = convert(Uint,x)
convert(::Type{Unsigned}, x::Char) = convert(Uint,x)
convert(::Type{Unsigned}, x::Bool) = convert(Uint,x)

convert(::Type{Integer}, x::Float32) = convert(Int,x)
convert(::Type{Integer}, x::Float64) = convert(Int,x)
Expand Down
5 changes: 3 additions & 2 deletions base/random.jl
Original file line number Diff line number Diff line change
Expand Up @@ -197,15 +197,16 @@ function rand{T<:Union(Uint64, Int64)}(g::RandIntGen{T,Uint64})
x = rand(Uint64)
end
end
return convert(T, g.a + rem_knuth(x, g.k))
return reinterpret(T, reinterpret(Uint64, g.a) + rem_knuth(x, g.k))
end

function rand{T<:Integer, U<:Unsigned}(g::RandIntGen{T,U})
x = rand(U)
while x > g.u
x = rand(U)
end
convert(T, g.a + rem_knuth(x, g.k))
# TODO: fix for when T is smaller than U
reinterpret(T, Base.asunsigned(g.a) + rem_knuth(x, g.k))
end

rand{T<:Union(Signed,Unsigned,Bool,Char)}(r::UnitRange{T}) = rand(RandIntGen(r))
Expand Down
4 changes: 2 additions & 2 deletions base/utf8proc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import Base: show, showcompact, ==, string, symbol, isless

# also exported by Base:
export normalize_string, is_valid_char, is_assigned_char,
islower, isupper, isalpha, isdigit, isnumber, isalnum,
islower, isupper, isalpha, isdigit, isnumber, isalnum,
iscntrl, ispunct, isspace, isprint, isgraph, isblank

# whether codepoints are valid Unicode
is_valid_char(c) = bool(ccall(:utf8proc_codepoint_valid, Cchar, (Int32,), c))
is_valid_char(c) = (0 <= c <= 0x110000) && bool(ccall(:utf8proc_codepoint_valid, Cuchar, (Int32,), c))

# utf8 category constants
const UTF8PROC_CATEGORY_LU = 1
Expand Down
2 changes: 1 addition & 1 deletion src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ static Value *generic_box(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ctx)
!(vxt->isPointerTy() && llvmt->getPrimitiveSizeInBits() == sizeof(void*)*8) &&
!(llvmt->isPointerTy() && vxt->getPrimitiveSizeInBits() == sizeof(void*)*8)) {
emit_error("box: argument is of incorrect size", ctx);
return vx;
return UndefValue::get(llvmt);
}
// PtrToInt and IntToPtr ignore size differences
if (vxt->isPointerTy() && !llvmt->isPointerTy()) {
Expand Down
19 changes: 17 additions & 2 deletions test/hashing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,24 @@ vals = [
typemax(Int64),
]

function coerce(T::Type, x)
if T<:Rational
Copy link
Member

Choose a reason for hiding this comment

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

coerce{T<:Rational}(::Type{T}, x) = convert(T, coerce(typeof(num(zero(T))), x)) ?

return convert(T, coerce(typeof(num(zero(T))), x))
end
if !(T<:Integer) || T===Bool
convert(T, x)
elseif sizeof(T) < sizeof(x)
itrunc(T, x)
elseif sizeof(T) == sizeof(x)
reinterpret(T, x)
else
convert(T, x)
end
end
Copy link
Member

Choose a reason for hiding this comment

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

for completeness:

coerce{T<:Rational}(::Type{T}, x) = convert(T, coerce(typeof(num(zero(T))), x))
function coerce{T<:Integer}(::Type{T}, x)
    if sizeof(T) < sizeof(x)
        itrunc(T, x)
    else if sizeof(T) == sizeof(x)
        reinterpret(T, x)
    else
        convert(T, x)
    end
end
coerce(::Type{Bool}, x) = convert(Bool, x)
coerce{T}(::Type{T}, x) = convert(T, x)

i think the above would generate better code, since it lets the compiler eliminate the tests and inline much better

Copy link
Member

Choose a reason for hiding this comment

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

oh, i see that this isn't in base, so it isn't as important. it seems like a pretty good name for assigned/asunsigned (e.g. coerce(Signed, x))

it's missing a test for isleaftype(T) before sizeof(T) which is needed if this does end up in base, and a few other possible definitions:

coerce(::Type{Unsigned}, x) = coerce(typeof(unsigned(zero(x))), x)
coerce(::Type{Signed}, x) = coerce(typeof(signed(zero(x))), x)
coerce{T}(::Type{T}, x::T) = x


for T=types, S=types, x=vals
a = convert(T,x)
b = convert(S,x)
a = coerce(T,x)
b = coerce(S,x)
if (isa(a,Char) && !is_valid_char(a)) || (isa(b,Char) && !is_valid_char(b))
continue
end
Expand Down
14 changes: 7 additions & 7 deletions test/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@

# sum

@test sum(Int8[]) === 0
@test sum(Int[]) === 0
@test sum(Int8[]) === int8(0)
@test sum(Int[]) === int(0)
@test sum(Float64[]) === 0.0

@test sum(int8(3)) === int8(3)
@test sum(3) === 3
@test sum(3.0) === 3.0

@test sum([int8(3)]) === 3
@test sum([int8(3)]) === int8(3)
@test sum([3]) === 3
@test sum([3.0]) === 3.0

Expand All @@ -49,14 +49,14 @@ fz = float(z)
a = randn(32) # need >16 elements to trigger BLAS code path
b = complex(randn(32), randn(32))
@test sumabs(Float64[]) === 0.0
@test sumabs([int8(-2)]) === 2
@test sumabs([int8(-2)]) === int8(2)
@test sumabs(z) === 14
@test sumabs(fz) === 14.0
@test_approx_eq sumabs(a) sum(abs(a))
@test_approx_eq sumabs(b) sum(abs(b))

@test sumabs2(Float64[]) === 0.0
@test sumabs2([int8(-2)]) === 4
@test sumabs2([int8(-2)]) === int8(4)
@test sumabs2(z) === 54
@test sumabs2(fz) === 54.0
@test_approx_eq sumabs2(a) sum(abs2(a))
Expand Down Expand Up @@ -105,8 +105,8 @@ prod(fz) === 120.0
prod2(itr) = invoke(prod, (Any,), itr)
@test prod(Int[]) === prod2(Int[]) === 1
@test prod(Int[7]) === prod2(Int[7]) === 7
@test typeof(prod(Int8[])) == typeof(prod(Int8[1])) == typeof(prod(Int8[1, 7])) == Int
@test typeof(prod2(Int8[])) == typeof(prod2(Int8[1])) == typeof(prod2(Int8[1 7])) == Int
@test typeof(prod(Int8[])) == typeof(prod(Int8[1])) == typeof(prod(Int8[1, 7])) == Int8
@test typeof(prod2(Int8[])) == typeof(prod2(Int8[1])) == typeof(prod2(Int8[1 7])) == Int8

# maximum & minimum & extrema

Expand Down