bug#37888: 27.0.50; Streams and errors in element generation

bug#37888: 27.0.50; Streams and errors in element generation

[Michael Heerdegen]

Hi,

consider this case (it appeared to me in real life):

--8<---------------cut here---------------start------------->8---
(defun test-stream (n)
  (stream-cons n (if (< n 0) (error "test") (test-stream (1- n)))))

(setq my-stream (test-stream 10))

(condition-case nil (seq-length my-stream)
  (error (message "Hmm, didn't work so well")))
--8<---------------cut here---------------end--------------->8---

Now, what happened to `my-stream' after evaluating this?  If you try to
use it, you get a quite confusing error:

(seq-length my-stream)

|-- stream--force: Wrong type argument: streamp, (((n . -1) t) nil ...)

i.e. the stream is broken.  I wonder if we could improve this case.  I'm
not sure how, however.  Should referencing the element whose generation
caused the error raise the same error again, or a standardized kind of
error?  Should the whole stream be invalidated in some way?  FWIW, in my
use case I expected the stream to raise the same error again.  Would it
be reasonable to make the stream just retry to calculate the problematic
element?


TIA,

Michael.


In GNU Emacs 27.0.50 (build 27, x86_64-pc-linux-gnu, GTK+ Version 3.24.12)
 of 2019-10-23 built on drachen
Repository revision: 39ba44f18445c7759de5ac91ce25e53123c2abeb
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux bullseye/sid

bug#37888: 27.0.50; Streams and errors in element generation

[Michael Heerdegen]

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> (defun test-stream (n)
>   (stream-cons n (if (< n 0) (error "test") (test-stream (1- n)))))
>
> (setq my-stream (test-stream 10))
>
> (condition-case nil (seq-length my-stream)
>   (error (message "Hmm, didn't work so well")))
>
> Now, what happened to `my-stream' after evaluating this?  If you try to
> use it, you get a quite confusing error:
>
> (seq-length my-stream)
>
> |-- stream--force: Wrong type argument: streamp, (((n . -1) t) nil ...)

Would something like this make sense (Noam)?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-stream-Fix-Bug-37888.patch --]
[-- Type: text/x-diff, Size: 999 bytes --]

From 4c778f26ff8d56d0e7018305aa3d46caa2f9fb38 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Wed, 23 Oct 2019 16:55:01 +0200
Subject: [PATCH] WIP [stream] Fix Bug#37888

---
 packages/stream/stream.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/stream/stream.el b/packages/stream/stream.el
index 9f73e8b861..d401fb7e3c 100644
--- a/packages/stream/stream.el
+++ b/packages/stream/stream.el
@@ -86,8 +86,8 @@ That value is the one passed to `stream-make'."
    ((eq (car-safe stream) stream--evald-identifier)
     (cdr stream))
    ((eq (car-safe stream) stream--fresh-identifier)
-    (setf (car stream) stream--evald-identifier)
-    (setf (cdr stream) (funcall (cdr stream))))
+    (prog1 (setf (cdr stream) (funcall (cdr stream)))
+      (setf (car stream) stream--evald-identifier)))
    (t (signal 'wrong-type-argument (list 'streamp stream)))))

 (defmacro stream-cons (first rest)
--
2.23.0


[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Michael.

bug#37888: 27.0.50; Streams and errors in element generation

[Noam Postavsky]

Michael Heerdegen <michael_heerdegen@web.de> writes:
>
> Would something like this make sense (Noam)?

>     ((eq (car-safe stream) stream--fresh-identifier)
> -    (setf (car stream) stream--evald-identifier)
> -    (setf (cdr stream) (funcall (cdr stream))))
> +    (prog1 (setf (cdr stream) (funcall (cdr stream)))
> +      (setf (car stream) stream--evald-identifier)))

Right, only mark the stream element as evaluated after we (successfully!)
evaluate it.  Makes sense to me.

bug#37888: 27.0.50; Streams and errors in element generation

[Michael Heerdegen]

Noam Postavsky <npostavs@gmail.com> writes:

> > Would something like this make sense (Noam)?
>
> >     ((eq (car-safe stream) stream--fresh-identifier)
> > -    (setf (car stream) stream--evald-identifier)
> > -    (setf (cdr stream) (funcall (cdr stream))))
> > +    (prog1 (setf (cdr stream) (funcall (cdr stream)))
> > +      (setf (car stream) stream--evald-identifier)))
>
> Right, only mark the stream element as evaluated after we (successfully!)
> evaluate it.  Makes sense to me.

Installed.

@Nicolas: Would you mind when I also bump the version of stream to 2.2.5
and update the copyright years of the files in the stream package?

TIA,

Michael.

bug#37888: 27.0.50; Streams and errors in element generation

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Installed.
>
> @Nicolas: Would you mind when I also bump the version of stream to
> 2.2.5 and update the copyright years of the files in the stream
> package?

I've done this now.  We are done and I'm closing this bug.


Thanks,

Michael.

bug#37888: 27.0.50; Streams and errors in element generation

[Michael Heerdegen]

Hello again,

Btw, AFAIU this problem pertains any form of nonlocal exit, not only
errors.  Using nonlocal exits in stream element generation (e.g. via
`while-no-input') is a common use case for me.  My suggested should fix
this.


Regards,

Michael.

> consider this case (it appeared to me in real life):
>
> (defun test-stream (n)
>   (stream-cons n (if (< n 0) (error "test") (test-stream (1- n)))))
>
> (setq my-stream (test-stream 10))
>
> (condition-case nil (seq-length my-stream)
>   (error (message "Hmm, didn't work so well")))
>
> Now, what happened to `my-stream' after evaluating this?  If you try to
> use it, you get a quite confusing error:
>
> (seq-length my-stream)
>
> |-- stream--force: Wrong type argument: streamp, (((n . -1) t) nil ...)