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

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

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




Reply | Threaded
Open this post in threaded view
|

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

Michael Heerdegen
Michael Heerdegen <[hidden email]> 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)?


From 4c778f26ff8d56d0e7018305aa3d46caa2f9fb38 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <[hidden email]>
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




Michael.
Reply | Threaded
Open this post in threaded view
|

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

Michael Heerdegen
In reply to this post by 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 ...)



Reply | Threaded
Open this post in threaded view
|

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

Noam Postavsky
In reply to this post by Michael Heerdegen
Michael Heerdegen <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

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

Michael Heerdegen
Noam Postavsky <[hidden email]> 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.