bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Dmitry Gutov]

Example:

;; -*- lexical-binding: t -*-

(iter-defun (str buf)
  (with-current-buffer buf
    (goto-char (point-min)))
  (let (done)
    (while (not done)
      (with-current-buffer buf
        (if (search-forward str)
            (iter-yield (match-beginning 0))
          (setq done 0))))))

...upon evaluation, says: "special form (save-current-buffer (set-buffer
buf) (if (search-forward str) (cps-internal-yield (match-beginning 0))
(setq cps-binding-done- 0))) incorrect or not supported".

The above example seemed to me to be the prime use case for generator.el
(e.g. lazily parsing output from an asynchronous process), so this
is disappointing.

In GNU Emacs 25.0.93.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.18.9)
 of 2016-04-30 built on axl
Repository revision: ffe701cb07cfb3584c4e4894976f0c9487d02c59
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
System Description:	Ubuntu 16.04 LTS

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Dmitry Gutov]

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

On 05/03/2016 04:31 AM, Dmitry Gutov wrote:

> The above example seemed to me to be the prime use case for generator.el
> (e.g. lazily parsing output from an asynchronous process), so this
> is disappointing.

Actually, I take this back.

- generator.el doesn't seem useful for parsing output from an 
asynchronous process because it has no way to indicate whether the 
process has new output, or to return control back to Emacs while the 
process is still running but has no new output. It seems to call for a 
different abstraction.

- with-current-buffer turned out not to be so essential. Nor 
save-excursion. I've reimplemented xref--buf-pairs-iterator using 
iter-lambda but couldn't get the full benefit of the package because 
that function also needs a way to reliably clean up resources. The 
result is a bit more comprehensible, but also longer, so I'm hesitant to 
use it (attached).

So sum up, maybe this bug can be closed, but I'd like to see Daniel's 
opinion. Supporting with-current-buffer might turn out to be more 
essential in other cases.

