Subject: Problems with format and scaling floats

From: Timothy Sample <[email-masked]>
Date: Mon, 18 Oct 2021 17:22:49 -0400

Hi Guilers,

It turns out there’s a little blunder in ‘format’ (from ‘ice-9’).  Look
at what happens when using the SCALE argument to format a fixed-point
float (this is Guile from the Git repo at the time of writing):

    GNU Guile 3.0.7.6-22120
    Copyright (C) 1995-2021 Free Software Foundation, Inc.

    Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
    This program is free software, and you are welcome to redistribute it
    under certain conditions; type `,show c' for details.

    Enter `,help' for help.
    scheme@(guile-user)> (format #t "~,,3f~%" 0.00123)
    0.23
    $3 = #t
    scheme@(guile-user)> (format #t "~,,1f~%" 0.00123)
    ice-9/boot-9.scm:1685:16: In procedure raise-exception:
    Value out of range 0 to 400: -1

    Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.

The first example gives the wrong result.  Scaling 0.00123 by 3 should
yield 1.23, not 0.23.  For the second example, instead of 0.0123, we get
an error!  What’s going on here?

Well, our ‘format’ code comes from SLIB and was written in 1998, so it’s
not easy to explain.  There’s so much mutation even a C programmer would
blush!  ;)  The issue happens in the ‘format:parse-float’ procedure
(which is defined inside of ‘format’).  It normalizes the string
representation of a number, and applies the scale argument when needed.
It does this by keeping a string of digits and the location of the
decimal point.  Another thing it keeps track of the leading zeros in a
variable called ‘left-zeros’.  Here’s the code that does the final
shifting and places the decimal point:

    (if (> left-zeros 0)
        (if (<= left-zeros shift) ; shift always > 0 here
            (format:fn-shiftleft shift) ; shift out 0s
            (begin
              (format:fn-shiftleft left-zeros)
              (set! format:fn-dot (- shift left-zeros))))
        (set! format:fn-dot (+ format:fn-dot shift)))

The issue is that the cases in the inner ‘if’ form are reversed.  That
is, if there are MORE leading zeros than we need to shift, we can just
shift.  Otherwise (if there are FEWER leading zeros), we need to shift
out the zeros and then move the decimal point (‘format:fn-dot’).

AFAICS, this bug was in the original SLIB implementation (1998) and has
not been fixed since then.  It’s been in Guile since 1999.

Anyway, that’s more than anyone cares to know....  Here’s a patch with
tests!  :)

>From c31d1f5d44343da1201ea1be86bc6b2ac8af6c8d Mon Sep 17 00:00:00 2001
From: Timothy Sample <[email-masked]>
Date: Mon, 18 Oct 2021 17:07:41 -0400
Subject: [PATCH] ice-9 format: Fix scaling floats with leading zeros

---
 module/ice-9/format.scm      |  4 ++--
 test-suite/tests/format.test | 10 ++++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/module/ice-9/format.scm b/module/ice-9/format.scm
index 48d9c0c84..ee7cba910 100644
--- a/module/ice-9/format.scm
+++ b/module/ice-9/format.scm
@@ -1359,10 +1359,10 @@
                       (else
                        (if (> left-zeros 0)
                            (if (<= left-zeros shift) ; shift always > 0 here
-                               (format:fn-shiftleft shift) ; shift out 0s
                                (begin
                                  (format:fn-shiftleft left-zeros)
-                                 (set! format:fn-dot (- shift left-zeros))))
+                                 (set! format:fn-dot (- shift left-zeros)))
+                               (format:fn-shiftleft shift)) ; shift out 0s
                            (set! format:fn-dot (+ format:fn-dot shift))))))))
 
                (let ((negexp          ; expon format m.nnnEee
diff --git a/test-suite/tests/format.test b/test-suite/tests/format.test
index b9aa7a854..d5111f1c6 100644
--- a/test-suite/tests/format.test
+++ b/test-suite/tests/format.test
@@ -2,7 +2,7 @@
 ;;;; Matthias Koeppe <[email-masked]> --- June 2001
 ;;;;
 ;;;; Copyright (C) 2001, 2003, 2004, 2006, 2010, 2011, 2012,
-;;;;   2014, 2017 Free Software Foundation, Inc.
+;;;;   2014, 2017, 2021 Free Software Foundation, Inc.
 ;;;;
 ;;;; This library is free software; you can redistribute it and/or
 ;;;; modify it under the terms of the GNU Lesser General Public
@@ -121,7 +121,13 @@
   ;; in guile prior to 1.6.9 and 1.8.1, leading zeros were incorrectly
   ;; stripped, moving the decimal point and giving "25.0" here
   (pass-if "string 02.5"
-    (string=? "2.5" (format #f "~f" "02.5"))))
+    (string=? "2.5" (format #f "~f" "02.5")))
+
+  (pass-if "scale with few leading zeros"
+    (string=? "1.23" (format #f "~,,3f" "0.00123")))
+
+  (pass-if "scale with many leading zeros"
+    (string=? "0.0123" (format #f "~,,1f" "0.00123"))))
 
 ;;;
 ;;; ~h
-- 
2.33.0