Subject: bug#76017: 31.0.50; Which args can cl-nintersection and cl-nset-difference mutate?

From: "Basil L. Contovounesios" <[email-masked]>
Date: Thu, 13 Feb 2025 19:13:40 +0100

Basil L. Contovounesios [2025-02-02 18:48 +0100] wrote:

> If there is agreement, I can follow up with a patch for our docs and
> tests.

I propose the attached.  WDYT?

[ I'm not sure whether it applies cleanly as is; it builds on the
  unrelated patch proposed in https://bugs.gnu.org/75633#22. ]

Thanks,
-- 
Basil

>From ad89034f0bfd18461af2f93ab00768efefd64e7f Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Sun, 2 Feb 2025 18:05:57 +0100
Subject: [PATCH 2/2] Document cl-n... set operations consistently

The docstrings of cl-nintersection and cl-nset-difference have been
inconsistent with their manual entries since the beginning of
emacs.git history (bug#76017).  This patch settles on the weaker and
thus backward-compatible requirement that only their first argument
be safe to mutate.

* lisp/emacs-lisp/bytecomp.el: Include only first argument in
mutates-arguments property.
* lisp/emacs-lisp/cl-seq.el (cl-nintersection, cl-nset-difference):
Make docstring consistent with manual in that the second argument is
not modified.
* test/lisp/emacs-lisp/cl-seq-tests.el (cl-nintersection-test)
(cl-nset-difference-test): Simplify.
(cl-nset-difference): Pass fresh list as second argument, otherwise
destructive modifications to it could go undetected.
---
 lisp/emacs-lisp/bytecomp.el          |  2 +-
 lisp/emacs-lisp/cl-seq.el            |  8 +++---
 test/lisp/emacs-lisp/cl-seq-tests.el | 42 +++++++++++++++-------------
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 931b155313e..2ddbb2ec1da 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3583,7 +3583,7 @@ bytecomp--mutargs-sort
          (cl-nsubst 3) (cl-nsubst-if 3) (cl-nsubst-if-not 3)
          (cl-nsubstitute 3) (cl-nsubstitute-if 3) (cl-nsubstitute-if-not 3)
          (cl-nsublis 2)
-         (cl-nunion 1 2) (cl-nintersection 1 2) (cl-nset-difference 1 2)
+         (cl-nunion 1 2) (cl-nintersection 1) (cl-nset-difference 1)
          (cl-nset-exclusive-or 1 2)
          (cl-nreconc 1)
          (cl-sort 1) (cl-stable-sort 1) (cl-merge 2 3)
diff --git a/lisp/emacs-lisp/cl-seq.el b/lisp/emacs-lisp/cl-seq.el
index 1878153f811..5b4337ad9cb 100644
--- a/lisp/emacs-lisp/cl-seq.el
+++ b/lisp/emacs-lisp/cl-seq.el
@@ -864,8 +864,8 @@ cl-intersection
 (defun cl-nintersection (cl-list1 cl-list2 &rest cl-keys)
   "Combine LIST1 and LIST2 using a set-intersection operation.
 The resulting list contains all items that appear in both LIST1 and LIST2.
-This is a destructive function; it reuses the storage of LIST1 and LIST2
-whenever possible.
+This is a destructive function; it reuses the storage of LIST1 (but not
+LIST2) whenever possible.
 \nKeywords supported:  :test :test-not :key
 \n(fn LIST1 LIST2 [KEYWORD VALUE]...)"
   (and cl-list1 cl-list2 (apply 'cl-intersection cl-list1 cl-list2 cl-keys)))
@@ -894,8 +894,8 @@ cl-set-difference
 (defun cl-nset-difference (cl-list1 cl-list2 &rest cl-keys)
   "Combine LIST1 and LIST2 using a set-difference operation.
 The resulting list contains all items that appear in LIST1 but not LIST2.
-This is a destructive function; it reuses the storage of LIST1 and LIST2
-whenever possible.
+This is a destructive function; it reuses the storage of LIST1 (but not
+LIST2) whenever possible.
 \nKeywords supported:  :test :test-not :key
 \n(fn LIST1 LIST2 [KEYWORD VALUE]...)"
   (if (or (null cl-list1) (null cl-list2)) cl-list1
diff --git a/test/lisp/emacs-lisp/cl-seq-tests.el b/test/lisp/emacs-lisp/cl-seq-tests.el
index 2348a7fc812..f72596e4a4b 100644
--- a/test/lisp/emacs-lisp/cl-seq-tests.el
+++ b/test/lisp/emacs-lisp/cl-seq-tests.el
@@ -914,18 +914,18 @@ cl-intersection-test
 
 (ert-deftest cl-nintersection-test ()
   (should-not (cl-nintersection () ()))
-  (should-not (cl-nintersection () (list 1 2 3)))
-  (should-not (cl-nintersection (list 1 2) (list 3 4)))
-  (should (equal (cl-nintersection (list 1 2 3 4) (list 3 4 5 6))
+  (should-not (cl-nintersection () '(1 2 3)))
+  (should-not (cl-nintersection (list 1 2) '(3 4)))
+  (should (equal (cl-nintersection (list 1 2 3 4) '(3 4 5 6))
                  '(4 3)))
-  (should (equal (cl-nintersection (list 1 2 3) (list 1 2 3))
+  (should (equal (cl-nintersection (list 1 2 3) '(1 2 3))
                  '(1 2 3)))
-  (should (equal (cl-nintersection (list 1 1 2 2 3) (list 2 2 3 4))
+  (should (equal (cl-nintersection (list 1 1 2 2 3) '(2 2 3 4))
                  '(3 2 2)))
   (should (equal (cl-nintersection (list 1 (copy-sequence "two") 3)
-                                   (list 3 "two" 4))
+                                   '(3 "two" 4))
                  '(3)))
-  (should (equal (cl-nintersection (list 1 2 3) (list 3 2 1) :test #'equal)
+  (should (equal (cl-nintersection (list 1 2 3) '(3 2 1) :test #'equal)
                  '(1 2 3))))
 
 (ert-deftest cl-set-difference-test ()
@@ -961,47 +961,49 @@ cl-set-difference-test
 
 (ert-deftest cl-nset-difference ()
   ;; Our nset-difference doesn't preserve order.
-  (let* ((l1 (list 1 2 3 4)) (l2 '(3 4 5 6))
+  (let* ((l1 (list 1 2 3 4)) (l2 (list 3 4 5 6))
          (diff (cl-nset-difference l1 l2)))
     (should (memq 1 diff))
     (should (memq 2 diff))
-    (should (= (length diff) 2))
+    (should (length= diff 2))
     (should (equal l2 '(3 4 5 6))))
-  (let* ((l1 (list "1" "2" "3" "4")) (l2 '("3" "4" "5" "6"))
+  (let* ((l1 (list "1" "2" "3" "4")) (l2 (list "3" "4" "5" "6"))
          (diff (cl-nset-difference l1 l2 :test #'equal)))
     (should (member "1" diff))
     (should (member "2" diff))
-    (should (= (length diff) 2))
+    (should (length= diff 2))
     (should (equal l2 '("3" "4" "5" "6"))))
   (let* ((l1 (list '(a . 1) '(b . 2) '(c . 3) '(d . 4)))
          (l2 (list '(c . 3) '(d . 4) '(e . 5) '(f . 6)))
          (diff (cl-nset-difference l1 l2 :key #'car)))
     (should (member '(a . 1) diff))
     (should (member '(b . 2) diff))
-    (should (= (length diff) 2)))
+    (should (length= diff 2))
+    (should (equal l2 '((c . 3) (d . 4) (e . 5) (f . 6)))))
   (let* ((l1 (list '("a" . 1) '("b" . 2) '("c" . 3) '("d" . 4)))
          (l2 (list '("c" . 3) '("d" . 4) '("e" . 5) '("f" . 6)))
          (diff (cl-nset-difference l1 l2 :key #'car :test #'string=)))
     (should (member '("a" . 1) diff))
     (should (member '("b" . 2) diff))
-    (should (= (length diff) 2))))
+    (should (length= diff 2))
+    (should (equal l2 '(("c" . 3) ("d" . 4) ("e" . 5) ("f" . 6))))))
 
 (ert-deftest cl-nset-difference-test ()
   (should-not (cl-nset-difference () ()))
   (should-not (cl-nset-difference () (list 1 2 3)))
-  (should-not (cl-nset-difference (list 1 2 3) (list 1 2 3)))
-  (should-not (cl-nset-difference (list 1 2 3) (list 3 2 1) :test #'equal))
+  (should-not (cl-nset-difference (list 1 2 3) '(1 2 3)))
+  (should-not (cl-nset-difference (list 1 2 3) '(3 2 1) :test #'equal))
   (should (equal (cl-nset-difference (list 1 2 3) ())
                  '(1 2 3)))
-  (should (equal (cl-nset-difference (list 1 2 3 4) (list 3 4 5 6))
+  (should (equal (cl-nset-difference (list 1 2 3 4) '(3 4 5 6))
                  '(1 2)))
-  (should (equal (cl-nset-difference (list 1 1 2 2 3) (list 3 4 5))
+  (should (equal (cl-nset-difference (list 1 1 2 2 3) '(3 4 5))
                  '(1 1 2 2)))
-  (should (equal (cl-nset-difference (list 1 2 3) (list 3 2 4))
+  (should (equal (cl-nset-difference (list 1 2 3) '(3 2 4))
                  '(1)))
-  (should (equal (cl-nset-difference (list 1 2 3 4 5) (list 3 4 5 6 7))
+  (should (equal (cl-nset-difference (list 1 2 3 4 5) '(3 4 5 6 7))
                  '(1 2)))
-  (should (equal (cl-nset-difference (list 1 (copy-sequence "a")) (list 1 "a"))
+  (should (equal (cl-nset-difference (list 1 (copy-sequence "a")) '(1 "a"))
                  '("a"))))
 
 (ert-deftest cl-set-exclusive-or-test ()
-- 
2.47.2