fennel: Fix huge numbers NPE, make best effort at keeping original integer format v3 PROPOSED

: 1
 Fix huge numbers NPE, make best effort at keeping original integer format

 5 files changed, 68 insertions(+), 8 deletions(-)

Phil Hagelberg <phil@hagelb.org> writes:

Whew. This is more complicated than I expected, but honestly I don't
see another way around it. It would be nice if Lua would do the right
thing out of the box, but there's nothing we can do about that.

I have some ideas about what fennel.view should do with infinity,
because I don't think the current behavior is correct, but that's a
different concern and doesn't need to block this.

I'll go ahead and apply and push your changes; thanks for your help with
this!

-Phil

[PATCH fennel v3] Fix huge numbers NPE, make best effort at keeping original integer format Export this patch

From: Andrey Listopadov <andreyorst@gmail.com>

In Lua, numbers bigger than 1e+308 compile to inf.  Prior to this
patch, Fennel compiled such numbers to inf too.  However, inf is not
a number in Lua, neither it is a reserved symbol, so the expression
(+ 1e+309 1) would compile to (inf + 1) resulting in attempt to add
nil with a number, because Lua treats inf as a regular, unbound,
variable.

This patch also tries to keep the number format of big numbers, bigger
than 1e+13 and less than 1e+309 in the e-notation. Numbers less than
1e+14 are formatted as integers, as done in PUC Lua. Starting from
1e+14 the e-notation is used.

If the number is big enough to be considered inf by Lua, it is
compiled to (1/0), a portable way to express infinity, without relying
on the existence of the math table. -inf is compiled to (-1/0).

Phil Hagelberg <phil@hagelb.org>

a month ago

Thanks! I will take a look tomorrow.

-Phil

---
 changelog.md            |  1 +
 src/fennel/compiler.fnl | 22 +++++++++++++++++++---
 src/fennel/view.fnl     | 22 +++++++++++++++++++---
 test/core.fnl           |  9 ++++++++-
 test/parser.fnl         | 22 +++++++++++++++++++++-
 5 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/changelog.md b/changelog.md
index 560fcf3..398af18 100644
--- a/changelog.md
+++ b/changelog.md
@@ -15,6 +15,7 @@ deprecated forms.
 * Macro quote expansion no longer breaks when `sym`, `list` or `sequence` is shadowed
 * Bring `fennel.traceback` behavior closer to Lua's `traceback` by
   not modifying non-string and non-`nil` values.
+* Avoid losing precision when compiling large numbers on LuaJIT.
 
 ## 1.5.0 / 2024-06-23
 
