From fb01dd28a5fbabdae6cb5f23c8493db9a377fda3 Mon Sep 17 00:00:00 2001 From: Nathan Zimmerberg <39104088+nhz2@users.noreply.github.com> Date: Sat, 21 Oct 2023 16:20:35 -0400 Subject: [PATCH] Throw OverflowError on typemin(Int)//(-1) (#51085) Fixes #32443 Currently `typemin(Int)//(-1) == typemin(Int)//(1)`, ignoring an overflow. As noted by @JeffreySarnoff in [#32443](https://github.com/JuliaLang/julia/issues/32443#issuecomment-506816722) This should throw an error instead of silently overflowing. To fix this I am using `checked_neg` instead of `-` in the Rational constructor. With this PR `(-one(T))//typemin(T)` will now also throw an `OverflowError` instead of an `ArgumentError` --- base/rational.jl | 6 ++---- test/rational.jl | 11 ++++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/base/rational.jl b/base/rational.jl index cfa1e257bc539..4671478f0b0d3 100644 --- a/base/rational.jl +++ b/base/rational.jl @@ -17,12 +17,10 @@ end unsafe_rational(num::T, den::T) where {T<:Integer} = unsafe_rational(T, num, den) unsafe_rational(num::Integer, den::Integer) = unsafe_rational(promote(num, den)...) -@noinline __throw_rational_argerror_typemin(T) = throw(ArgumentError("invalid rational: denominator can't be typemin($T)")) function checked_den(::Type{T}, num::T, den::T) where T<:Integer if signbit(den) - den = -den - signbit(den) && __throw_rational_argerror_typemin(typeof(den)) - num = -num + den = checked_neg(den) + num = checked_neg(num) end return unsafe_rational(T, num, den) end diff --git a/test/rational.jl b/test/rational.jl index 4b29618bd15e0..9efbc931650c4 100644 --- a/test/rational.jl +++ b/test/rational.jl @@ -28,8 +28,8 @@ using Test @test (1//typemax(Int)) / (1//typemax(Int)) == 1 @test_throws OverflowError (1//2)^63 @test inv((1+typemin(Int))//typemax(Int)) == -1 - @test_throws ArgumentError inv(typemin(Int)//typemax(Int)) - @test_throws ArgumentError Rational(0x1, typemin(Int32)) + @test_throws OverflowError inv(typemin(Int)//typemax(Int)) + @test_throws OverflowError Rational(0x1, typemin(Int32)) @test @inferred(rationalize(Int, 3.0, 0.0)) === 3//1 @test @inferred(rationalize(Int, 3.0, 0)) === 3//1 @@ -43,10 +43,15 @@ using Test # issue 26823 @test_throws InexactError rationalize(Int, NaN) # issue 32569 - @test_throws ArgumentError 1 // typemin(Int) + @test_throws OverflowError 1 // typemin(Int) @test_throws ArgumentError 0 // 0 @test -2 // typemin(Int) == -1 // (typemin(Int) >> 1) @test 2 // typemin(Int) == 1 // (typemin(Int) >> 1) + # issue 32443 + @test Int8(-128)//Int8(1) == -128 + @test_throws OverflowError Int8(-128)//Int8(-1) + @test_throws OverflowError Int8(-1)//Int8(-128) + @test Int8(-128)//Int8(-2) == 64 @test_throws InexactError Rational(UInt(1), typemin(Int32)) @test iszero(Rational{Int}(UInt(0), 1))