fix #7717 roundtrip float to string; fix `parseFloat` for js (#18248)

* refs #7717 roundtrip float to string
* make parseFloat more correct
* improve float tests
* improve float tests
* cleanup

3 files changed, 125 insertions, 76 deletions

diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim
index 371cb7962..7cb5652c5 100644
--- a/lib/system/jssys.nim
+++ b/lib/system/jssys.nim
@@ -716,19 +716,23 @@ proc tenToThePowerOf(b: int): BiggestFloat =
 const
   IdentChars = {'a'..'z', 'A'..'Z', '0'..'9', '_'}
 
-# XXX use JS's native way here
-proc nimParseBiggestFloat(s: string, number: var BiggestFloat, start = 0): int {.
-                          compilerproc.} =
-  var
-    esign = 1.0
-    sign = 1.0
-    i = start
-    exponent: int
-    flags: int
-  number = 0.0
+
+proc parseFloatNative(a: string): float =
+  let a2 = a.cstring
+  asm """
+  `result` = Number(`a2`);
+  """
+
+#[
+xxx how come code like this doesn't give IndexDefect ?
+let z = s[10000] == 'a'
+]#
+proc nimParseBiggestFloat(s: string, number: var BiggestFloat, start: int): int {.compilerproc.} =
+  var sign: bool
+  var i = start
   if s[i] == '+': inc(i)
   elif s[i] == '-':
-    sign = -1.0
+    sign = true
     inc(i)
   if s[i] == 'N' or s[i] == 'n':
     if s[i+1] == 'A' or s[i+1] == 'a':