diff --git a/src/fennel/compiler.fnl b/src/fennel/compiler.fnl
index 0faf74b..0f34b65 100644
--- a/src/fennel/compiler.fnl
+++ b/src/fennel/compiler.fnl
@@ -533,10 +533,26 @@ (fn compile-sym
 
 ;; We do gsub transformation because some locales use , for
 ;; decimal separators, which will not be accepted by Lua.
+;; Makes best effort to keep the original notation of the number.
 (fn serialize-number [n]
-  (if (= (math.floor n) n)
-      (string.format "%d" n)
-      (string.gsub (tostring n) "," ".")))
+  (let [val (if (= (math.floor n) n)
+                (let [s1 (string.format "%.f" n)]
+                  (if (= s1 "inf") "(1/0)" ; portable inf
+                      (= s1 "-inf") "(-1/0)"
+                      (= s1 (tostring n)) s1 ; no precision loss
+                      (or (faccumulate [s nil
+                                        i 0 308 ; beyond 308 every number turns to inf
+                                        :until s]
+                            (let [s (string.format (.. "%." i "e") n)]
+                              (when (= n (tonumber s))
+                                (let [exp (s:match "e%+?(%d+)$")]
+                                  ;; Lua keeps numbers in standard notation up to e+14
+                                  (if (and exp (> (tonumber exp) 14))
+                                      s
+                                      s1)))))
+                          s1)))
+                (tostring n))]
+    (pick-values 1 (string.gsub val "," "."))))
 
 (fn compile-scalar [ast _scope parent opts]
   (let [serialize (match (type ast)
diff --git a/src/fennel/view.fnl b/src/fennel/view.fnl
index 08ef7f8..952ec8f 100644
--- a/src/fennel/view.fnl
+++ b/src/fennel/view.fnl
@@ -282,11 +282,27 @@ (fn pp-table
     (set options.level (- options.level 1))
     x))
 
+;; A modified copy of compiler.serialize-number that doesn't handle
+;; the infinity cases
 (fn number->string [n]
   ;; Transform number to a string without depending on correct `os.locale`
-  (if (= (math.floor n) n)
-      (string.format "%d" n)
-      (pick-values 1 (string.gsub (tostring n) "," "."))))
+  ;; Makes best effort to keep the original notation of the number.
+  (let [val (if (= (math.floor n) n)
+                (let [s1 (string.format "%.f" n)]
+                  (if (= s1 (tostring n)) s1 ; no precision loss
+                      (or (faccumulate [s nil
+                                        i 0 308 ; beyond 308 every number turns to inf
+                                        :until s]
+                            (let [s (string.format (.. "%." i "e") n)]
+                              (when (= n (tonumber s))
+                                (let [exp (s:match "e%+?(%d+)$")]
+                                  ;; Lua keeps numbers in standard notation up to e+14
+                                  (if (and exp (> (tonumber exp) 14))
+                                      s
+                                      s1)))))
+                          s1)))
+                (tostring n))]
+    (pick-values 1 (string.gsub val "," "."))))
 
 (fn colon-string? [s]
   ;; Test if given string is valid colon string.
diff --git a/test/core.fnl b/test/core.fnl
index 4b138b5..4b9ed83 100644
--- a/test/core.fnl
+++ b/test/core.fnl
@@ -561,7 +561,14 @@ (fn test-with-open
       ["asdf" "closed file" "closed file"])
   (== [(with-open [proc1 (io.popen "echo hi") proc2 (io.popen "echo bye")]
          (values (proc1:read) (proc2:read)))]
-      ["hi" "bye"]))
+      ["hi" "bye"])
+  (== (do
+        (var fh nil)
+        (local (ok msg) (pcall #(with-open [f (io.tmpfile)]
+                                  (set fh f)
+                                  (error {:bork! :bark!}))))
+        [(io.type fh) ok (case msg {:bork! :bark!} msg _ "didn't match")])
+      ["closed file" false {:bork! :bark!}]))
 
 (fn test-comment []
   (t.= "--[[ hello world ]]\nreturn nil"
diff --git a/test/parser.fnl b/test/parser.fnl
index 57df216..2a1810f 100644
--- a/test/parser.fnl
+++ b/test/parser.fnl
@@ -20,8 +20,28 @@ (fn test-basics
        [((fennel.parser (fennel.string-stream "&abc ")))])
   (t.= "141791343654238"
        (fennel.view (fennel.eval "141791343654238")))
+  (t.= "141791343654238"
+       (fennel.view (fennel.eval "1.41791343654238e+14")))
+  (t.= "1.41791343654238e+15"
+       (fennel.view (fennel.eval "1.41791343654238e+15")))
   (t.= "14179134365.125"
-       (fennel.view (fennel.eval "14179134365.125"))))
+       (fennel.view (fennel.eval "14179134365.125")))
+  (t.= "2.3456789012e+76"
+       (fennel.view (fennel.eval "23456789012000000000000000000000000000000000000000000000000000000000000000000")))
+  (t.= "1.23456789e-13"
+       (fennel.view (fennel.eval "1.23456789e-13")))
+  (t.= "inf"
+       (fennel.view (fennel.eval "1e+999999")))
+  (t.= "-inf"
+       (fennel.view (fennel.eval "-1e+999999")))
+  (t.= "1e+308"
+       (fennel.view (fennel.eval (faccumulate [res "" _ 1 308] (.. res "9")))))
+  (t.= "inf"
+       (fennel.view (fennel.eval (faccumulate [res "" _ 1 309] (.. res "9")))))
+  (t.= "inf"
+       (fennel.view (fennel.eval "(/ 1 0)")))
+  (t.= "-inf"
+       (fennel.view (fennel.eval "(/ -1 0)"))))
 
 (fn test-comments []
   (let [(ok? ast) ((fennel.parser (fennel.string-stream ";; abc")
-- 
2.45.0

builds.sr.ht <builds@sr.ht>

a month ago

fennel/patches/.build.yml: SUCCESS in 31s

[Fix huge numbers NPE, make best effort at keeping original integer format][0] v3 from [][1]

[0]: https://lists.sr.ht/~technomancy/fennel/patches/54452
[1]: mailto:andreyorst@gmail.com

✓ #1302536 SUCCESS fennel/patches/.build.yml https://builds.sr.ht/~technomancy/job/1302536

Phil Hagelberg <phil@hagelb.org>

a month ago

andreyorst@gmail.com writes: