From 37940b347052418f0589bd52b06e56fffb594ea2 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 6 Mar 2017 15:14:32 -0800 Subject: [PATCH] min and max now return one of their arguments * doc/lispref/numbers.texi (Comparison of Numbers): * etc/NEWS: Document this. * src/data.c (Amax, Amin): Remove constants. All uses removed. (minmax_driver): New function. (Fmax, Fmin): Use it instead of arith_driver. * test/src/data-tests.el (data-tests-max, data-tests-min): New tests. --- doc/lispref/numbers.texi | 6 +----- etc/NEWS | 7 +++++++ src/data.c | 43 +++++++++++++++++++--------------------- test/src/data-tests.el | 20 +++++++++++++++++++ 4 files changed, 48 insertions(+), 28 deletions(-) diff --git a/doc/lispref/numbers.texi b/doc/lispref/numbers.texi index deae5fd85dc..3fdc94169bd 100644 --- a/doc/lispref/numbers.texi +++ b/doc/lispref/numbers.texi @@ -427,8 +427,6 @@ the following argument. It returns @code{t} if so, @code{nil} otherwise. @defun max number-or-marker &rest numbers-or-markers This function returns the largest of its arguments. -If any of the arguments is floating point, the value is returned -as floating point, even if it was given as an integer. @example (max 20) @@ -436,14 +434,12 @@ as floating point, even if it was given as an integer. (max 1 2.5) @result{} 2.5 (max 1 3 2.5) - @result{} 3.0 + @result{} 3 @end example @end defun @defun min number-or-marker &rest numbers-or-markers This function returns the smallest of its arguments. -If any of the arguments is floating point, the value is returned -as floating point, even if it was given as an integer. @example (min -4 1) diff --git a/etc/NEWS b/etc/NEWS index 8f7356f3e03..ce20dfb15d7 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -803,6 +803,13 @@ Unicode horizontal whitespace as defined in the Unicode Technical Standard #18. If you only want to match space and tab, use [ \t] instead. ++++ +** 'min' and 'max' now always return one of their arguments. +Formerly, they returned a floating-point value if any argument was +floating-point, which was sometimes numerically incorrect. For +example, (min most-positive-fixnum (+ 1.0 most-positive-fixnum)) now +always returns its first argument instead of its second. + * Lisp Changes in Emacs 26.1 diff --git a/src/data.c b/src/data.c index 66f4c9c738d..ae88e3f8aa5 100644 --- a/src/data.c +++ b/src/data.c @@ -2719,9 +2719,7 @@ enum arithop Adiv, Alogand, Alogior, - Alogxor, - Amax, - Amin + Alogxor }; static Lisp_Object float_arith_driver (double, ptrdiff_t, enum arithop, @@ -2807,14 +2805,6 @@ arith_driver (enum arithop code, ptrdiff_t nargs, Lisp_Object *args) case Alogxor: accum ^= next; break; - case Amax: - if (!argnum || next > accum) - accum = next; - break; - case Amin: - if (!argnum || next < accum) - accum = next; - break; } } @@ -2871,14 +2861,6 @@ float_arith_driver (double accum, ptrdiff_t argnum, enum arithop code, case Alogior: case Alogxor: wrong_type_argument (Qinteger_or_marker_p, val); - case Amax: - if (!argnum || isnan (next) || next > accum) - accum = next; - break; - case Amin: - if (!argnum || isnan (next) || next < accum) - accum = next; - break; } } @@ -2975,22 +2957,37 @@ Both X and Y must be numbers or markers. */) return val; } +static Lisp_Object +minmax_driver (ptrdiff_t nargs, Lisp_Object *args, + enum Arith_Comparison comparison) +{ + eassume (0 < nargs); + Lisp_Object accum; + for (ptrdiff_t argnum = 0; argnum < nargs; argnum++) + { + Lisp_Object val = args[argnum]; + if (argnum == 0 || !NILP (arithcompare (val, accum, comparison))) + accum = val; + else if (FLOATP (accum) && isnan (XFLOAT_DATA (accum))) + break; + } + return accum; +} + DEFUN ("max", Fmax, Smax, 1, MANY, 0, doc: /* Return largest of all the arguments (which must be numbers or markers). -The value is always a number; markers are converted to numbers. usage: (max NUMBER-OR-MARKER &rest NUMBERS-OR-MARKERS) */) (ptrdiff_t nargs, Lisp_Object *args) { - return arith_driver (Amax, nargs, args); + return minmax_driver (nargs, args, ARITH_GRTR); } DEFUN ("min", Fmin, Smin, 1, MANY, 0, doc: /* Return smallest of all the arguments (which must be numbers or markers). -The value is always a number; markers are converted to numbers. usage: (min NUMBER-OR-MARKER &rest NUMBERS-OR-MARKERS) */) (ptrdiff_t nargs, Lisp_Object *args) { - return arith_driver (Amin, nargs, args); + return minmax_driver (nargs, args, ARITH_LESS); } DEFUN ("logand", Flogand, Slogand, 0, MANY, 0, diff --git a/test/src/data-tests.el b/test/src/data-tests.el index d38760cdde6..70ffdabe4d4 100644 --- a/test/src/data-tests.el +++ b/test/src/data-tests.el @@ -80,6 +80,26 @@ ;; Short circuits before getting to bad arg (should-not (>= 8 9 'foo))) +(ert-deftest data-tests-max () + (should-error (max)) + (should (= 1 (max 1))) + (should (= 3 (max 3 2))) + (should (= 666 (max 666 1 0 0 -2 -3 -3 -3 -4 -8 -8 -9 -999))) + (should (= (1+ most-negative-fixnum) + (max (float most-negative-fixnum) (1+ most-negative-fixnum)))) + (should (= 8 (apply #'max '(3 8 3)))) + (should-error (max 9 8 'foo))) + +(ert-deftest data-tests-min () + (should-error (min)) + (should (= 1 (min 1))) + (should (= 2 (min 3 2))) + (should (= -999 (min 666 1 0 0 -2 -3 -3 -3 -4 -8 -8 -9 -999))) + (should (= most-positive-fixnum + (min (+ 1.0 most-positive-fixnum) most-positive-fixnum))) + (should (= 3 (apply #'min '(3 8 3)))) + (should-error (min 9 8 'foo))) + ;; Bool vector tests. Compactly represent bool vectors as hex ;; strings. -- 2.39.5