* bug#31938: regression in (format "%d" -0.0)
@ 2018-06-22 18:01 Paul Pogonyshev
2018-06-23 10:21 ` bug#31938: followup Paul Pogonyshev
0 siblings, 1 reply; 4+ messages in thread
From: Paul Pogonyshev @ 2018-06-22 18:01 UTC (permalink / raw)
To: 31938
Severity: serious
Emacs 24-26:
(format "%d" -0.0) => "0"
Emacs 27:
(format "%d" -0.0) => "-0"
I think this is a very important regression, because it happens in
very low-level code and can lead to unpredictable results in certain
special cases. Caught with real-world `datetime' package: on Emacs 27
all its regression tests fail because of this change, on Emacs 24-26
they pass.
In the library, -0.0 comes from the fact that `mod' built-in can
return it as a result (it is probably fine, because it is _equal_ to
0.0, so it is not negative, even if it looks like it).
I.e. real code looks more like this:
(let ((x -2.0)) (format "%d" (mod x 2)))
which gives "-0" and is a regression in Emacs 27.
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#31938: followup
2018-06-22 18:01 bug#31938: regression in (format "%d" -0.0) Paul Pogonyshev
@ 2018-06-23 10:21 ` Paul Pogonyshev
2018-06-23 11:46 ` Eli Zaretskii
0 siblings, 1 reply; 4+ messages in thread
From: Paul Pogonyshev @ 2018-06-23 10:21 UTC (permalink / raw)
To: 31938
Another difference. In Emacs 24-26:
(format "%d" 0.9) => "0"
In Emacs 27:
(format "%d" 0.9) => "1"
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#31938: followup
2018-06-23 10:21 ` bug#31938: followup Paul Pogonyshev
@ 2018-06-23 11:46 ` Eli Zaretskii
2018-06-25 19:38 ` Paul Eggert
0 siblings, 1 reply; 4+ messages in thread
From: Eli Zaretskii @ 2018-06-23 11:46 UTC (permalink / raw)
To: Paul Pogonyshev, Paul Eggert; +Cc: 31938
> From: Paul Pogonyshev <pogonyshev@gmail.com>
> Date: Sat, 23 Jun 2018 12:21:00 +0200
>
> Another difference. In Emacs 24-26:
>
> (format "%d" 0.9) => "0"
>
> In Emacs 27:
>
> (format "%d" 0.9) => "1"
This looks like the (unintended?) result of 80e145fc9. CC'ing Paul
who made that change.
(FWIW, I'm not sure why this is a problem.)
P.S. Please don't change the Subject when you post to a bug, keep the
original Subject you used when reporting it. That makes looking for
related messages in a mailer easier, at least for me.
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#31938: followup
2018-06-23 11:46 ` Eli Zaretskii
@ 2018-06-25 19:38 ` Paul Eggert
0 siblings, 0 replies; 4+ messages in thread
From: Paul Eggert @ 2018-06-25 19:38 UTC (permalink / raw)
To: Eli Zaretskii, Paul Pogonyshev; +Cc: 31938-done
[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]
On 06/23/2018 04:46 AM, Eli Zaretskii wrote:
>> From: Paul Pogonyshev <pogonyshev@gmail.com>
>> Date: Sat, 23 Jun 2018 12:21:00 +0200
>>
>> Another difference. In Emacs 24-26:
>>
>> (format "%d" 0.9) => "0"
>>
>> In Emacs 27:
>>
>> (format "%d" 0.9) => "1"
> This looks like the (unintended?) result of 80e145fc9.
I vaguely recall relying on the Elisp manual, which says that %d must be
applied only to integers, and I guess I figured the semantics didn't
matter so that the simplest implementation (thought that rounding and
keeping -0) would satisfy the doc and anyway would be more helpful than
truncating and discarding the sign of -0. But now that I reread the
manual I see that it says %d must signal an error when given a float,
which is obviously not the behavior, so the manual is wrong. And now
that this issue has come up I notice that 80e145fc9 caused %d to behave
inconsistently with %x; the latter truncates and converts -0 to 0
whereas the former rounds and keeps -0. This inconsistency is not good.
To try to improve this, I installed the attached into master to change
'format' to be more compatible with Emacs 26 etc. and am boldly closing
the bug report.
[-- Attachment #2: 0001-format-d-F-now-truncates-floating-F.patch --]
[-- Type: text/x-patch, Size: 5175 bytes --]
From 32a9df02ac3c5b12d7ff6efa0c3924d740d70268 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 25 Jun 2018 12:21:40 -0700
Subject: [PATCH] (format "%d" F) now truncates floating F
Problem reported by Paul Pogonyshev (Bug#31938).
* src/editfns.c: Include math.h, for trunc.
(styled_format): For %d, truncate floating-point numbers and
convert -0 to 0, going back to how Emacs 26 did things.
* doc/lispref/strings.texi (Formatting Strings):
Document behavior of %o, %d, %x, %X on floating-point numbers.
* src/floatfns.c (trunc) [!HAVE_TRUNC]: Rename from emacs_trunc
and make it an extern function, so that editfns.c can use it.
All callers changed.
* test/src/editfns-tests.el (format-%d-float): New test.
---
doc/lispref/strings.texi | 11 ++++++++---
src/editfns.c | 7 +++++++
src/floatfns.c | 13 +++++--------
src/lisp.h | 5 ++++-
test/src/editfns-tests.el | 8 ++++++++
5 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 70ba1aa613..026ba749cb 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -922,18 +922,23 @@ Formatting Strings
@item %o
@cindex integer to octal
Replace the specification with the base-eight representation of an
-unsigned integer.
+unsigned integer. 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.
@item %d
Replace the specification with the base-ten representation of a signed
-integer.
+integer. The object can also be a floating-point number that is
+formatted as an integer, dropping any fraction.
@item %x
@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.
+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.
@item %c
Replace the specification with the character which is the value given.
diff --git a/src/editfns.c b/src/editfns.c
index 30d585cd01..7d032a7ca4 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -47,6 +47,7 @@ along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */
#include <errno.h>
#include <float.h>
#include <limits.h>
+#include <math.h>
#ifdef HAVE_TIMEZONE_T
# include <sys/param.h>
@@ -4671,6 +4672,12 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
{
strcpy (f - pMlen - 1, "f");
double x = XFLOAT_DATA (arg);
+
+ /* Truncate and then convert -0 to 0, to be more
+ consistent with %x etc.; see Bug#31938. */
+ x = trunc (x);
+ x = x ? x : 0;
+
sprintf_bytes = sprintf (sprintf_buf, convspec, 0, x);
char c0 = sprintf_buf[0];
bool signedp = ! ('0' <= c0 && c0 <= '9');
diff --git a/src/floatfns.c b/src/floatfns.c
index ec0349fbf4..e7d404a84e 100644
--- a/src/floatfns.c
+++ b/src/floatfns.c
@@ -435,11 +435,9 @@ emacs_rint (double d)
}
#endif
-#ifdef HAVE_TRUNC
-#define emacs_trunc trunc
-#else
-static double
-emacs_trunc (double d)
+#ifndef HAVE_TRUNC
+double
+trunc (double d)
{
return (d < 0 ? ceil : floor) (d);
}
@@ -482,8 +480,7 @@ Rounds ARG toward zero.
With optional DIVISOR, truncate ARG/DIVISOR. */)
(Lisp_Object arg, Lisp_Object divisor)
{
- return rounding_driver (arg, divisor, emacs_trunc, truncate2,
- "truncate");
+ return rounding_driver (arg, divisor, trunc, truncate2, "truncate");
}
@@ -543,7 +540,7 @@ DEFUN ("ftruncate", Fftruncate, Sftruncate, 1, 1, 0,
{
CHECK_FLOAT (arg);
double d = XFLOAT_DATA (arg);
- d = emacs_trunc (d);
+ d = trunc (d);
return make_float (d);
}
\f
diff --git a/src/lisp.h b/src/lisp.h
index d0c52d8567..8c884dce15 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3425,8 +3425,11 @@ extern Lisp_Object string_make_unibyte (Lisp_Object);
extern void syms_of_fns (void);
/* Defined in floatfns.c. */
-extern void syms_of_floatfns (void);
+#ifndef HAVE_TRUNC
+extern double trunc (double);
+#endif
extern Lisp_Object fmod_float (Lisp_Object x, Lisp_Object y);
+extern void syms_of_floatfns (void);
/* Defined in fringe.c. */
extern void syms_of_fringe (void);
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index 1ed0bd5bba..c828000bb4 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -176,6 +176,14 @@ transpose-test-get-byte-positions
(should-error (format "%o" -1e-37)
:type 'overflow-error))
+;; Bug#31938
+(ert-deftest format-%d-float ()
+ (should (string-equal (format "%d" -1.1) "-1"))
+ (should (string-equal (format "%d" -0.9) "0"))
+ (should (string-equal (format "%d" -0.0) "0"))
+ (should (string-equal (format "%d" 0.0) "0"))
+ (should (string-equal (format "%d" 0.9) "0"))
+ (should (string-equal (format "%d" 1.1) "1")))
;;; Check format-time-string with various TZ settings.
;;; Use only POSIX-compatible TZ values, since the tests should work
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-06-25 19:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-22 18:01 bug#31938: regression in (format "%d" -0.0) Paul Pogonyshev
2018-06-23 10:21 ` bug#31938: followup Paul Pogonyshev
2018-06-23 11:46 ` Eli Zaretskii
2018-06-25 19:38 ` 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.