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

[akater <nuclearspace@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4816 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +(cl-defmacro cl--do-seq-type-signature ((type-var signature &optional result)
>> +                                        &body body)
>
> I'm not fond of this internal macro used at only one place.
> I'd rather we write the resulting code at its caller's location.>

It can be reused to reimplement other functions in cl-extra.  See the
bug report on why they probably should be reimplemented:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02374.html


>> +      `(let ((,sig ,signature) ,type-var)
>> +         ;; (declare (type (integer (1)) ,sig)
>> +         ;;          ;; Let's keep nil for now.
>> +         ;;          (type (member nil list vector) ,type-var))
>> +         (cl-check-type ,sig (integer (1)))
>> +         (cl-loop (cond
>> +                   ((or (when (= 2 ,sig) (setq ,type-var 'list))
>> +                        (when (= 3 ,sig) (setq ,type-var 'array)))
>> +                    ;; This duplicates main code sometimes.  Maybe,
>> +                    ;; there is elegant enough way to eliminate duplication.
>> +                    ,@(or first main) (cl-return ,result))
>> +                   (t (setq ,type-var (if (zerop (mod ,sig 2))
>> +                                          'list
>> +                                        'array))
>> +                      ,@main))
>> +                  (setf ,sig (floor ,sig 2)))))))
>
>     (let (,type-var)
>       ...
>       (setq ,type-var ...)
>       ...)
>
> generates worse code than
>
>      ...
>      (let ((,type-var ...))
>        ...)

Rewriting it with a correct initial value would complicate the code
while we don't want this code to be particularly efficient as it only
runs once per signature.


>> +    (funcall
>> +     (if do-not-compile #'identity #'byte-compile)
>> +     `(lambda ,(cons (setq f (make-symbol "f")) (cons result-var ss))
>
> I think you meant `#'eval` (or better: (lambda (x) (eval x t))) instead
> of `#'identity`.

The purpose of do-not-compile: t is to get a human-readable expression,
e.g. for better understanding and for tests (which is why it's not
made default).  eval would produce something less straightforward.


> Also you'll want to bind `lexical-binding` around the call to `byte-compile`.

Will do.  What about native-compile?  I suggested above to use it as
default (and use COMPILE rather than DO-NOT-COMPILE argname) but I'm not
familiar with it.


>> +  (apply
>> +   (let* ((sig (apply #'cl--compute-map-into-signature result-sequence
>> +                      sequences))
>> +          (small (< sig cl--map-into-max-small-signature)))
>> +     (with-memoization (if small (aref cl--map-into-mappers-array sig)
>> +                         ;; TODO: Order alist entries for faster lookup
>> +                         ;; (note that we'll have to abandon alist-get then).
>> +                         (alist-get sig cl--map-into-mappers-alist
>> +                                    nil nil #'=))
>> +                       (cl--make-map-into-mapper sig)))
>> +   function result-sequence sequences))
>
>  Makes me wonder if we could define this as a cl-generic function

Define what exactly as cl-generic function?  make-mapper?  What would it
dispatch on?

I have to reiterate: it's almost always a mistake trying to use CLOS
when there's no class hierarchy at sight.  The mistake does not seem to
be recognised even though CL users keep writing CL libraries that try to
implement non-CLOS dispatch, for valid reasons.

CLOS dispatch is deliberately limited in what it can work with, so that
it can order methods for combining.  Often, you simply need a more
general dispatch which is incompatible with predictable ordering of
methods.  Here we dispatch on signatures which are integers.  One can't
canonically order “methods” that specialize on integer ranges, for
example.  In any case, here we'd dispatch on values which are,
similarly, too complex to order into a hierarchy.  Which makes the whole
method-combination mechanism unapplicable.

>   and use something like method combinators to generate the code on
>   the fly.  More specifically, if we can't with the current code, it
>   seems like a fun exercise to see what it would take to make it
>   possible.

It so happens that I'm currently writing another general purpose
sequence-oriented library in elisp, trying to actually use cl-generic
there fore code generation.  The topic has been intriguing me for quite
some time.  But for cl-map-into, I'm fine with what's already there.  It
works, for reasonable purposes it is as efficient as compiler will allow
it to be, and it's straightforwardly extensible to other sequence types.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]