Re: [PATCH] Add cl-map-into, revision 2

[Eli Zaretskii <eliz@gnu.org>]

> From: akater <nuclearspace@gmail.com>
> Date: Wed, 06 Oct 2021 23:35:59 +0000
> Cc: emacs-devel@gnu.org
> 
> map-into is a standard Common Lisp function that acts as cl-map, only
> values are recorded into a preallocated sequence.
> 
> * lisp/emacs-lisp/cl-extra.el
> (cl-map-into): New primary function
> (cl--map-into-basic-call-arguments-limit,
> cl--map-into-max-small-signature): New auxiliary constant
> (cl--map-into-mappers-array, cl--map-into-mappers-alist): New variable
> (cl--compute-map-into-signature, cl--make-map-into-mapper): New auxiliary function
> (cl--do-seq-type-signature): New auxiliary macro

Thanks.  Please always accompany changes that introduce new features
by corresponding changes to documentation.  The Common Lisp
compatibility library is documented in cl.texi, and the new macro(s)
should be called out in NEWS.

See some additional minor comments below.

> +(cl-defmacro cl--do-seq-type-signature ((type-var signature &optional result)
> +                                        &body body)
> +  "With TYPE-VAR bound to sequence type, evaluate BODY forms.  Return RESULT.

The first line of a doc string should be a single sentence, not longer
than 79 characters.

> +(defun cl--make-map-into-mapper (signature &optional do-not-compile)
> +  "Return mapper for `cl-map-into' specialized on arglists of type
> +encoded by SIGNATURE.

Same here.

> +(defun cl-map-into (target function &rest sequences)
> +  "Common Lisp's map-into.

The first line of a doc string of a public interface should describe
the arguments, at least the mandatory ones.

> +TARGET and each element of SEQUENCES can each be either a list
> +or a vector.

Is there any reasons you excluded other kinds of sequences (strings
and bool-vectors)?

> +(ert-deftest cl-map-into ()
> +  (should (equal '(42 42 42)
> +                 (cl-map-into (list 0 0 0) #'+ '(1 2 3) [41 40 39])))
> +  (should (equal '(42 42 42)
> +                 (cl-map-into (list 0 0 0) #'+ [41 40 39] '(1 2 3))))
> +  (should (equal '(42 42 42)
> +                 (cl-map-into (list 0 0 0) #'* '(1 2 3) [42 21 14])))
> +  (should (equal '(42 42 42)
> +                 (let ((s (list 0 0 0)))
> +                   (cl-map-into s #'+ '(1 2 3) [41 40 39])
> +                   s)))
> +  (should (equal '(42 42 42)
> +                 (let ((s (list 0 0 0)))
> +                   (cl-map-into s #'+ s [41 40 39] '(1 2 3))
> +                   s)))
> +  (should (equal '(42 42 42)
> +                 (let ((s (list 0 0 0)))
> +                   (cl-map-into s #'+ '(1 2 3) s [41 40 39])
> +                   s)))
> +  (should (equal '(42 42 42)
> +                 (let ((s (list 0 0 0)))
> +                   (cl-map-into s #'+ '(1 2 3) [41 40 39] s)
> +                   s)))
> +  (should (equal '(42 42 42)
> +                 (let ((s (list 18 19 20)))
> +                   (cl-map-into s #'+ s '(6 4 2 1 not-even-a-number) s)
> +                   s)))
> +  (should (equal [42 42 42]
> +                 (let ((s (vector 0 0 0)))
> +                   (cl-map-into s #'+ '(1 2 3) [41 40 39])
> +                   s)))
> +  (should (equal [42 42 42]
> +                 (let ((s (vector 0 0 0)))
> +                   (cl-map-into s #'+ [41 40 39] '(1 2 3))
> +                   s)))
> +  (should (equal [42 42 42]
> +                 (let ((s (vector 18 19 20)))
> +                   (cl-map-into s #'+ s '(6 4 2 1 not-even-a-number) s)
> +                   s))))
> +

I don't see here any tests where the lengths of the sequences are
different.  Can you add some of those?

Thanks again for working on this.