Skip to content

Commit ddee978

Browse files
NHDalyNHDaly
authored
Fix bug in v0.5.1 for comparisons that underflow (large negatives) (#89)
* Fix bug in v0.5.1 for comparisons that underflow (large negatives) * Bump version to v0.5.2 for this bug fix
1 parent 507185c commit ddee978

File tree

3 files changed

+142
-56
lines changed

3 files changed

+142
-56
lines changed

‎Project.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name = "FixedPointDecimals"
22
uuid = "fb4d412d-6eee-574d-9565-ede6634db7b0"
33
authors = ["Fengyang Wang <fengyang.wang.0@gmail.com>", "Curtis Vogt <curtis.vogt@gmail.com>"]
4-
version = "0.5.1"
4+
version = "0.5.2"
55

66
[deps]
77
Parsers = "69de0a69-1ddd-5017-9359-2bf0b02dc9f0"

‎src/FixedPointDecimals.jl‎

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -558,19 +558,34 @@ for comp_op in (:(==), :(<), :(<=))
558558
xi, yi = promote(x.i, y.i)
559559
if f1 > f2
560560
C = coefficient(newFD) ÷ coefficient(FD{T2,f2})
561-
yi, overflowed = Base.mul_with_overflow(yi, C)
562-
if $(comp_op == :(==))
563-
overflowed && return false
564-
else
565-
# y overflowed, so it's bigger than x, since it doesn't fit in x's type
566-
overflowed && return true
561+
yi, wrapped = Base.mul_with_overflow(yi, C)
562+
if wrapped
563+
is_underflow = (y.i < 0)
564+
if !is_underflow
565+
# Whether we're computing `==`, `<` or `<=`, if y overflowed, it
566+
# means it's bigger than x.
567+
return $(comp_op == :(==)) ? false : true
568+
else # underflow
569+
# If y is negative, then y is definitely less than x, since y is so
570+
# small, it doesn't even fit in y's type.
571+
return false
572+
end
567573
end
568574
else
569575
C = coefficient(newFD) ÷ coefficient(FD{T1,f1})
570-
xi, overflowed = Base.mul_with_overflow(xi, C)
571-
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
572-
# bigger than y, so this is false.
573-
overflowed && return false
576+
xi, wrapped = Base.mul_with_overflow(xi, C)
577+
if wrapped
578+
is_underflow = (x.i < 0)
579+
if !is_underflow
580+
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it
581+
# means it's bigger than y, so this is false.
582+
return false
583+
else # underflow
584+
# If x is negative, then x is definitely less than y, since x is so
585+
# small, it doesn't even fit in y's type.
586+
return $(comp_op == :(==)) ? false : true
587+
end
588+
end
574589
end
575590
return $comp_op(xi, yi)
576591
end
@@ -593,10 +608,19 @@ for comp_op in (:(==), :(<), :(<=))
593608
end
594609
# Now it's safe to truncate x down to y's type.
595610
xi = x % T
596-
xi, overflowed = Base.mul_with_overflow(xi, coefficient(FD{T,f}))
597-
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
598-
# bigger than y, so this is false.
599-
overflowed && return false
611+
xi, wrapped = Base.mul_with_overflow(xi, coefficient(FD{T,f}))
612+
if wrapped
613+
is_underflow = (x < 0)
614+
if !is_underflow
615+
# Whether we're computing `==`, `<` or `<=`, if x overflowed, it means it's
616+
# bigger than y, so this is false.
617+
return false
618+
else # underflow
619+
# If x is negative, then x is definitely less than y, since x is so
620+
# small, it doesn't even fit in y's type.
621+
return $(comp_op == :(==)) ? false : true
622+
end
623+
end
600624
return $comp_op(xi, y.i)
601625
end
602626
end
@@ -618,12 +642,18 @@ for comp_op in (:(==), :(<), :(<=))
618642
end
619643
# Now it's safe to truncate x down to y's type.
620644
yi = y % T
621-
yi, overflowed = Base.mul_with_overflow(yi, coefficient(FD{T,f}))
622-
if $(comp_op == :(==))
623-
overflowed && return false
624-
else
625-
# y overflowed, so it's bigger than x, since it doesn't fit in x's type
626-
overflowed && return true
645+
yi, wrapped = Base.mul_with_overflow(yi, coefficient(FD{T,f}))
646+
if wrapped
647+
is_underflow = (y < 0)
648+
if !is_underflow
649+
# Whether we're computing `==`, `<` or `<=`, if y overflowed, it means it's
650+
# bigger than x.
651+
return $(comp_op == :(==)) ? false : true
652+
else # underflow
653+
# If y is negative, then y is definitely less than x, since y is so
654+
# small, it doesn't even fit in y's type.
655+
return false
656+
end
627657
end
628658
return $comp_op(x.i, yi)
629659
end

‎test/FixedDecimal.jl‎

Lines changed: 91 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,10 @@ end
297297
@test FD{Int8, 2}(0) != typemax(Int16)
298298
@test FD{Int8, 0}(0) != typemin(Int16)
299299
@test FD{Int8, 2}(0) != typemin(Int16)
300+
301+
# less precision allows smaller and bigger numbers
302+
@test typemin(FD{Int8, 1}) != typemin(FD{Int8,2})
303+
@test typemax(FD{Int8, 1}) != typemax(FD{Int8,2})
300304
end
301305
@testset "inequality between types" begin
302306
@test FD{Int8, 0}(1) <= FD{Int8, 2}(1)
@@ -312,41 +316,93 @@ end
312316
@test FD{Int8, 0}(4) >= FD{Int16, 4}(1) # FD{Int16,4}(4) doesn't fit
313317
@test FD{Int16, 4}(1) < FD{Int8, 0}(4) # FD{Int16,4}(4) doesn't fit
314318

315-
# Integer == FD
316-
@test 1 <= FD{Int8, 2}(1) <= 1
317-
@test 1 >= FD{Int8, 2}(1) >= 1
318-
@test 2 > FD{Int8, 2}(1)
319-
@test FD{Int8, 2}(1) < 2
320-
@test 2 >= FD{Int8, 2}(1)
321-
@test FD{Int8, 2}(1) <= 2
322-
@test 1 <= FD{Int8, 0}(1) < 2
323-
324-
@test typemax(Int16) > FD{Int8, 0}(1) > typemin(Int16)
325-
@test typemax(Int16) > FD{Int8, 2}(1) > typemin(Int16)
326-
@test typemin(Int16) < FD{Int8, 0}(1) < typemax(Int16)
327-
@test typemin(Int16) < FD{Int8, 2}(1) < typemax(Int16)
328-
@test !(typemax(Int16) < FD{Int8, 0}(1) < typemin(Int16))
329-
@test !(typemax(Int16) < FD{Int8, 2}(1) < typemin(Int16))
330-
@test !(typemin(Int16) > FD{Int8, 0}(1) > typemax(Int16))
331-
@test !(typemin(Int16) > FD{Int8, 2}(1) > typemax(Int16))
332-
333-
@test typemax(Int16) > FD{Int8, 0}(-1) > typemin(Int16)
334-
@test typemax(Int16) > FD{Int8, 2}(-1) > typemin(Int16)
335-
@test typemin(Int16) < FD{Int8, 0}(-1) < typemax(Int16)
336-
@test typemin(Int16) < FD{Int8, 2}(-1) < typemax(Int16)
337-
@test !(typemax(Int16) < FD{Int8, 0}(-1) < typemin(Int16))
338-
@test !(typemax(Int16) < FD{Int8, 2}(-1) < typemin(Int16))
339-
@test !(typemin(Int16) > FD{Int8, 0}(-1) > typemax(Int16))
340-
@test !(typemin(Int16) > FD{Int8, 2}(-1) > typemax(Int16))
341-
342-
@test typemax(Int16) >= FD{Int8, 0}(0) >= typemin(Int16)
343-
@test typemax(Int16) >= FD{Int8, 2}(0) >= typemin(Int16)
344-
@test typemin(Int16) <= FD{Int8, 0}(0) <= typemax(Int16)
345-
@test typemin(Int16) <= FD{Int8, 2}(0) <= typemax(Int16)
346-
@test !(typemax(Int16) <= FD{Int8, 0}(-1) <= typemin(Int16))
347-
@test !(typemax(Int16) <= FD{Int8, 2}(-1) <= typemin(Int16))
348-
@test !(typemin(Int16) >= FD{Int8, 0}(-1) >= typemax(Int16))
349-
@test !(typemin(Int16) >= FD{Int8, 2}(-1) >= typemax(Int16))
319+
# less precision allows smaller numbers
320+
@test typemin(FD{Int8, 1}) < typemin(FD{Int8,2})
321+
@test typemin(FD{Int8, 1}) <= typemin(FD{Int8,2})
322+
@test typemin(FD{Int8, 2}) > typemin(FD{Int8,1})
323+
@test typemin(FD{Int8, 2}) >= typemin(FD{Int8,1})
324+
# less precision allows bigger numbers
325+
@test typemax(FD{Int8, 1}) > typemax(FD{Int8,2})
326+
@test typemax(FD{Int8, 1}) >= typemax(FD{Int8,2})
327+
@test typemax(FD{Int8, 2}) < typemax(FD{Int8,1})
328+
@test typemax(FD{Int8, 2}) <= typemax(FD{Int8,1})
329+
330+
@test !(typemin(FD{Int8, 2}) <= typemin(FD{Int8,1}))
331+
@test !(typemin(FD{Int8, 1}) >= typemin(FD{Int8,2}))
332+
@test !(typemin(FD{Int8, 1}) > typemin(FD{Int8,2}))
333+
@test !(typemin(FD{Int8, 1}) >= typemin(FD{Int8,2}))
334+
@test !(typemin(FD{Int8, 2}) < typemin(FD{Int8,1}))
335+
@test !(typemin(FD{Int8, 2}) <= typemin(FD{Int8,1}))
336+
@test !(typemax(FD{Int8, 1}) < typemax(FD{Int8,2}))
337+
@test !(typemax(FD{Int8, 1}) <= typemax(FD{Int8,2}))
338+
@test !(typemax(FD{Int8, 2}) > typemax(FD{Int8,1}))
339+
@test !(typemax(FD{Int8, 2}) >= typemax(FD{Int8,1}))
340+
341+
@testset "Integer and FD" begin
342+
@test 1 <= FD{Int8, 2}(1) <= 1
343+
@test 1 >= FD{Int8, 2}(1) >= 1
344+
@test 2 > FD{Int8, 2}(1)
345+
@test FD{Int8, 2}(1) < 2
346+
@test 2 >= FD{Int8, 2}(1)
347+
@test FD{Int8, 2}(1) <= 2
348+
@test 1 <= FD{Int8, 0}(1) < 2
349+
350+
# negatives
351+
@test -1 <= FD{Int8, 2}(-1) <= -1
352+
@test -1 >= FD{Int8, 2}(-1) >= -1
353+
@test -2 < FD{Int8, 2}(-1)
354+
@test FD{Int8, 2}(-1) > -2
355+
@test -2 <= FD{Int8, 2}(-1)
356+
@test FD{Int8, 2}(-1) >= -2
357+
@test -1 <= FD{Int8, 0}(-1) < 2
358+
359+
# Same types
360+
@test typemax(Int8) > FD{Int8, 0}(1) > typemin(Int8)
361+
@test typemax(Int8) > FD{Int8, 2}(1) > typemin(Int8)
362+
@test typemin(Int8) < FD{Int8, 0}(1) < typemax(Int8)
363+
@test typemin(Int8) < FD{Int8, 2}(1) < typemax(Int8)
364+
@test !(typemax(Int8) < FD{Int8, 0}(1) < typemin(Int8))
365+
@test !(typemax(Int8) < FD{Int8, 2}(1) < typemin(Int8))
366+
@test !(typemin(Int8) > FD{Int8, 0}(1) > typemax(Int8))
367+
@test !(typemin(Int8) > FD{Int8, 2}(1) > typemax(Int8))
368+
369+
@test typemax(Int8) > FD{Int8, 0}(-1) > typemin(Int8)
370+
@test typemax(Int8) > FD{Int8, 2}(-1) > typemin(Int8)
371+
@test typemin(Int8) < FD{Int8, 0}(-1) < typemax(Int8)
372+
@test typemin(Int8) < FD{Int8, 2}(-1) < typemax(Int8)
373+
@test !(typemax(Int8) < FD{Int8, 0}(-1) < typemin(Int8))
374+
@test !(typemax(Int8) < FD{Int8, 2}(-1) < typemin(Int8))
375+
@test !(typemin(Int8) > FD{Int8, 0}(-1) > typemax(Int8))
376+
@test !(typemin(Int8) > FD{Int8, 2}(-1) > typemax(Int8))
377+
378+
# Different types
379+
@test typemax(Int16) > FD{Int8, 0}(1) > typemin(Int16)
380+
@test typemax(Int16) > FD{Int8, 2}(1) > typemin(Int16)
381+
@test typemin(Int16) < FD{Int8, 0}(1) < typemax(Int16)
382+
@test typemin(Int16) < FD{Int8, 2}(1) < typemax(Int16)
383+
@test !(typemax(Int16) < FD{Int8, 0}(1) < typemin(Int16))
384+
@test !(typemax(Int16) < FD{Int8, 2}(1) < typemin(Int16))
385+
@test !(typemin(Int16) > FD{Int8, 0}(1) > typemax(Int16))
386+
@test !(typemin(Int16) > FD{Int8, 2}(1) > typemax(Int16))
387+
388+
@test typemax(Int16) > FD{Int8, 0}(-1) > typemin(Int16)
389+
@test typemax(Int16) > FD{Int8, 2}(-1) > typemin(Int16)
390+
@test typemin(Int16) < FD{Int8, 0}(-1) < typemax(Int16)
391+
@test typemin(Int16) < FD{Int8, 2}(-1) < typemax(Int16)
392+
@test !(typemax(Int16) < FD{Int8, 0}(-1) < typemin(Int16))
393+
@test !(typemax(Int16) < FD{Int8, 2}(-1) < typemin(Int16))
394+
@test !(typemin(Int16) > FD{Int8, 0}(-1) > typemax(Int16))
395+
@test !(typemin(Int16) > FD{Int8, 2}(-1) > typemax(Int16))
396+
397+
@test typemax(Int16) >= FD{Int8, 0}(0) >= typemin(Int16)
398+
@test typemax(Int16) >= FD{Int8, 2}(0) >= typemin(Int16)
399+
@test typemin(Int16) <= FD{Int8, 0}(0) <= typemax(Int16)
400+
@test typemin(Int16) <= FD{Int8, 2}(0) <= typemax(Int16)
401+
@test !(typemax(Int16) <= FD{Int8, 0}(-1) <= typemin(Int16))
402+
@test !(typemax(Int16) <= FD{Int8, 2}(-1) <= typemin(Int16))
403+
@test !(typemin(Int16) >= FD{Int8, 0}(-1) >= typemax(Int16))
404+
@test !(typemin(Int16) >= FD{Int8, 2}(-1) >= typemax(Int16))
405+
end
350406
end
351407

352408
@testset "128-bit conversion correctness" begin

0 commit comments

Comments
 (0)