Re: [Emacs-diffs] master f6b5db6: Add support for generators

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

>     Add support for generators
>    
>     diff --git a/doc/lispref/ChangeLog b/doc/lispref/ChangeLog
[...]
>     +	* automated/generator-tests.el: New tests

The content of the commit log is fine, but the form sucks.  Please get
rid of the diff-cruft in those commit messages.

> +  A @dfn{generator} is a function that produces a potentially-infinite
                            ^^^^^^^^
Do we want to document it as a function?  Maybe it would be better to
document it as an "object" of unspecified implementation (i.e. calling
it via funcall rather than via iter-next is unsupported).

> +@defmac iter-yield-from iterator
> +@code{iter-yield-from} yields all the values that @var{iterator}
> +produces and evaluates to the value that @var{iterator}'s generator
> +function returns normally.  While it has control, @var{iterator}
> +receives sent to the iterator using @code{iter-next}.
   ^^^^^^^^^^^^^
Some word(s) is missing here.

> +  To use a generator function, first call it normally, producing a
> +@dfn{iterator} object.

Ah, right, so that's why you defined generators as functions.  OK, now
that makes sense.  Maybe this description of the difference between
a generator and an iterator should be placed earlier, tho.

> +@defun iter-close iterator
> +If @var{iterator} is suspended inside a @code{unwind-protect} and
> +becomes unreachable, Emacs will eventually run unwind handlers after a
> +garbage collection pass.  To ensure that these handlers are run before
> +then, use @code{iter-close}.
> +@end defun

Hmm... but earlier you said:

   +@code{iter-yield} and @code{iter-yield-from} cannot appear inside
   +@code{unwind-protect} forms.

Oh, wait, I think I understand: we can use iter-yield inside the main
body of unwind-protect but not inside the unwind forms, right?  I think
this should be clarified.

To get back to iter-close, this seems to be a tricky aspect of the
implementation.  Could you give me more details, such as an example
problematic case where calling iter-close makes a difference?
I guess this has to do with those finalizers, so if you could explain
how/why these are used, I'd really appreciate it.

> +@example
> +(iter-defun my-iter (x)
> +  (iter-yield (1+ (iter-yield (1+ x))))
> +  -1 ;; Return normally
> +  )

Please don't leave a close-paren alone on its line in such example code.

> -	* vc/vc.el (vc-responsible-backend): Add autoload cooking for
> +	* vc/vc.el (vc-responsible-backend): Add autoload cookie for

Thanks ;-)

> +(defvar *cps-bindings* nil)
> +(defvar *cps-states* nil)
> +(defvar *cps-value-symbol* nil)
> +(defvar *cps-state-symbol* nil)
> +(defvar *cps-cleanup-table-symbol* nil)
> +(defvar *cps-cleanup-function* nil)
> +
> +(defvar *cps-dynamic-wrappers* '(identity)
> +  "List of transformer functions to apply to atomic forms we
> +evaluate in CPS context.")

This CL naming style is not used in Elisp.  I.e. just drop the
surrounding stars.

> +  (let* ((state (cl-gensym (format "cps-state-%s-" kind))))
                    ^^^^^^^^^
Elisp prefers make-symbol.

> +(defvar cps-disable-atomic-optimization nil

We usually use "inhibit" rather than "disable".

> +    ;; Unfortunately, because elisp lacks a mechanism for generically
> +    ;; capturing the reason for an arbitrary non-local control
> +    ;; transfer and restarting the transfer at a later point, we
> +    ;; cannot reify non-local transfers and cannot allow
> +    ;; continuation-passing code inside UNWINDFORMS.

I think it's an acceptable limitation.  Unwind-forms should be "short
and to the point".  We actually should run them with inhibit-quit (tho
we currently don't, which leads to some problems when the user hits C-g
repeatedly in panic).

> +(put 'iter-end-of-sequence 'error-conditions '(iter-end-of-sequence))
> +(put 'iter-end-of-sequence 'error-message "iteration terminated")

This should use the newish `define-error'.

> +(defmacro iter-yield-from (value)
> +  "When used inside a generator function, delegate to a sub-iterator.
> +The values that the sub-iterator yields are passed directly to
> +the caller, and values supplied to `iter-next' are sent to the
> +sub-iterator.  `iter-yield-from' evaluates to the value that the
> +sub-iterator function returns via `iter-end-of-sequence'."
> +  (let ((errsym (cl-gensym "yield-from-result"))
> +        (valsym (cl-gensym "yield-from-value")))
> +    `(let ((,valsym ,value))
> +       (unwind-protect
> +            (condition-case ,errsym
> +                (let ((vs nil))
> +                  (while t
> +                    (setf vs (iter-yield (iter-next ,valsym vs)))))
> +              (iter-end-of-sequence (cdr ,errsym)))
> +         (iter-close ,valsym)))))

[Mostly out of curiosity:] Could this be implemented as

   `(iter--blabla ,value (lambda (x) (iter-yield x)))

> +(defmacro iter-defun (name arglist &rest body)
> +  "Creates a generator NAME.
> +When called as a function, NAME returns an iterator value that
> +encapsulates the state of a computation that produces a sequence
> +of values.  Callers can retrieve each value using `iter-next'."
> +  (declare (indent defun))
> +  (cl-assert lexical-binding)
> +  `(defun ,name ,arglist
> +     ,(cps-generate-evaluator
> +       `(cl-macrolet ((iter-yield (value) `(cps-internal-yield ,value)))
> +          ,@body))))

IIUC you don't support any declarations between ARGLIST and BODY.
You could use macroexp-parse-body for that.

In cps-generate-evaluator I see you do:

> +                         (macroexpand-all form)

IIUC this will lose any local macro stored in
macroexpand-all-environment.  IOW you need to pass
macroexpand-all-environment to this macroexpand-all call.

Also, both calls to cps-generate-evaluator look identical, so I think
it'd be better to just pass `body' to cps-generate-evaluator, and then
have cps-generate-evaluator handle the local macro definition of iter-yield.

> +(eval-after-load 'elisp-mode
> +  (lambda ()
> +    (font-lock-add-keywords
> +     'emacs-lisp-mode
> +     '(("(\\(iter-defun\\)\\_>\\s *\\(\\(?:\\sw\\|\\s_\\)+\\)?"
> +        (1 font-lock-keyword-face nil t)
> +        (2 font-lock-function-name-face nil t))
> +       ("(\\(iter-next\\)\\_>"
> +        (1 font-lock-keyword-face nil t))
> +       ("(\\(iter-lambda\\)\\_>"
> +        (1 font-lock-keyword-face nil t))
> +       ("(\\(iter-yield\\)\\_>"
> +        (1 font-lock-keyword-face nil t))
> +       ("(\\(iter-yield-from\\)\\_>"
> +        (1 font-lock-keyword-face nil t))))))

Using fewer entries (at most 2) would make this more efficient
(especially since they all start with "(iter-)").


        Stefan