all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Checking for loss of information on integer conversion
@ 2018-02-18  1:27 Paul Eggert
  2018-02-18 22:31 ` Juliusz Chroboczek
       [not found] ` <83y3jq9q4m.fsf@gnu.org>
  0 siblings, 2 replies; 25+ messages in thread
From: Paul Eggert @ 2018-02-18  1:27 UTC (permalink / raw)
  To: Emacs Development

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

Bug#30408 reminded me of a problem that's bitten me before, which is that Emacs 
Lisp reading and printing sometimes loses information when converting from 
integers to strings or vice versa, and this loss of information can lead to real 
problems. Without going to bignums we can't fix this problem nicely in general; 
however, Emacs can do a better job of signaling an error when information is 
lost in the current implementation. I've proposed a patch here:

https://debbugs.gnu.org/30408#16

and am attaching it to this email for convenience.

Briefly, the patch is twofold. First, it causes calls like (format "%d" 
18446744073709551616) to return the mathematically-correct value 
"18446744073709551616" instead of silently returning the 
mathematically-incorrect value "9223372036854775807" as they do now. If the 
function cannot return the correct value due to an implementation limit -- e.g., 
(format "%x" 18446744073709551616) -- the patch causes the function to signal an 
overflow.

Second, although Emacs still reads large integers like 18446744073709551616 as 
if they were floating-point, it now signals an error if information is lost in 
the process. For example, the number 18446744073709551615 now causes the reader 
to signal an error, since it cannot be represented exactly either as a fixnum or 
as a floating-point number. If you want inexact representation, you can append 
".0" or "e0" to the integer.

As these are incompatible changes to Emacs I thought I'd mention them on this list.

Another possibility would be for Emacs to signal an error when reading any 
integer that does not fit in fixnum bounds. That would be a bigger change, though.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-losing-info-when-converting-integers.patch --]
[-- Type: text/x-patch; name="0001-Avoid-losing-info-when-converting-integers.patch", Size: 8127 bytes --]

From e1865be990e1a520feddc07507a71916d097d633 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 17 Feb 2018 16:45:17 -0800
Subject: [PATCH] Avoid losing info when converting integers

