Skip to content

Commit

Permalink
Throw OverflowError on copysign(typemin(Int)//1, 1) (#53395)
Browse files Browse the repository at this point in the history
The default `copysign(x::Real, y::Real)` in `number.jl` works, so the
incorrect method in `rational.jl` isn't needed.

Here is a benchmark of the new version.
```julia
using BenchmarkTools
function foo!(c,a,b) 
    c .= copysign.(a, b)
    nothing
end
N = 1000
@Btime foo!(c,a,b) setup=(c=zeros(Rational{Int},N); a=rand(Int,N).//rand(Int,N); b=fill(-1,N))
```
On master: 406.215 ns (0 allocations: 0 bytes)
On this PR: 869.327 ns (0 allocations: 0 bytes)
  • Loading branch information
nhz2 committed Feb 21, 2024
1 parent 3742d33 commit 9d896dc
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
2 changes: 0 additions & 2 deletions base/rational.jl
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,6 @@ denominator(x::Rational) = x.den

sign(x::Rational) = oftype(x, sign(x.num))
signbit(x::Rational) = signbit(x.num)
copysign(x::Rational, y::Real) = unsafe_rational(copysign(x.num, y), x.den)
copysign(x::Rational, y::Rational) = unsafe_rational(copysign(x.num, y.num), x.den)

abs(x::Rational) = unsafe_rational(checked_abs(x.num), x.den)

Expand Down
3 changes: 3 additions & 0 deletions test/numbers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,9 @@ end
@test copysign(big(-1), 0x02) == 1
@test copysign(big(-1.0), 0x02) == 1.0
@test copysign(-1//2, 0x01) == 1//2

# Verify overflow is checked with rational
@test_throws OverflowError copysign(typemin(Int)//1, 1)
end

@testset "isnan/isinf/isfinite" begin
Expand Down

0 comments on commit 9d896dc

Please sign in to comment.