Skip to content

Commit

Permalink
Fix integer overflow in isapprox (#50730)
Browse files Browse the repository at this point in the history
Ensure that `isapprox` gives correct results when comparing an integer
with another integer or with a float. For comparison between integers,
the fix only works when keeping default values for `rtol` and `norm`,
and with `atol < 1`.

It is not possible to handle the (atypical) case where `norm !== abs`,
but that's OK since the user is responsible for providing a safe
function.

It would be possible to handle the case where `rtol > 0` or `atol >= 1`,
but with complex code which would check for overflow and handle all
possible corner cases; it would work only for types defined in Base and
would not be extensible by packages. So I'm not sure that's worth it. At
least with PR fixes the most common case.

Fixes #50380.
  • Loading branch information
nalimilan authored Aug 14, 2023
1 parent ad0712f commit 5f03a18
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
15 changes: 14 additions & 1 deletion base/floatfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,20 @@ true
function isapprox(x::Number, y::Number;
atol::Real=0, rtol::Real=rtoldefault(x,y,atol),
nans::Bool=false, norm::Function=abs)
x == y || (isfinite(x) && isfinite(y) && norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))) || (nans && isnan(x) && isnan(y))
x′, y′ = promote(x, y) # to avoid integer overflow
x == y ||
(isfinite(x) && isfinite(y) && norm(x-y) <= max(atol, rtol*max(norm(x′), norm(y′)))) ||
(nans && isnan(x) && isnan(y))
end

function isapprox(x::Integer, y::Integer;
atol::Real=0, rtol::Real=rtoldefault(x,y,atol),
nans::Bool=false, norm::Function=abs)
if norm === abs && atol < 1 && rtol == 0
return x == y
else
return norm(x - y) <= max(atol, rtol*max(norm(x), norm(y)))
end
end

"""
Expand Down
44 changes: 44 additions & 0 deletions test/floatfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,47 @@ end
struct CustomNumber <: Number end
@test !isnan(CustomNumber())
end

@testset "isapprox and integer overflow" begin
for T in (Int8, Int16, Int32)
T === Int && continue
@test !isapprox(typemin(T), T(0))
@test !isapprox(typemin(T), unsigned(T)(0))
@test !isapprox(typemin(T), 0)
@test !isapprox(typemin(T), T(0), atol=0.99)
@test !isapprox(typemin(T), unsigned(T)(0), atol=0.99)
@test !isapprox(typemin(T), 0, atol=0.99)
@test_broken !isapprox(typemin(T), T(0), atol=1)
@test_broken !isapprox(typemin(T), unsigned(T)(0), atol=1)
@test !isapprox(typemin(T), 0, atol=1)

@test !isapprox(typemin(T)+T(10), T(10))
@test !isapprox(typemin(T)+T(10), unsigned(T)(10))
@test !isapprox(typemin(T)+T(10), 10)
@test !isapprox(typemin(T)+T(10), T(10), atol=0.99)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=0.99)
@test !isapprox(typemin(T)+T(10), 10, atol=0.99)
@test_broken !isapprox(typemin(T)+T(10), T(10), atol=1)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=1)
@test !isapprox(typemin(T)+T(10), 10, atol=1)

@test isapprox(typemin(T), 0.0, rtol=1)
end
for T in (Int, Int64, Int128)
@test !isapprox(typemin(T), T(0))
@test !isapprox(typemin(T), unsigned(T)(0))
@test !isapprox(typemin(T), T(0), atol=0.99)
@test !isapprox(typemin(T), unsigned(T)(0), atol=0.99)
@test_broken !isapprox(typemin(T), T(0), atol=1)
@test_broken !isapprox(typemin(T), unsigned(T)(0), atol=1)

@test !isapprox(typemin(T)+T(10), T(10))
@test !isapprox(typemin(T)+T(10), unsigned(T)(10))
@test !isapprox(typemin(T)+T(10), T(10), atol=0.99)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=0.99)
@test_broken !isapprox(typemin(T)+T(10), T(10), atol=1)
@test !isapprox(typemin(T)+T(10), unsigned(T)(10), atol=1)

@test isapprox(typemin(T), 0.0, rtol=1)
end
end

0 comments on commit 5f03a18

Please sign in to comment.