@@ -741,52 +745,41 @@ proc nimParseBiggestFloat(s: string, number: var BiggestFloat, start = 0): int {
     if s[i+1] == 'N' or s[i+1] == 'n':
       if s[i+2] == 'F' or s[i+2] == 'f':
         if s[i+3] notin IdentChars:
-          number = Inf*sign
+          number = if sign: -Inf else: Inf
           return i+3 - start
     return 0
-  while s[i] in {'0'..'9'}:
-    # Read integer part
-    flags = flags or 1
-    number = number * 10.0 + toFloat(ord(s[i]) - ord('0'))
+
+  var buf: string
+    # we could also use an `array[char, N]` buffer to avoid reallocs, or
+    # use a 2-pass algorithm that first computes the length.
+  if sign: buf.add '-'
+  template addInc =
+    buf.add s[i]
     inc(i)
+  template eatUnderscores =
     while s[i] == '_': inc(i)
-  # Decimal?
-  if s[i] == '.':
-    var hd = 1.0
+  while s[i] in {'0'..'9'}: # Read integer part
+    buf.add s[i]
     inc(i)
-    while s[i] in {'0'..'9'}:
-      # Read fractional part
-      flags = flags or 2
-      number = number * 10.0 + toFloat(ord(s[i]) - ord('0'))
-      hd = hd * 10.0
-      inc(i)
-      while s[i] == '_': inc(i)
-    number = number / hd # this complicated way preserves precision
+    eatUnderscores()
+  if s[i] == '.': # Decimal?
+    addInc()
+    while s[i] in {'0'..'9'}: # Read fractional part
+      addInc()
+      eatUnderscores()
   # Again, read integer and fractional part
-  if flags == 0: return 0
-  # Exponent?
-  if s[i] in {'e', 'E'}:
-    inc(i)
-    if s[i] == '+':
-      inc(i)
-    elif s[i] == '-':
-      esign = -1.0
-      inc(i)
-    if s[i] notin {'0'..'9'}:
-      return 0
+  if buf.len == ord(sign): return 0
+  if s[i] in {'e', 'E'}: # Exponent?
+    addInc()
+    if s[i] == '+': inc(i)
+    elif s[i] == '-': addInc()
+    if s[i] notin {'0'..'9'}: return 0
     while s[i] in {'0'..'9'}:
-      exponent = exponent * 10 + ord(s[i]) - ord('0')
-      inc(i)
-      while s[i] == '_': inc(i)
-  # Calculate Exponent
-  let hd = tenToThePowerOf(exponent)
-  if esign > 0.0: number = number * hd
-  else:           number = number / hd
-  # evaluate sign
-  number = number * sign
+      addInc()
+      eatUnderscores()
+  number = parseFloatNative(buf)
   result = i - start
 
-
 # Workaround for IE, IE up to version 11 lacks 'Math.trunc'. We produce
 # 'Math.trunc' for Nim's ``div`` and ``mod`` operators:
 const jsMathTrunc = """
diff --git a/tests/float/tfloat6.nim b/tests/float/tfloat6.nim
deleted file mode 100644
index fa8fc9da8..000000000
--- a/tests/float/tfloat6.nim
+++ /dev/null
@@ -1,28 +0,0 @@
-discard """
-  output: '''
-0.000001 : 0.000001
-0.000001 : 0.000001
-0.001 : 0.001
-0.000001 : 0.000001
-0.000001 : 0.000001
-10.000001 : 10.000001
-100.000001 : 100.000001
-'''
-  disabled: "windows"
-"""
-
-import strutils
-
-echo "0.00_0001".parseFloat(), " : ", 1E-6
-echo "0.00__00_01".parseFloat(), " : ", 1E-6
-echo "0.0_01".parseFloat(), " : ", 0.001
-echo "0.00_000_1".parseFloat(), " : ", 1E-6
-echo "0.00000_1".parseFloat(), " : ", 1E-6
-
-echo "1_0.00_0001".parseFloat(), " : ", 10.000001
-echo "1__00.00_0001".parseFloat(), " : ", 1_00.000001
-
-# bug #18148
-
-var a = 1.1'f32
-doAssert $a == "1.1", $a # fails
diff --git a/tests/float/tfloats.nim b/tests/float/tfloats.nim
new file mode 100644
index 000000000..406ddb6d9
--- /dev/null
+++ b/tests/float/tfloats.nim
@@ -0,0 +1,84 @@
+discard """
+  targets: "c cpp js"
+"""
+# disabled: "windows"
+
+#[
+xxx merge all or most float tests into this file
+]#
+
+import std/[fenv, math, strutils]
+
+proc equalsOrNaNs(a, b: float): bool =
+  if isNaN(a): isNaN(b)
+  elif a == 0:
+    b == 0 and signbit(a) == signbit(b)
+  else:
+    a == b
+
+template reject(a) =
+  doAssertRaises(ValueError): discard parseFloat(a)
+
+template main =
+  block:
+    proc test(a: string, b: float) =
+      let a2 = a.parseFloat
+      doAssert equalsOrNaNs(a2, b), $(a, a2, b)
+    test "0.00_0001", 1E-6
+    test "0.00__00_01", 1E-6
+    test "0.0_01", 0.001
+    test "0.00_000_1", 1E-6
+    test "0.00000_1", 1E-6
+    test "1_0.00_0001", 10.000001
+    test "1__00.00_0001", 1_00.000001
+    test "inf", Inf
+    test "-inf", -Inf
+    test "-Inf", -Inf
+    test "-INF", -Inf
+    test "NaN", NaN
+    test "-nan", NaN
+    test ".1", 0.1
+    test "-.1", -0.1
+    test "-0", -0.0
+    when false: # pending bug #18246
+      test "-0", -0.0
+    test ".1e-1", 0.1e-1
+    test "0_1_2_3.0_1_2_3E+0_1_2", 123.0123e12
+    test "0_1_2.e-0", 12e0
+    test "0_1_2e-0", 12e0
+    test "-0e0", -0.0
+    test "-0e-0", -0.0
+
+  reject "a"
+  reject ""
+  reject "e1"
+  reject "infa"
+  reject "infe1"
+  reject "_"
+  reject "1e"
+
+  when false: # gray area; these numbers should probably be invalid
+    reject "1_"
+    reject "1_.0"
+    reject "1.0_"
+
+  block: # bug #18148
+    var a = 1.1'f32
+    doAssert $a == "1.1", $a # was failing
+
+  block: # bug #7717
+    proc test(f: float) =
+      let f2 = $f
+      let f3 = parseFloat(f2)
+      doAssert equalsOrNaNs(f, f3), $(f, f2, f3)
+
+    test 1.0 + epsilon(float64)
+    test 1000000.0000000123
+    test log2(100000.0)
+    test maximumPositiveValue(float32)
+    test maximumPositiveValue(float64)
+    test minimumPositiveValue(float32)
+    test minimumPositiveValue(float64)
+
+static: main()
+main()