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.

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

Check for overflow in gcd algorithm #15228

Merged
merged 1 commit into from
Feb 27, 2016
Merged

Check for overflow in gcd algorithm #15228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 8 additions & 5 deletions base/intfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function gcd{T<:Integer}(a::T, b::T)
b = rem(a, b)
a = t
end
abs(a)
checked_abs(a)
end

# binary GCD (aka Stein's) algorithm
Expand All @@ -19,20 +19,23 @@ function gcd{T<:Union{Int64,UInt64,Int128,UInt128}}(a::T, b::T)
za = trailing_zeros(a)
zb = trailing_zeros(b)
k = min(za, zb)
u = abs(a >> za)
v = abs(b >> zb)
u = unsigned(abs(a >> za))
v = unsigned(abs(b >> zb))
Copy link
Contributor

Choose a reason for hiding this comment

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

would checked_abs give the same exception here as the general T<:Integer version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checked_abs is too strict here; if a == b == typemin(Int), then unsigned(abs(...)) is well-defined, but checked_abs would fail.

I just see that this code right-shifts before calling abs, and given that typemin(Int) has many trailing zeros, the code is probably fine.

No, they give different errors. checked_abs returns OverflowError, whereas type conversions return InexactError. (I find that weird, but that's how things are.) The generic gcd algorithm also might return DivideError instead.

while u != v
if u > v
u, v = v, u
end
v -= u
v >>= trailing_zeros(v)
end
u << k
r = u << k
# T(r) would throw InexactError; we want OverflowError instead
r > typemax(T) && throw(OverflowError())
r % T
end

# explicit a==0 test is to handle case of lcm(0,0) correctly
lcm{T<:Integer}(a::T, b::T) = a == 0 ? a : abs(a * div(b, gcd(b,a)))
lcm{T<:Integer}(a::T, b::T) = a == 0 ? a : checked_abs(a * div(b, gcd(b,a)))

gcd(a::Integer) = a
lcm(a::Integer) = a
Expand Down
51 changes: 30 additions & 21 deletions test/intfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,35 @@
# This file is a part of Julia. License is MIT: http://julialang.org/license

@test gcd(3, 5) == 1
@test gcd(3, 15) == 3
@test gcd(0, 15) == 15
@test gcd(3, -15) == 3
@test gcd(-3, -15) == 3
@test gcd(0, 0) == 0

@test gcd(2, 4, 6) == 2

@test typeof(gcd(Int32(3), Int32(15))) == Int32

@test lcm(2, 3) == 6
@test lcm(4, 6) == 12
@test lcm(3, 0) == 0
@test lcm(0, 0) == 0
@test lcm(4, -6) == 12
@test lcm(-4, -6) == 12

@test lcm(2, 4, 6) == 12

@test typeof(lcm(Int32(2), Int32(3))) == Int32
# Int32 and Int64 take different code paths -- test both
for T in (Int32, Int64)
@test gcd(T(3), T(5)) === T(1)
@test gcd(T(3), T(15)) === T(3)
@test gcd(T(0), T(15)) === T(15)
@test gcd(T(3), T(-15)) === T(3)
@test gcd(T(-3), T(-15)) === T(3)
@test gcd(T(0), T(0)) === T(0)

@test gcd(T(2), T(4), T(6)) === T(2)

@test gcd(typemax(T), T(1)) === T(1)
@test gcd(-typemax(T), T(1)) === T(1)
@test gcd(typemin(T), T(1)) === T(1)
@test_throws OverflowError gcd(typemin(T), typemin(T))

@test lcm(T(2), T(3)) === T(6)
@test lcm(T(4), T(6)) === T(12)
@test lcm(T(3), T(0)) === T(0)
@test lcm(T(0), T(0)) === T(0)
@test lcm(T(4), T(-6)) === T(12)
@test lcm(T(-4), T(-6)) === T(12)

@test lcm(T(2), T(4), T(6)) === T(12)

@test lcm(typemax(T), T(1)) === typemax(T)
@test lcm(-typemax(T), T(1)) === typemax(T)
@test_throws OverflowError lcm(typemin(T), T(1))
@test_throws OverflowError lcm(typemin(T), typemin(T))
end

@test gcdx(5, 12) == (1, 5, -2)
@test gcdx(5, -12) == (1, 5, 2)
Expand Down