From 39ce7a8e67c365288e8ab9cd38ad32cd6733ad14 Mon Sep 17 00:00:00 2001
From: kimikage <kimikage.ceo@gmail.com>
Date: Sat, 28 Dec 2019 11:56:12 +0900
Subject: [PATCH] Modify Fixed constructors

This adds explicit input range checks.
---
 src/fixed.jl  | 28 +++++++++++++++++++++++++---
 test/fixed.jl | 22 +++++++++++++++-------
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/fixed.jl b/src/fixed.jl
index df747140..d185e58c 100644
--- a/src/fixed.jl
+++ b/src/fixed.jl
@@ -48,17 +48,39 @@ fracmask(x::Fixed{T,f}) where {T, f} = ~intmask(x) # Signed
 
 # constructor-style conversions
 function _convert(::Type{F}, x::Fixed{T2,f2}) where {T, T2, f, f2, F <: Fixed{T,f}}
-    y = round(((1<<f)/(1<<f2))*reinterpret(x)) # FIXME: avoid overflow
+    y = round(@exp2(f-f2) * reinterpret(x))
     (typemin(T) <= y) & (y <= typemax(T)) || throw_converterror(F, x)
     reinterpret(F, _unsafe_trunc(T, y))
 end
 
 function _convert(::Type{F}, x::Integer) where {T, f, F <: Fixed{T,f}}
-    reinterpret(F, round(T, convert(widen1(T),x)<<f)) # TODO: optimization and input range checking
+    if ((typemin(T) >> f) <= x) & (x <= (typemax(T) >> f))
+        reinterpret(F, unsafe_trunc(T, x) << f)
+    else
+        throw_converterror(F, x)
+    end
 end
 
 function _convert(::Type{F}, x::AbstractFloat) where {T, f, F <: Fixed{T,f}}
-    reinterpret(F, round(T, trunc(widen1(T),x)<<f + rem(x,1)*(one(widen1(T))<<f))) # TODO: optimization and input range checking
+    bigx = big(x)
+    bmin = BigFloat(typemin(F)) - @exp2(-f-1)
+    bmax = BigFloat(typemax(F)) + @exp2(-f-1)
+    if bmin <= bigx < bmax
+        reinterpret(F, round(T, bigx * @exp2(f)))
+    else
+        throw_converterror(F, x)
+    end
+end
+
+_convert(::Type{F}, x::Float16) where {T, f, F <: Fixed{T,f}} = F(Float32(x))
+
+function _convert(::Type{F}, x::Union{Float32, Float64}) where {T, f, F <: Fixed{T,f}}
+    Tf = typeof(x)
+    if Tf(typemin(F) - @exp2(-f-1)) <= x < Tf(typemax(F) + @exp2(-f-1))
+        reinterpret(F, round(T, x * @exp2(f)))
+    else
+        throw_converterror(F, x)
+    end
 end
 
 function _convert(::Type{F}, x::Rational) where {T, f, F <: Fixed{T,f}}
diff --git a/test/fixed.jl b/test/fixed.jl
index 4ad1ebc3..dd0764a8 100644
--- a/test/fixed.jl
+++ b/test/fixed.jl
@@ -82,8 +82,8 @@ end
 end
 
 @testset "inexactness" begin
-    @test_throws InexactError Q0f7(-2)
     # TODO: change back to InexactError when it allows message strings
+    @test_throws ArgumentError Q0f7(-2)
     @test_throws ArgumentError one(Q0f15)
     @test_throws ArgumentError oneunit(Q0f31)
     @test_throws ArgumentError one(Fixed{Int8,8}) # TODO: remove this at end of its support
@@ -92,11 +92,19 @@ end
 @testset "conversion" begin
     @test isapprox(convert(Fixed{Int8,7}, 0.8), 0.797, atol=0.001)
     @test isapprox(convert(Fixed{Int8,7}, 0.9), 0.898, atol=0.001)
-    @test_throws InexactError convert(Fixed{Int8, 7}, 0.999)
-    @test_throws InexactError convert(Fixed{Int8, 7}, 1.0)
-    @test_throws InexactError convert(Fixed{Int8, 7}, 1)
-    @test_throws InexactError convert(Fixed{Int8, 7}, 2)
-    @test_throws InexactError convert(Fixed{Int8, 7}, 128)
+    @test_throws ArgumentError convert(Fixed{Int8, 7}, 0.999)
+    @test_throws ArgumentError convert(Fixed{Int8, 7}, 1.0)
+    @test_throws ArgumentError convert(Fixed{Int8, 7}, 1)
+    @test_throws ArgumentError convert(Fixed{Int8, 7}, 2)
+    @test_throws ArgumentError convert(Fixed{Int8, 7}, 128)
+    @test_throws ArgumentError convert(Fixed{Int8, 7}, 1.0)
+
+    @test convert(Q0f7, -128.5/128) == -1
+
+    @test convert(Q0f7, -0.75f0) == -0.75
+    @test convert(Q0f7, Float16(-0.75)) == -0.75
+    @test convert(Q0f7, BigFloat(-0.75)) == -0.75
+    @test_throws ArgumentError convert(Q0f7, BigFloat(127.5/128))
 
     @test convert(Q2f5, -1//2) === -0.5Q2f5
     @test convert(Q1f6, Rational{Int8}(-3//4)) === -0.75Q1f6
@@ -105,7 +113,7 @@ end
     @test_throws ArgumentError convert(Q0f7, typemax(Rational{Int8}))
 
     @test convert(Q0f7, Base.TwicePrecision(0.5)) === 0.5Q0f7
-    @test_throws InexactError convert(Q7f8, Base.TwicePrecision(0x80, 0x01))
+    @test_throws ArgumentError convert(Q7f8, Base.TwicePrecision(0x80, 0x01))
     tp = Base.TwicePrecision(0xFFFFFFFFp-32, 0xFFFFFFFEp-64)
     @test convert(Q0f63, tp) === reinterpret(Q0f63, typemax(Int64))
 end