[Emacs-diffs] master 37940b3: min and max now return one of their arguments

[Paul Eggert]

branch: master
commit 37940b347052418f0589bd52b06e56fffb594ea2
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    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 deae5fd..3fdc941 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 8f7356f..ce20dfb 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 66f4c9c..ae88e3f 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 d38760c..70ffdab 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.