Subject: bug#32252: closed (Re: bug#32252: [PATCH] %o and %x now format signed numbers)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 26 Jul 2018 07:45:02 +0000

Your bug report

#32252: [PATCH] %o and %x now format signed numbers

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
32252: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=32252
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#32252: [PATCH] %o and %x now format signed numbers
From: Paul Eggert <[email-masked]>
To: Eli Zaretskii <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Thu, 26 Jul 2018 00:44:34 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Eli Zaretskii wrote:
>> Cc: [email-masked], [email-masked]
>> From: Paul Eggert <[email-masked]>
>>
>> That being said, it appears that there are enough qualms about the
>> change that I plan to install it with the new variable set the other
>> way. I.e., the default will be the current behavior and people can set
>> the new variable to get the new behavior, to try it this behavior out to
>> see whether they have problems.
> 
> Thank you.

OK, I installed the attached patch which does that, and am closing the bug 
report. I plan to run with binary-as-unsigned set to nil and to report any 
problems I encounter. I encourage others (particularly skeptics :-) to do the same.
From 4a56ca5bbfabbb9c581828cd91648346e6b03844 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 26 Jul 2018 00:34:10 -0700
Subject: [PATCH] %o and %x can now format signed integers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Optionally treat integers as signed numbers with %o
and %x format specifiers, instead of treating them as
a machine-dependent two’s complement representation.
This option is more machine-independent, allows formats
like "#x%x" to be useful for reading later, and is
better-insulated for future changes involving bignums.
Setting the new variable ‘binary-as-unsigned’ to nil
enables the new behavior (Bug#32252).
This is a simplified version of the change proposed in:
https://lists.gnu.org/r/emacs-devel/2018-07/msg00763.html
I simplified that proposal by omitting bitwidth modifiers, as
I could not find an any example uses in the Emacs source code
that needed them and doing them correctly would have been
quite a bit more work for apparently little benefit.
* doc/lispref/strings.texi (Formatting Strings):
Document that %x and %o format negative integers in a
platform-dependent way.  Also, document how to format
numbers so that the same values can be read back in.
* etc/NEWS: Document the change.
* src/editfns.c (styled_format): Treat integers as signed
numbers even with %o and %x, if binary-as-unsigned is nil.
Support the + and space flags with %o and %x, since they’re
about signs.
(syms_of_editfns): New variable binary-as-unsigned.
* test/src/editfns-tests.el (read-large-integer):
Test that maximal integers can be read after printing
with all integer formats, if binary-as-unsigned is nil.
---
 doc/lispref/strings.texi  | 17 +++++++++++++++--
 etc/NEWS                  |  9 +++++++++
 src/editfns.c             | 43 ++++++++++++++++++++++++++++++++++++++-----
 test/src/editfns-tests.el | 10 ++++++----
 4 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 2fff3c7..3558f17 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -922,7 +922,8 @@ Formatting Strings
 @item %o
 @cindex integer to octal
 Replace the specification with the base-eight representation of an
-unsigned integer.  The object can also be a nonnegative floating-point
+integer.  Negative integers are formatted in a platform-dependent
+way.  The object can also be a nonnegative floating-point
 number that is formatted as an integer, dropping any fraction, if the
 integer does not exceed machine limits.
 
@@ -935,7 +936,8 @@ Formatting Strings
 @itemx %X
 @cindex integer to hexadecimal
 Replace the specification with the base-sixteen representation of an
-unsigned integer.  @samp{%x} uses lower case and @samp{%X} uses upper
+integer.  Negative integers are formatted in a platform-dependent
+way.  @samp{%x} uses lower case and @samp{%X} uses upper
 case.  The object can also be a nonnegative floating-point number that
 is formatted as an integer, dropping any fraction, if the integer does
 not exceed machine limits.
@@ -1108,6 +1110,17 @@ Formatting Strings
 precision is what the local library functions of the @code{printf}
 family produce.
 
+@cindex formatting numbers for rereading later
+  If you plan to use @code{read} later on the formatted string to
+retrieve a copy of the formatted value, use a specification that lets
+@code{read} reconstruct the value.  To format numbers in this
+reversible way you can use @samp{%s} and @samp{%S}, to format just
+integers you can also use @samp{%d}, and to format just nonnegative
+integers you can also use @samp{#x%x} and @samp{#o%o}.  Other formats
+may be problematic; for example, @samp{%d} and @samp{%g} can mishandle
+NaNs and can lose precision and type, and @samp{#x%x} and @samp{#o%o}
+can mishandle negative integers.  @xref{Input Functions}.
+
 @node Case Conversion
 @section Case Conversion in Lisp
 @cindex upper case
diff --git a/etc/NEWS b/etc/NEWS
index 995ceb6..089fc40 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -812,6 +812,15 @@ between two strings.
 ** 'print-quoted' now defaults to t, so if you want to see
 (quote x) instead of 'x you will have to bind it to nil where applicable.
 
++++
+** Numbers formatted via %o or %x may now be formatted as signed integers.
+This avoids problems in calls like (read (format "#x%x" -1)), and is
+more compatible with bignums, a planned feature.  To get this
+behavior, set the experimental variable binary-as-unsigned to nil,
+and if the new behavior breaks your code please email
[email-masked].  Because %o and %x can now format signed
+integers, they now support the + and space flags.
+
 ** To avoid confusion caused by "smart quotes", the reader signals an
 error when reading Lisp symbols which begin with one of the following
 quotation characters: ‘’‛“”‟〞＂＇.  A symbol beginning with such a
diff --git a/src/editfns.c b/src/editfns.c
index 1d6040d..df25721 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4196,8 +4196,8 @@ contain either numbered or unnumbered %-sequences but not both, except
 that %% can be mixed with numbered %-sequences.
 
 The + flag character inserts a + before any nonnegative number, while a
-space inserts a space before any nonnegative number; these flags only
-affect %d, %e, %f, and %g sequences, and the + flag takes precedence.
+space inserts a space before any nonnegative number; these flags
+affect only numeric %-sequences, and the + flag takes precedence.
 The - and 0 flags affect the width specifier, as described below.
 
 The # flag means to use an alternate display form for %o, %x, %X, %e,
@@ -4736,10 +4736,22 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		}
 	      else
 		{
-		  /* Don't sign-extend for octal or hex printing.  */
 		  uprintmax_t x;
+		  bool negative;
 		  if (INTEGERP (arg))
-		    x = XUINT (arg);
+		    {
+		      if (binary_as_unsigned)
+			{
+			  x = XUINT (arg);
+			  negative = false;
+			}
+		      else
+			{
+			  EMACS_INT i = XINT (arg);
+			  negative = i < 0;
+			  x = negative ? -i : i;
+			}
+		    }
 		  else
 		    {
 		      double d = XFLOAT_DATA (arg);
@@ -4747,8 +4759,13 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		      if (! (0 <= d && d < uprintmax + 1))
 			xsignal1 (Qoverflow_error, arg);
 		      x = d;
+		      negative = false;
 		    }
-		  sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
+		  sprintf_buf[0] = negative ? '-' : plus_flag ? '+' : ' ';
+		  bool signedp = negative | plus_flag | space_flag;
+		  sprintf_bytes = sprintf (sprintf_buf + signedp,
+					   convspec, prec, x);
+		  sprintf_bytes += signedp;
 		}
 
 	      /* Now the length of the formatted item is known, except it omits
@@ -5558,6 +5575,22 @@ functions if all the text being accessed has this property.  */);
   DEFVAR_LISP ("operating-system-release", Voperating_system_release,
 	       doc: /* The release of the operating system Emacs is running on.  */);
 
+  DEFVAR_BOOL ("binary-as-unsigned",
+	       binary_as_unsigned,
+	       doc: /* Non-nil means `format' %x and %o treat integers as unsigned.
+This has machine-dependent results.  Nil means to treat integers as
+signed, which is portable; for example, if N is a negative integer,
+(read (format "#x%x") N) returns N only when this variable is nil.
+
+This variable is experimental; email [email-masked] if you need
+it to be non-nil.  */);
+  /* For now, default to true if bignums exist, false in traditional Emacs.  */
+#ifdef lisp_h_FIXNUMP
+  binary_as_unsigned = true;
+#else
+  binary_as_unsigned = false;
+#endif
+
   defsubr (&Spropertize);
   defsubr (&Schar_equal);
   defsubr (&Sgoto_char);
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index c828000..2951270 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -165,10 +165,12 @@ transpose-test-get-byte-positions
                 :type 'overflow-error)
   (should-error (read (substring (format "%d" most-negative-fixnum) 1))
                 :type 'overflow-error)
-  (should-error (read (format "#x%x" most-negative-fixnum))
-                :type 'overflow-error)
-  (should-error (read (format "#o%o" most-negative-fixnum))
-                :type 'overflow-error)
+  (let ((binary-as-unsigned nil))
+    (dolist (fmt '("%d" "%s" "#o%o" "#x%x"))
+      (dolist (val (list most-negative-fixnum (1+ most-negative-fixnum)
+                         -1 0 1
+                         (1- most-positive-fixnum) most-positive-fixnum))
+        (should (eq val (read (format fmt val)))))))
   (should-error (read (format "#32rG%x" most-positive-fixnum))
                 :type 'overflow-error))
 
-- 
2.7.4

Subject: [PATCH] %o and %x now format signed numbers
From: Paul Eggert <[email-masked]>
To: [email-masked]
Cc: Paul Eggert <[email-masked]>
Date: Mon, 23 Jul 2018 12:12:50 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Treat integers as signed numbers when using the %o
and %x format specifiers, instead of treating them as
a machine-dependent two’s complement representation.
This should make Emacs more machine-independent, and
better-insulated for future changes involving bignums.
The new compatibility variable ‘binary-as-unsigned’
enables the old machine-dependent behavior.
This is a simplified version of the change proposed in:
https://lists.gnu.org/r/emacs-devel/2018-07/msg00763.html
I simplified that proposal by omitting bitwidth modifiers, as
I could not find an any example uses in the Emacs source code
that needed them and doing them correctly would have been
quite a bit more work for apparently little benefit.
* doc/lispref/strings.texi (Formatting Strings):
* etc/NEWS: Document change.
* src/editfns.c (styled_format): Treat integers as signed
numbers even with %o and %x.  Support the + and space
flags with %o and %x, since they’re about signs.
(syms_of_editfns): New compatibility variable binary-as-signed.
* test/src/editfns-tests.el (read-large-integer):
Test that maximal integers can be read after printing
with all integer formats.
(format-%o-invalid-float): Adjust test to match new
behavior, which allows negative octal.
---
 doc/lispref/strings.texi  |  6 +++---
 etc/NEWS                  |  8 ++++++++
 src/editfns.c             | 42 ++++++++++++++++++++++++++++++++++-----
 test/src/editfns-tests.el | 11 +++++-----
 4 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 2fff3c7c75..582561fe42 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -922,7 +922,7 @@ Formatting Strings
 @item %o
 @cindex integer to octal
 Replace the specification with the base-eight representation of an
-unsigned integer.  The object can also be a nonnegative floating-point
+integer.  The object can also be a floating-point
 number that is formatted as an integer, dropping any fraction, if the
 integer does not exceed machine limits.
 
@@ -935,8 +935,8 @@ Formatting Strings
 @itemx %X
 @cindex integer to hexadecimal
 Replace the specification with the base-sixteen representation of an
-unsigned integer.  @samp{%x} uses lower case and @samp{%X} uses upper
-case.  The object can also be a nonnegative floating-point number that
+integer.  @samp{%x} uses lower case and @samp{%X} uses upper
+case.  The object can also be a floating-point number that
 is formatted as an integer, dropping any fraction, if the integer does
 not exceed machine limits.
 
diff --git a/etc/NEWS b/etc/NEWS
index fc2a5d4c03..b735cf49b3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -697,6 +697,14 @@ To get the old behavior, set the new, experimental variable
 read-integer-overflow-as-float to t and please email
 [email-masked] if you need that.  (Bug#30408).
 
++++
+** Numbers formatted via %o or %x are now formatted as signed integers.
+Formerly they were formatted using machine-dependent two's complement
+representations.  To get the old behavior, set the new, experimental
+variable binary-as-unsigned to t and please email [email-masked]
+if you need that.  Because %o and %x now format signed integers, they
+now support the + and space flags.
+
 ---
 ** Some functions and variables obsolete since Emacs 22 have been removed:
 archive-mouse-extract, assoc-ignore-case, assoc-ignore-representation,
diff --git a/src/editfns.c b/src/editfns.c
index 09f836c3eb..a8fc499d63 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4196,8 +4196,8 @@ contain either numbered or unnumbered %-sequences but not both, except
 that %% can be mixed with numbered %-sequences.
 
 The + flag character inserts a + before any nonnegative number, while a
-space inserts a space before any nonnegative number; these flags only
-affect %d, %e, %f, and %g sequences, and the + flag takes precedence.
+space inserts a space before any nonnegative number; these flags
+affect only numeric %-sequences, and the + flag takes precedence.
 The - and 0 flags affect the width specifier, as described below.
 
 The # flag means to use an alternate display form for %o, %x, %X, %e,
@@ -4736,19 +4736,42 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		}
 	      else
 		{
-		  /* Don't sign-extend for octal or hex printing.  */
 		  uprintmax_t x;
+		  bool negative;
 		  if (INTEGERP (arg))
-		    x = XUINT (arg);
+		    {
+		      EMACS_INT i;
+		      if (binary_as_unsigned)
+			{
+			  i = XUINT (arg);
+			  negative = false;
+			}
+		      else
+			{
+			  i = XINT (arg);
+			  negative = i < 0;
+			  if (negative)
+			    i = -i;
+			}
+		      x = i;
+		    }
 		  else
 		    {
 		      double d = XFLOAT_DATA (arg);
 		      double uprintmax = TYPE_MAXIMUM (uprintmax_t);
+		      negative = d < 0;
+		      if (negative)
+			d = -d;
 		      if (! (0 <= d && d < uprintmax + 1))
 			xsignal1 (Qoverflow_error, arg);
 		      x = d;
+		      negative &= x != 0;
 		    }
-		  sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
+		  sprintf_buf[0] = negative ? '-' : plus_flag ? '+' : ' ';
+		  bool signedp = negative | plus_flag | space_flag;
+		  sprintf_bytes = sprintf (sprintf_buf + signedp,
+					   convspec, prec, x);
+		  sprintf_bytes += signedp;
 		}
 
 	      /* Now the length of the formatted item is known, except it omits
@@ -5558,6 +5581,15 @@ functions if all the text being accessed has this property.  */);
   DEFVAR_LISP ("operating-system-release", Voperating_system_release,
 	       doc: /* The release of the operating system Emacs is running on.  */);
 
+  DEFVAR_BOOL ("binary-as-unsigned",
+	       binary_as_unsigned,
+	       doc: /* Non-nil means `format' %x and %o treat numbers as unsigned.
+Nil (the default) means to treat them as signed.
+Treating them as unsigned has machine-dependent results.
+
+This variable is experimental; email [email-masked] if you need it.  */);
+  binary_as_unsigned = false;
+
   defsubr (&Spropertize);
   defsubr (&Schar_equal);
   defsubr (&Sgoto_char);
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index c828000bb4..57a2081fc5 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -165,15 +165,16 @@ transpose-test-get-byte-positions
                 :type 'overflow-error)
   (should-error (read (substring (format "%d" most-negative-fixnum) 1))
                 :type 'overflow-error)
-  (should-error (read (format "#x%x" most-negative-fixnum))
-                :type 'overflow-error)
-  (should-error (read (format "#o%o" most-negative-fixnum))
-                :type 'overflow-error)
+  (dolist (fmt '("%d" "%s" "#o%o" "#x%x"))
+    (dolist (val (list most-negative-fixnum (1+ most-negative-fixnum)
+                       -1 0 1
+                       (1- most-positive-fixnum) most-positive-fixnum))
+      (should (eq val (read (format fmt val))))))
   (should-error (read (format "#32rG%x" most-positive-fixnum))
                 :type 'overflow-error))
 
 (ert-deftest format-%o-invalid-float ()
-  (should-error (format "%o" -1e-37)
+  (should-error (format "%o" -1e+INF)
                 :type 'overflow-error))
 
 ;; Bug#31938
-- 
2.17.1