all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Tino Calancha <tino.calancha@gmail.com>
To: npostavs@users.sourceforge.net
Cc: 25826@debbugs.gnu.org, tino.calancha@gmail.com
Subject: bug#25826: 26.0.50; cl-mapc and cl-mapl do needless consing
Date: Wed, 22 Feb 2017 14:46:45 +0900	[thread overview]
Message-ID: <87y3wyojzu.fsf@calancha-pc> (raw)
In-Reply-To: <alpine.DEB.2.20.1702221312560.2112@calancha-pc> (Tino Calancha's message of "Wed, 22 Feb 2017 13:14:04 +0900 (JST)")

Tino Calancha <tino.calancha@gmail.com> writes:

> On Tue, 21 Feb 2017, npostavs@users.sourceforge.net wrote:
>
>> Tino Calancha <tino.calancha@gmail.com> writes:
>>
>>> How about the following updated patch?
>>
>> Looks good.  Perhaps some tests would be a good idea too?
> I think so, i will prepare some and post them here.  Thanks.
It's tricky to write a test to check if cl-mapc/cl-mapl over cons.
Thus, i have written tests that just ensure the return values
are as expected.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 2824777a3b1b5217fb1dd5cddc89f4f2b5679b7a Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 22 Feb 2017 14:38:17 +0900
Subject: [PATCH] Prevent for consing in cl-mapc and cl-mapl

* lisp/emacs-lisp/cl-extra.el (cl--mapcar-many): Add optional arg ACC;
If non-nil, accumulate values in the result (Bug#25826).
(cl-mapc): Do computations inside function instead of call cl-map.
(cl-mapl): Do computations inside function instead of call cl-maplist.
* lisp/emacs-lisp/cl-lib.el (mapcar): Add autoload cookie.
Call cl--mapcar-many with non-nil 3rd argument.
* test/lisp/emacs-lisp/cl-extra-tests.el (cl-extra-test-map)
(cl-extra-test-mapc, cl-extra-test-mapcar, cl-extra-test-mapl)
(cl-extra-test-maplist): New tests.
---
 lisp/emacs-lisp/cl-extra.el            | 38 +++++++++++++++-------
 lisp/emacs-lisp/cl-lib.el              |  5 +--
 test/lisp/emacs-lisp/cl-extra-tests.el | 59 ++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/cl-extra.el b/lisp/emacs-lisp/cl-extra.el
index edd14b816f..8cba913710 100644
--- a/lisp/emacs-lisp/cl-extra.el
+++ b/lisp/emacs-lisp/cl-extra.el
@@ -89,7 +89,7 @@ cl-equalp
 ;;; Control structures.
 
 ;;;###autoload
-(defun cl--mapcar-many (cl-func cl-seqs)
+(defun cl--mapcar-many (cl-func cl-seqs &optional acc)
   (if (cdr (cdr cl-seqs))
       (let* ((cl-res nil)
 	     (cl-n (apply 'min (mapcar 'length cl-seqs)))
@@ -106,20 +106,23 @@ cl--mapcar-many
 			  (setcar cl-p1 (cdr (car cl-p1))))
 		      (aref (car cl-p1) cl-i)))
 	    (setq cl-p1 (cdr cl-p1) cl-p2 (cdr cl-p2)))
-	  (push (apply cl-func cl-args) cl-res)
+	  (if acc
+	      (push (apply cl-func cl-args) cl-res)
+	    (apply cl-func cl-args))
 	  (setq cl-i (1+ cl-i)))
-	(nreverse cl-res))
+	(and acc (nreverse cl-res)))
     (let ((cl-res nil)
 	  (cl-x (car cl-seqs))
 	  (cl-y (nth 1 cl-seqs)))
       (let ((cl-n (min (length cl-x) (length cl-y)))
 	    (cl-i -1))
 	(while (< (setq cl-i (1+ cl-i)) cl-n)
-	  (push (funcall cl-func
-                         (if (consp cl-x) (pop cl-x) (aref cl-x cl-i))
-                         (if (consp cl-y) (pop cl-y) (aref cl-y cl-i)))
-                cl-res)))
-      (nreverse cl-res))))
+	  (let ((val (funcall cl-func
+			      (if (consp cl-x) (pop cl-x) (aref cl-x cl-i))
+			      (if (consp cl-y) (pop cl-y) (aref cl-y cl-i)))))
+	    (when acc
+	      (push val cl-res)))))
+	(and acc (nreverse cl-res)))))
 
 ;;;###autoload
 (defun cl-map (cl-type cl-func cl-seq &rest cl-rest)
@@ -142,7 +145,7 @@ cl-maplist
 	(while (not (memq nil cl-args))
 	  (push (apply cl-func cl-args) cl-res)
 	  (setq cl-p cl-args)
-	  (while cl-p (setcar cl-p (cdr (pop cl-p)) )))
+	  (while cl-p (setcar cl-p (cdr (pop cl-p)))))
 	(nreverse cl-res))
     (let ((cl-res nil))
       (while cl-list
@@ -155,8 +158,14 @@ cl-mapc
   "Like `cl-mapcar', but does not accumulate values returned by the function.
 \n(fn FUNCTION SEQUENCE...)"
   (if cl-rest
-      (progn (apply 'cl-map nil cl-func cl-seq cl-rest)
-	     cl-seq)
+      (if (or (cdr cl-rest) (nlistp cl-seq) (nlistp (car cl-rest)))
+          (progn
+            (cl--mapcar-many cl-func (cons cl-seq cl-rest))
+            cl-seq)
+        (let ((cl-x cl-seq) (cl-y (car cl-rest)))
+          (while (and cl-x cl-y)
+            (funcall cl-func (pop cl-x) (pop cl-y)))
+          cl-seq))
     (mapc cl-func cl-seq)))
 
 ;;;###autoload
@@ -164,7 +173,12 @@ cl-mapl
   "Like `cl-maplist', but does not accumulate values returned by the function.
 \n(fn FUNCTION LIST...)"
   (if cl-rest
-      (apply 'cl-maplist cl-func cl-list cl-rest)
+      (let ((cl-args (cons cl-list (copy-sequence cl-rest)))
+	    cl-p)
+	(while (not (memq nil cl-args))
+          (apply cl-func cl-args)
+	  (setq cl-p cl-args)
+	  (while cl-p (setcar cl-p (cdr (pop cl-p))))))
     (let ((cl-p cl-list))
       (while cl-p (funcall cl-func cl-p) (setq cl-p (cdr cl-p)))))
   cl-list)
diff --git a/lisp/emacs-lisp/cl-lib.el b/lisp/emacs-lisp/cl-lib.el
index 5aa8f1bf65..8c4455a3da 100644
--- a/lisp/emacs-lisp/cl-lib.el
+++ b/lisp/emacs-lisp/cl-lib.el
@@ -347,8 +347,9 @@ cl-float-negative-epsilon
 
 (cl--defalias 'cl-copy-seq 'copy-sequence)
 
-(declare-function cl--mapcar-many "cl-extra" (cl-func cl-seqs))
+(declare-function cl--mapcar-many "cl-extra" (cl-func cl-seqs &optional acc))
 
+;;;###autoload
 (defun cl-mapcar (cl-func cl-x &rest cl-rest)
   "Apply FUNCTION to each element of SEQ, and make a list of the results.
 If there are several SEQs, FUNCTION is called with that many arguments,
@@ -358,7 +359,7 @@ cl-mapcar
 \n(fn FUNCTION SEQ...)"
   (if cl-rest
       (if (or (cdr cl-rest) (nlistp cl-x) (nlistp (car cl-rest)))
-	  (cl--mapcar-many cl-func (cons cl-x cl-rest))
+	  (cl--mapcar-many cl-func (cons cl-x cl-rest) 'accumulate)
 	(let ((cl-res nil) (cl-y (car cl-rest)))
 	  (while (and cl-x cl-y)
 	    (push (funcall cl-func (pop cl-x) (pop cl-y)) cl-res))
diff --git a/test/lisp/emacs-lisp/cl-extra-tests.el b/test/lisp/emacs-lisp/cl-extra-tests.el
index 3e2388acc6..82b2206a6c 100644
--- a/test/lisp/emacs-lisp/cl-extra-tests.el
+++ b/test/lisp/emacs-lisp/cl-extra-tests.el
@@ -35,4 +35,63 @@
     (should (eq (cl-getf plist 'y :none) nil))
     (should (eq (cl-getf plist 'z :none) :none))))
 
+(ert-deftest cl-extra-test-mapc ()
+  (let ((lst '(a b c))
+        (lst2 '(d e f))
+        (lst3 '(1 2 3))
+        (fn1 (lambda (x) nil))
+        (fn2 (lambda (x y) nil))
+        (fn3 (lambda (x y z) nil)))
+    (should (equal lst (cl-mapc fn1 lst)))
+    (should (equal lst (cl-mapc fn2 lst lst2)))
+    (should (equal lst (cl-mapc fn3 lst lst2 lst3)))))
+
+(ert-deftest cl-extra-test-mapl ()
+  (let ((lst '(a b c))
+        (lst2 '(d e f))
+        (lst3 '(1 2 3))
+        (fn1 (lambda (x) nil))
+        (fn2 (lambda (x y) nil))
+        (fn3 (lambda (x y z) nil)))
+    (should (equal lst (cl-mapl fn1 lst)))
+    (should (equal lst (cl-mapl fn2 lst lst2)))
+    (should (equal lst (cl-mapl fn3 lst lst2 lst3)))))
+
+(ert-deftest cl-extra-test-mapcar ()
+  (let ((lst '(a b c))
+        (lst2 '(d e f))
+        (lst3 '(1 2 3))
+        (fn1 (lambda (x) x))
+        (fn2 (lambda (x y) y))
+        (fn3 (lambda (x y z) z)))
+    (should (equal lst (cl-mapcar fn1 lst)))
+    (should (equal lst2 (cl-mapcar fn2 lst lst2)))
+    (should (equal lst3 (cl-mapcar fn3 lst lst2 lst3)))))
+
+(ert-deftest cl-extra-test-map ()
+  (let ((lst '(a b c))
+        (lst2 '(d e f))
+        (lst3 '(1 2 3))
+        (fn1 (lambda (x) x))
+        (fn2 (lambda (x y) y))
+        (fn3 (lambda (x y z) (string-to-char (format "%S" x)))))
+    (should (equal lst (cl-map 'list fn1 lst)))
+    (should (equal (vconcat lst2) (cl-map 'vector fn2 lst lst2)))
+    (should (equal (mapconcat (lambda (x) (format "%S" x)) lst "")
+                   (cl-map 'string fn3 lst lst2 lst3)))))
+
+(ert-deftest cl-extra-test-maplist ()
+  (let ((lst '(a b c))
+        (lst2 '(d e f))
+        (lst3 '(1 2 3))
+        (fn1 (lambda (x) x))
+        (fn2 (lambda (x y) y))
+        (fn3 (lambda (x y z) z)))
+    (should (equal (list lst (cdr lst) (cddr lst))
+                   (cl-maplist fn1 lst)))
+    (should (equal (list lst2 (cdr lst2) (cddr lst2))
+                   (cl-maplist fn2 lst lst2)))
+    (should (equal (list lst3 (cdr lst3) (cddr lst3))
+                   (cl-maplist fn3 lst lst2 lst3)))))
+
 ;;; cl-extra-tests.el ends here
-- 
2.11.0

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.22.7)
 of 2017-02-22
Repository revision: fb997d30af28da4712ca64876feddbd07db20e13






  reply	other threads:[~2017-02-22  5:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-21  8:04 bug#25826: 26.0.50; cl-mapc and cl-mapl do needless consing Tino Calancha
2017-02-21 13:40 ` npostavs
2017-02-22  3:08   ` Tino Calancha
2017-02-22  3:33     ` npostavs
2017-02-22  4:14       ` Tino Calancha
2017-02-22  5:46         ` Tino Calancha [this message]
2017-02-23  1:55           ` npostavs
2017-02-23  2:09             ` Tino Calancha
2017-02-27  7:36               ` Tino Calancha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y3wyojzu.fsf@calancha-pc \
    --to=tino.calancha@gmail.com \
    --cc=25826@debbugs.gnu.org \
    --cc=npostavs@users.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.