This fixes some glitches with large integers (Bug#30408).
* doc/lispref/numbers.texi (Integer Basics): Say that
decimal integers out of fixnum range must be representable
exactly as floating-point.
* etc/NEWS: Mention this.
* src/data.c (syms_of_data): Add Qinexact_error.
* src/editfns.c (styled_format): Use %.0f when formatting %d or %i
values outside machine integer range, to avoid losing info.
Signal an error for %o or %x values that are too large to be
formatted, to avoid losing info.
* src/lread.c (string_to_number): When converting an integer-format
string to floating-point, signal an error if info is lost.
---
 doc/lispref/numbers.texi |  8 +++--
 etc/NEWS                 |  9 +++++
 src/data.c               |  1 +
 src/editfns.c            | 93 ++++++++++++++++++++----------------------------
 src/lread.c              | 14 ++++++++
 5 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/doc/lispref/numbers.texi b/doc/lispref/numbers.texi
index e692ee1cc2..252aafd8fd 100644
--- a/doc/lispref/numbers.texi
+++ b/doc/lispref/numbers.texi
@@ -53,9 +53,11 @@ Integer Basics
 chapter assume the minimum integer width of 30 bits.
 @cindex overflow
 
-  The Lisp reader reads an integer as a sequence of digits with optional
-initial sign and optional final period.  An integer that is out of the
-Emacs range is treated as a floating-point number.
+  The Lisp reader can read an integer as a nonempty sequence of
+decimal digits with optional initial sign and optional final period.
+A decimal integer that is out of the Emacs range is treated as
+floating-point if it can be represented exactly as a floating-point
+number.
 
 @example
  1               ; @r{The integer 1.}
diff --git a/etc/NEWS b/etc/NEWS
index 8db638e5ed..36cbcf6500 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -248,6 +248,12 @@ as new-style, bind the new variable 'force-new-style-backquotes' to t.
 'cl-struct-define' whose name clashes with a builtin type (e.g.,
 'integer' or 'hash-table') now signals an error.
 
+** When formatting a floating-point number as an octal or hexadecimal
+integer, Emacs now signals an error if the number is too large for the
+implementation to format.  When reading an integer outside Emacs
+fixnum range, Emacs now signals an error if the integer cannot be
+represented exactly as a floating-point number.  See Bug#30408.
+
 \f
 * Lisp Changes in Emacs 27.1
 
@@ -289,6 +295,9 @@ remote systems, which support this check.
 If the optional third argument is non-nil, 'make-string' will produce
 a multibyte string even if its second argument is an ASCII character.
 
+** (format "%d" X) no longer mishandles floating-point X values that
+do not fit in a machine integer (Bug#30408).
+
 ** New JSON parsing and serialization functions 'json-serialize',
 'json-insert', 'json-parse-string', and 'json-parse-buffer'.  These
 are implemented in C using the Jansson library.
diff --git a/src/data.c b/src/data.c
index 72abfefb01..8856583f13 100644
--- a/src/data.c
+++ b/src/data.c
@@ -3729,6 +3729,7 @@ syms_of_data (void)
   DEFSYM (Qrange_error, "range-error");
   DEFSYM (Qdomain_error, "domain-error");
   DEFSYM (Qsingularity_error, "singularity-error");
+  DEFSYM (Qinexact_error, "inexact-error");
   DEFSYM (Qoverflow_error, "overflow-error");
   DEFSYM (Qunderflow_error, "underflow-error");
 
diff --git a/src/editfns.c b/src/editfns.c
index 96bb271b2d..d26549ddb8 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4563,32 +4563,30 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		 and with pM inserted for integer formats.
 		 At most two flags F can be specified at once.  */
 	      char convspec[sizeof "%FF.*d" + max (INT_AS_LDBL, pMlen)];
-	      {
-		char *f = convspec;
-		*f++ = '%';
-		/* MINUS_FLAG and ZERO_FLAG are dealt with later.  */
-		*f = '+'; f +=  plus_flag;
-		*f = ' '; f += space_flag;
-		*f = '#'; f += sharp_flag;
-                *f++ = '.';
-                *f++ = '*';
-		if (float_conversion)
-		  {
-		    if (INT_AS_LDBL)
-		      {
-			*f = 'L';
-			f += INTEGERP (arg);
-		      }
-		  }
-		else if (conversion != 'c')
-		  {
-		    memcpy (f, pMd, pMlen);
-		    f += pMlen;
-		    zero_flag &= ! precision_given;
-		  }
-		*f++ = conversion;
-		*f = '\0';
-	      }
+	      char *f = convspec;
+	      *f++ = '%';
+	      /* MINUS_FLAG and ZERO_FLAG are dealt with later.  */
+	      *f = '+'; f +=  plus_flag;
+	      *f = ' '; f += space_flag;
+	      *f = '#'; f += sharp_flag;
+	      *f++ = '.';
+	      *f++ = '*';
+	      if (float_conversion)
+		{
+		  if (INT_AS_LDBL)
+		    {
+		      *f = 'L';
+		      f += INTEGERP (arg);
+		    }
+		}
+	      else if (conversion != 'c')
+		{
+		  memcpy (f, pMd, pMlen);
+		  f += pMlen;
+		  zero_flag &= ! precision_given;
+		}
+	      *f++ = conversion;
+	      *f = '\0';
 
 	      int prec = -1;
 	      if (precision_given)
@@ -4630,29 +4628,18 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		}
 	      else if (conversion == 'd' || conversion == 'i')
 		{
-		  /* For float, maybe we should use "%1.0f"
-		     instead so it also works for values outside
-		     the integer range.  */
-		  printmax_t x;
 		  if (INTEGERP (arg))
-		    x = XINT (arg);
+		    {
+		      printmax_t x = XINT (arg);
+		      sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
+		    }
 		  else
 		    {
-		      double d = XFLOAT_DATA (arg);
-		      if (d < 0)
-			{
-			  x = TYPE_MINIMUM (printmax_t);
-			  if (x < d)
-			    x = d;
-			}
-		      else
-			{
-			  x = TYPE_MAXIMUM (printmax_t);
-			  if (d < x)
-			    x = d;
-			}
+		      strcpy (f - pMlen - 1, "f");
+		      prec = 0;
+		      double x = XFLOAT_DATA (arg);
+		      sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
 		    }
-		  sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
 		}
 	      else
 		{
@@ -4663,22 +4650,18 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		  else
 		    {
 		      double d = XFLOAT_DATA (arg);
-		      if (d < 0)
-			x = 0;
-		      else
-			{
-			  x = TYPE_MAXIMUM (uprintmax_t);
-			  if (d < x)
-			    x = d;
-			}
+		      if (! (0 <= d && d < TYPE_MAXIMUM (uprintmax_t)))
+			xsignal1 (Qoverflow_error, arg);
+		      x = d;
 		    }
 		  sprintf_bytes = sprintf (sprintf_buf, convspec, prec, x);
 		}
 
 	      /* Now the length of the formatted item is known, except it omits
 		 padding and excess precision.  Deal with excess precision
-		 first.  This happens only when the format specifies
-		 ridiculously large precision.  */
+		 first.  This happens when the format specifies
+		 ridiculously large precision, or when %d or %i has
+		 nonzero precision and formats a float.  */
 	      ptrdiff_t excess_precision
 		= precision_given ? precision - prec : 0;
 	      ptrdiff_t leading_zeros = 0, trailing_zeros = 0;
diff --git a/src/lread.c b/src/lread.c
index d009bd0cd2..cfeaac8030 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -3794,6 +3794,20 @@ string_to_number (char const *string, int base, bool ignore_trailing)
   if (! value)
     value = atof (string + signedp);
 
+  if (! float_syntax)
+    {
+      /* Check that converting the integer-format STRING to a
+	 floating-point number does not lose info.  See Bug#30408.  */
+      char const *bp = string + signedp;
+      while (*bp == '0')
+	bp++;
+      char checkbuf[DBL_MAX_10_EXP + 2];
+      int checkbuflen = sprintf (checkbuf, "%.0f", value);
+      if (! (cp - bp - !!(state & DOT_CHAR) == checkbuflen
+	     && memcmp (bp, checkbuf, checkbuflen) == 0))
+	xsignal1 (Qinexact_error, build_string (string));
+    }
+
   return make_float (negative ? -value : value);
 }
 
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-03-29 18:09 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-18  1:27 Checking for loss of information on integer conversion Paul Eggert
2018-02-18 22:31 ` Juliusz Chroboczek
2018-02-18 22:41   ` Stefan Monnier
2018-02-18 23:46     ` Juliusz Chroboczek
2018-02-19  1:47       ` Stefan Monnier
2018-02-19  2:22         ` Paul Eggert
2018-02-19  3:20           ` Drew Adams
2018-02-19 15:05       ` Richard Stallman
2018-02-22 16:31         ` Juliusz Chroboczek
2018-02-22 17:01           ` Eli Zaretskii
2018-02-22 19:31             ` Stefan Monnier
2018-02-23  9:49           ` Richard Stallman
2018-02-19  6:03   ` John Wiegley
     [not found] ` <83y3jq9q4m.fsf@gnu.org>
2018-02-18 20:04   ` Paul Eggert
2018-02-18 20:24     ` Eli Zaretskii
2018-03-09  5:00       ` bug#30408: " Paul Eggert
2018-03-09  8:22         ` Eli Zaretskii
2018-03-21 19:13           ` Paul Eggert
2018-03-21 19:29             ` Eli Zaretskii
2018-02-18 20:24     ` Eli Zaretskii
2018-02-18 21:52     ` Drew Adams
2018-02-18 20:04   ` Paul Eggert
2018-03-27 23:19   ` Paul Eggert
2018-03-29 11:11     ` Eli Zaretskii
2018-03-29 18:09       ` Paul Eggert

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.