Merge #157

157: use improved range check of 128 for float64 r=alaviss a=krux02

 * Add a range check function to test if a float64 is in rage of the `int128` type.
 * Use that range check function for `semexpr.checkConvertible`
 * fixes #93

While this fixes the original issue mentioned above, it does not fix or implement the discussions in that issue. They need to be ported to their own issue.

Co-authored-by: Arne Döring <arne.doering@gmx.net>

compiler/sem/semexprs.nim

@@ -182,10 +182,11 @@ proc checkConvertible(c: PContext, targetTyp: PType, src: PNode): TConvStatus =
       if src.kind in nkCharLit..nkUInt64Lit and
           src.getInt notin firstOrd(c.config, targetTyp)..lastOrd(c.config, targetTyp):
         result = convNotInRange
-      elif src.kind in nkFloatLit..nkFloat64Lit and
-          (classify(src.floatVal) in {fcNan, fcNegInf, fcInf} or
-            src.floatVal.int64 notin firstOrd(c.config, targetTyp)..lastOrd(c.config, targetTyp)):
-        result = convNotInRange
+      elif src.kind in nkFloatLit..nkFloat64Lit:
+        if not src.floatVal.inInt128Range:
+          result = convNotInRange
+        elif src.floatVal.toInt128 notin firstOrd(c.config, targetTyp)..lastOrd(c.config, targetTyp):
+          result = convNotInRange
     elif targetBaseTyp.kind in tyFloat..tyFloat64:
       if src.kind in nkFloatLit..nkFloat64Lit and
           not floatRangeCheck(src.floatVal, targetTyp):

compiler/utils/int128.nim

@@ -3,7 +3,7 @@
 ## hold all from `low(BiggestInt)` to `high(BiggestUInt)`, This
 ## type is for that purpose.
 
-from math import trunc
+from math import trunc, pow, isNaN
 
 type
   Int128* = object
