Subject: Re: bug#75105: (cl-random -1.0e+INF)

From: Pip Cet <[email-masked]>
Date: Sun, 16 Feb 2025 00:50:04 +0000

"Pip Cet via \"Bug reports for GNU Emacs, the Swiss army knife of text editors\"" <[email-masked]> writes:

> The current behavior of (cl-random -1.0e+INF) is to return NaN with a
> probability of 1.1920928955078125e-07, and -1.0e+INF in the remaining
> cases.

Okay, so maybe I was wrong and that should be fixed.  This patch will
make cl-random behave consistently, though one could argue about some
cases:

(cl-random 0) involves a division by zero, as it should
(cl-random 1.0e+INF) returns 1.0e+INF, as it should
(cl-random 0.0) returns 0.0, but one could argue it should throw
(cl-random -0.0) should do the same as (cl-random -0.0) since negative
zero is a myth.

Updating the docstring would be nice, too.

>From 22e59a481a7a1c2cbff349af0f6faa2f428981a8 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH] Make cl-random behave consistently for unusual arguments
 (bug#75105)

The old behavior was for (cl-random -1.0e+INF) to return NaN in about
one of eight million calls, and -1.0e+INF otherwise.  Other unusual
arguments were handled inconsistently as well.

* lisp/emacs-lisp/cl-extra.el (cl-random): Handle nonnegative
arguments consistently, error for negative arguments.
* test/lisp/emacs-lisp/cl-extra-tests.el (cl-extra-test-random): New
test.
---
 lisp/emacs-lisp/cl-extra.el            | 20 +++++++++++++-------
 test/lisp/emacs-lisp/cl-extra-tests.el | 10 ++++++++++
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index 09470457d93..8c218ff9d7e 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -494,13 +494,19 @@ cl-random
     (let* ((i (cl-callf (lambda (x) (% (1+ x) 55)) (cl--random-state-i state)))
 	   (j (cl-callf (lambda (x) (% (1+ x) 55)) (cl--random-state-j state)))
 	   (n (aset vec i (logand 8388607 (- (aref vec i) (aref vec j))))))
-      (if (integerp lim)
-	  (if (<= lim 512) (% n lim)
-	    (if (> lim 8388607) (setq n (+ (ash n 9) (cl-random 512 state))))
-	    (let ((mask 1023))
-	      (while (< mask (1- lim)) (setq mask (1+ (+ mask mask))))
-	      (if (< (setq n (logand n mask)) lim) n (cl-random lim state))))
-	(* (/ n '8388608e0) lim)))))
+      (cond
+       ((natnump lim)
+	(if (<= lim 512) (% n lim)
+	  (if (> lim 8388607) (setq n (+ (ash n 9) (cl-random 512 state))))
+	  (let ((mask 1023))
+	    (while (< mask (1- lim)) (setq mask (1+ (+ mask mask))))
+	    (if (< (setq n (logand n mask)) lim) n (cl-random lim state)))))
+       ((< 0 lim 1.0e+INF)
+        (* (/ n '8388608e0) lim))
+       ((< lim -0.0)
+        (error "negative limit supplied to cl-random"))
+       (t
+        lim)))))
 
 ;;;###autoload
 (defun cl-make-random-state (&optional state)
diff --git a/test/lisp/emacs-lisp/cl-extra-tests.el b/test/lisp/emacs-lisp/cl-extra-tests.el
index 75533b36f29..d87c0bfec5d 100644
--- a/test/lisp/emacs-lisp/cl-extra-tests.el
+++ b/test/lisp/emacs-lisp/cl-extra-tests.el
@@ -348,4 +348,14 @@ cl-extra-test-tailp
     (should (cl-tailp l l))
     (should (not (cl-tailp '(4 5) l)))))
 
+(ert-deftest cl-extra-test-random ()
+  (should-error (cl-random -1))
+  (should-error (cl-random -0.5))
+  (should-error (cl-random -1.0e+INF))
+  (should-error (cl-random 0))
+  (should (eql (cl-random 1) 0)))
+  (should (eql (cl-random 0.0) 0.0))
+  (should (= (cl-random -0.0) 0.0))
+  (should (= (cl-random 1.0e+INF) 1.0e+INF))
+
 ;;; cl-extra-tests.el ends here
-- 
2.48.1