Skip to content

Commit

Permalink
fix off-by-1 in isqrt. closes #4884
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Dec 22, 2013
1 parent d0d4514 commit 2e9cccb
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
4 changes: 3 additions & 1 deletion base/intfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,10 @@ end
isqrt(x::Integer) = oftype(x, trunc(sqrt(x)))

function isqrt(x::Union(Int64,Uint64,Int128,Uint128))
x==0 && return x
s = oftype(x, trunc(sqrt(x)))
# fix with a Newton iteration, since conversion to float discards
# too many bits.
(s + div(x,s)) >> 1
s = (s + div(x,s)) >> 1
s*s > x ? s-1 : s
end
12 changes: 12 additions & 0 deletions test/math.jl
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,15 @@ end
# isqrt (issue #4884)
@test isqrt(9223372030926249000) == 3037000498
@test isqrt(typemax(Int128)) == int128("13043817825332782212")
@test isqrt(int128(typemax(Int64))^2-1) == 9223372036854775806
@test isqrt(0) == 0
for i = 1:1000
n = rand(Uint128)
s = isqrt(n)
@test s*s <= n
@test (s+1)*(s+1) > n
n = rand(Uint64)
s = isqrt(n)
@test s*s <= n
@test (s+1)*(s+1) > n
end

2 comments on commit 2e9cccb

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

I really doubt this works for x close to the typemax.

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

So far I can't find such a case, but feel free.

Please sign in to comment.