@@ -20,8 +20,8 @@ const
   Zero* = Int128(udata: [0'u32, 0, 0, 0])
   One* = Int128(udata: [1'u32, 0, 0, 0])
   Ten* = Int128(udata: [10'u32, 0, 0, 0])
-  Min = Int128(udata: [0'u32, 0, 0, 0x80000000'u32])
-  Max = Int128(udata: [high(uint32), high(uint32), high(uint32), uint32(high(int32))])
+  Min* = Int128(udata: [0'u32, 0, 0, 0x80000000'u32])
+  Max* = Int128(udata: [high(uint32), high(uint32), high(uint32), uint32(high(int32))])
   NegOne* = Int128(udata: [0xffffffff'u32, 0xffffffff'u32, 0xffffffff'u32, 0xffffffff'u32])
 
 template low*(t: typedesc[Int128]): Int128 = Min
@@ -525,8 +525,16 @@ proc `+`*(a: BiggestInt, b: Int128): Int128 =
 proc `+`*(a: Int128, b: BiggestInt): Int128 =
   a + toInt128(b)
 
+const lowerBoundF64 = -pow(2'f64, 127)
+# 2^127-1 can't be represented with float64 precision, therefore upper bound is
+# Exclusive
+const upperBoundF64 = +pow(2'f64, 127)
+
 proc toFloat64*(arg: Int128): float64 =
   let isNegative = isNegative(arg)
+  if arg == Min:
+    return lowerBoundF64
+
   let arg = abs(arg)
 
   let a = float64(bitconcat(arg.udata[1], arg.udata[0]))
@@ -540,7 +548,25 @@ proc ldexp(x: float64, exp: cint): float64 {.importc: "ldexp", header: "<math.h>
 
 template bitor(a, b, c: Int128): Int128 = bitor(bitor(a, b), c)
 
+
+proc inInt128Range*(arg: float64): bool =
+  ## Simple range check. Of coures NaN is defined not to be in range.
+  lowerBoundF64 <= arg and arg < upperBoundF64
+
 proc toInt128*(arg: float64): Int128 =
+  # actually enabling this assert will cause /tests/misc/tconv.nim to fail
+  # assert(inInt128Range(arg), "out of range")
+
+  # dealing with out of range values, not correct but deterministic.
+  if isNaN(arg):
+    # The pick of 0 is inspired by WASM and Rust
+    return Zero
+  # inclusive test here `abs(arg)` doesn't work on `Min`
+  if arg <= lowerBoundF64:
+    return Min
+  if upperBoundF64 <= arg:
+    return Max
+
   let isNegative = arg < 0
   let v0 = ldexp(abs(arg), -100)
   let w0 = uint64(trunc(v0))

tests/compiler/tint128.nim

@@ -182,3 +182,48 @@ for i in 0 ..< 128:
   let str1 = toHex(e shl i)
   let str2 = strArray2[i]
   doAssert str1 == str2
+
+doAssert toInt128(Inf) == int128.Max
+doAssert toInt128(-Inf) == int128.Min
+doAssert toInt128(NaN) == int128.Zero
+doAssert toInt128(0.0) == int128.Zero
+doAssert toInt128(1e100) == int128.Max
+doAssert toInt128(-1e100) == int128.Min
+
+doAssert inInt128Range(Inf) == false
+doAssert inInt128Range(-Inf) == false
+doAssert inInt128Range(NaN) == false
+doAssert inInt128Range(0.0) == true
+doAssert inInt128Range(1e100) == false
+doAssert inInt128Range(-1e100) == false
+
+from math import pow
+
+proc nextafter(`from`,to: float64): float64 {.importc: "nextafter", header: "<math.h>".}
+
+let minValue = -pow(2'f64, 127)
+let maxValue = pow(2'f64, 127)
+
+let nonMinValue = nextafter(minValue, Inf)
+let lowerThanMinValue = nextafter(minValue, -Inf)
+
+proc testInt128RoundTrip(arg: float64) =
+  let int128Val = toInt128(arg)
+  let convertedBack = toFloat64(int128Val)
+  doAssert(arg == convertedBack)
+
+testInt128RoundTrip(minValue)
+# max value is exclusive, out of bounds. It doesn't need to be roundtrip safe
+testInt128RoundTrip(nextafter(maxValue, -Inf))
+testInt128RoundTrip(nonMinValue)
+testInt128RoundTrip(1337'f64)
+testInt128RoundTrip(4711'f64)
+testInt128RoundTrip(1000000'f64)
+
+doAssert $toInt128(minValue)                  == "-170141183460469231731687303715884105728"
+doAssert $toInt128(maxValue)                  ==  "170141183460469231731687303715884105727" # <- last digit different
+# test clamping
+doAssert $toInt128(nextafter(minValue, -Inf)) == "-170141183460469231731687303715884105728"
+doAssert $toInt128(nextafter(maxValue,  Inf)) ==  "170141183460469231731687303715884105727"
+
+doAssert toFloat64(toInt128(NaN)) == 0.0

tests/range/tcompiletime_range_checks.nim

@@ -1,22 +1,23 @@
 discard """
   cmd: "nim check --hint:Processing:off --hint:Conf:off $file"
   errormsg: "18446744073709551615 can't be converted to int8"
-  nimout: '''tcompiletime_range_checks.nim(36, 21) Error: 2147483648 can't be converted to int32
-tcompiletime_range_checks.nim(37, 23) Error: -1 can't be converted to uint64
-tcompiletime_range_checks.nim(38, 34) Error: 255 can't be converted to FullNegativeRange
-tcompiletime_range_checks.nim(39, 34) Error: 18446744073709551615 can't be converted to HalfNegativeRange
-tcompiletime_range_checks.nim(40, 34) Error: 300 can't be converted to FullPositiveRange
-tcompiletime_range_checks.nim(41, 30) Error: 101 can't be converted to UnsignedRange
-tcompiletime_range_checks.nim(42, 32) Error: -9223372036854775808 can't be converted to SemiOutOfBounds
-tcompiletime_range_checks.nim(44, 22) Error: nan can't be converted to int32
-tcompiletime_range_checks.nim(46, 23) Error: 1e+100 can't be converted to uint64
-tcompiletime_range_checks.nim(49, 22) Error: 18446744073709551615 can't be converted to int64
-tcompiletime_range_checks.nim(50, 22) Error: 18446744073709551615 can't be converted to int32
-tcompiletime_range_checks.nim(51, 22) Error: 18446744073709551615 can't be converted to int16
-tcompiletime_range_checks.nim(52, 21) Error: 18446744073709551615 can't be converted to int8
+  nimout: '''
+tcompiletime_range_checks.nim(37, 21) Error: 2147483648 can't be converted to int32
+tcompiletime_range_checks.nim(38, 23) Error: -1 can't be converted to uint64
+tcompiletime_range_checks.nim(39, 34) Error: 255 can't be converted to FullNegativeRange
+tcompiletime_range_checks.nim(40, 34) Error: 18446744073709551615 can't be converted to HalfNegativeRange
+tcompiletime_range_checks.nim(41, 34) Error: 300 can't be converted to FullPositiveRange
+tcompiletime_range_checks.nim(42, 30) Error: 101 can't be converted to UnsignedRange
+tcompiletime_range_checks.nim(43, 32) Error: -9223372036854775808 can't be converted to SemiOutOfBounds
+tcompiletime_range_checks.nim(45, 22) Error: nan can't be converted to int32
+tcompiletime_range_checks.nim(46, 22) Error: 1e+100 can't be converted to int64
+tcompiletime_range_checks.nim(47, 23) Error: 1e+100 can't be converted to uint64
+tcompiletime_range_checks.nim(50, 22) Error: 18446744073709551615 can't be converted to int64
+tcompiletime_range_checks.nim(51, 22) Error: 18446744073709551615 can't be converted to int32
+tcompiletime_range_checks.nim(52, 22) Error: 18446744073709551615 can't be converted to int16
+tcompiletime_range_checks.nim(53, 21) Error: 18446744073709551615 can't be converted to int8
   '''
 """
-
 type
   UnsignedRange* = range[0'u64 .. 100'u64]
   SemiOutOfBounds* = range[0x7ffffffffffffe00'u64 .. 0x8000000000000100'u64]