[-- Attachment #2: xref-gen.diff --]
[-- Type: text/x-patch, Size: 2161 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index f651dc9..f8b3c85 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -71,6 +71,7 @@
 (require 'ring)
 (require 'pcase)
 (require 'project)
+(require 'generator)
 
 (eval-when-compile
   (require 'semantic/symref)) ;; for hit-lines slot
@@ -534,19 +535,12 @@ xref-query-replace-in-results
       (funcall iter :cleanup))))
 
 (defun xref--buf-pairs-iterator (xrefs)
-  (let (chunk-done item next-pair file-buf pairs all-pairs)
-    (lambda (action)
-      (pcase action
-        (:next
-         (when (or xrefs next-pair)
-           (setq chunk-done nil)
-           (when next-pair
-             (setq file-buf (marker-buffer (car next-pair))
-                   pairs (list next-pair)
-                   next-pair nil))
-           (while (and (not chunk-done)
-                       (setq item (pop xrefs)))
-             (save-excursion
+  (let* (item
+         file-buf pairs all-pairs
+         (iter
+          (funcall
+           (iter-lambda ()
+             (while (setq item (pop xrefs))
                (let* ((loc (xref-item-location item))
                       (beg (xref-location-marker loc))
                       (end (move-marker (make-marker)
@@ -568,9 +562,19 @@ xref--buf-pairs-iterator
                       ((equal file-buf (marker-buffer beg))
                        (push pair pairs))
                       (t
-                       (setq chunk-done t
-                             next-pair pair))))))))
-           (cons file-buf (nreverse pairs))))
+                       (iter-yield (cons file-buf (nreverse pairs)))
+                       (setq file-buf (marker-buffer beg)
+                             pairs (list pair)))))))
+               (when (null xrefs)
+                 (iter-yield (cons file-buf (nreverse pairs)))))))))
+    (lambda (action)
+      (pcase action
+        (:next
+         (condition-case nil
+             (save-excursion
+               (iter-next iter))
+           (iter-end-of-sequence
+            nil)))
         (:cleanup
          (dolist (pair all-pairs)
            (move-marker (car pair) nil)

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Michael Heerdegen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> So sum up, maybe this bug can be closed, but I'd like to see Daniel's
> opinion. Supporting with-current-buffer might turn out to be more
> essential in other cases.

FWIW it's not crystal clear to me what the semantics of a supported
`with-current-buffer' would be.  In particular: would `iter-yield' "jump
out" of the `with-current-buffer' and restore the previous buffer, and
would a restart of the generator change the current buffer again?


Regards,

Michael.

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Dmitry Gutov]

On 05/07/2016 02:25 AM, Michael Heerdegen wrote:

> FWIW it's not crystal clear to me what the semantics of a supported
> `with-current-buffer' would be.  In particular: would `iter-yield' "jump
> out" of the `with-current-buffer' and restore the previous buffer, and
> would a restart of the generator change the current buffer again?

I think so, yeah. In the "rewritten" version of code, 
with-current-buffer would surround the function's body, so each time the 
function is called, with-current-buffer would be applied.

The value of point may or may not be saved and restored (this question 
also seems problematic, admittedly).

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Stefan Kangas]

Hi Dmitry,

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 05/03/2016 04:31 AM, Dmitry Gutov wrote:
>
>> The above example seemed to me to be the prime use case for generator.el
>> (e.g. lazily parsing output from an asynchronous process), so this
>> is disappointing.
>
> Actually, I take this back.
>
> - generator.el doesn't seem useful for parsing output from an asynchronous
>  process because it has no way to indicate whether the process has new output,
> or to return control back to Emacs while the process is still running but has no
> new output. It seems to call for a different abstraction.
>
> - with-current-buffer turned out not to be so essential. Nor
>   save-excursion. I've reimplemented xref--buf-pairs-iterator using
> iter-lambda but couldn't get the full benefit of the package because that
> function also needs a way to reliably clean up resources. The result is a bit
> more comprehensible, but also longer, so I'm hesitant to use it (attached).
>
> So sum up, maybe this bug can be closed, but I'd like to see Daniel's
> opinion. Supporting with-current-buffer might turn out to be more
> essential in other cases.

Is this patch still relevant or should this be closed?

Best regards,
Stefan Kangas

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Dmitry Gutov]

On 12.08.2020 05:41, Stefan Kangas wrote:
> Is this patch still relevant or should this be closed?

The patch was just an illustration.

But problem is still there, and still seems valid.

Before we close this, I'd like to hear from somebody who understands 
what generator.el is actually for.

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Michael Heerdegen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Before we close this, I'd like to hear from somebody who understands
> what generator.el is actually for.

I think it's just a straightforward implementation of generators as
described here:

  https://en.wikipedia.org/wiki/Generator_%28computer_programming%29

I learned about them in university, and have used them once in a while.

AFAIK handling asynchronous process output is not a good task for them.

I think: The problem of the concept of generators in an editor is that
generators are good for saving the state of a computation, but Emacs as
an editor has a lot of "environment" state (current buffer, value of
point, ...), and when the computation represented by the generator
messes with this state (or has side effects), the concept doesn't fit
that well.

When working with streams, I make the handling of the according part of
the environment explicit saving it in variables (iterators are closures)
and "yield" outside of any xxx-recursion, in your introductory example,
that would look like this:

#+begin_src emacs-lisp
(require 'generator)

(iter-defun my-search (str buf)
  (with-current-buffer buf
    (goto-char (point-min)))
  (let ((pos (point))
        (yield nil)
        (done nil))
    (while (not done)
      (when yield
        (iter-yield yield)
        (setq yield nil))
      (with-current-buffer buf
        (goto-char pos)
        (if (search-forward str)
            (setq yield (match-beginning 0)
                  pos   (point))
          (setq done 0))))))
#+end_src

I guess it would be nice if that could work implicitly in some way, but
that would probably require the introduction of new special forms
adapted to the situation, like `iterator-save-buffer-and-point' or so.

Just extrapolating my university knowledge however...


Michael.

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Dmitry Gutov]

On 16.08.2020 16:33, Michael Heerdegen wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> Before we close this, I'd like to hear from somebody who understands
>> what generator.el is actually for.
> 
> I think it's just a straightforward implementation of generators as
> described here:
> 
>    https://en.wikipedia.org/wiki/Generator_%28computer_programming%29
> 
> I learned about them in university, and have used them once in a while.

Right. I was really wondering, though, whether we can/should use them 
more often.

> AFAIK handling asynchronous process output is not a good task for them.

Perhaps if we also used a separate thread for waiting for the output to 
come...

> I think: The problem of the concept of generators in an editor is that
> generators are good for saving the state of a computation, but Emacs as
> an editor has a lot of "environment" state (current buffer, value of
> point, ...), and when the computation represented by the generator
> messes with this state (or has side effects), the concept doesn't fit
> that well.
> 
> When working with streams, I make the handling of the according part of
> the environment explicit saving it in variables (iterators are closures)
> and "yield" outside of any xxx-recursion, in your introductory example,
> that would look like this:
> 
> #+begin_src emacs-lisp
> (require 'generator)
> 
> (iter-defun my-search (str buf)
>    (with-current-buffer buf
>      (goto-char (point-min)))
>    (let ((pos (point))
>          (yield nil)
>          (done nil))
>      (while (not done)
>        (when yield
>          (iter-yield yield)
>          (setq yield nil))
>        (with-current-buffer buf
>          (goto-char pos)
>          (if (search-forward str)
>              (setq yield (match-beginning 0)
>                    pos   (point))
>            (setq done 0))))))
> #+end_src

Thanks. I think this is fairly close to the approach I showed in the 
patch. Problem is, it's no shorter than an implementation one can write 
using plain functions. Longer, usually.

> I guess it would be nice if that could work implicitly in some way, but
> that would probably require the introduction of new special forms
> adapted to the situation, like `iterator-save-buffer-and-point' or so.

Perhaps.

bug#23430: 25.0.93; iter-defun does not support special form save-current-buffer

[Michael Heerdegen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> But problem is still there, and still seems valid.
>
> Before we close this, I'd like to hear from somebody who understands
> what generator.el is actually for.

I've just CC'd Daniel assuming he may has missed this bug report.

Michael.