Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9

Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9

[Mark H Weaver]

Hi Chris,

Thanks for working on this, but in the future, please post your proposed
patches for review before pushing to stable-2.0 or master.

> commit 4a1cdc9d5d643d05fa7a18febc7c12070f3ef6d9
> Author: Chris K. Jester-Young <cky944@gmail.com>
> Date:   Thu Apr 4 22:18:40 2013 -0400
>
>     Add record type printers for srfi-41 and srfi-45.
>     
>     * module/srfi/srfi-41.scm: Add record type printer for streams.
>     * module/srfi/srfi-45.scm: Add record type printer for promises.
>
> -----------------------------------------------------------------------
>
> Summary of changes:
>  module/srfi/srfi-41.scm |   23 ++++++++++++++++++++++-
>  module/srfi/srfi-45.scm |    8 +++++++-
>  2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/module/srfi/srfi-41.scm b/module/srfi/srfi-41.scm
> index edf95d7..243bd44 100644
> --- a/module/srfi/srfi-41.scm
> +++ b/module/srfi/srfi-41.scm
> @@ -27,6 +27,7 @@
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-8)
>    #:use-module (srfi srfi-9)
> +  #:use-module (srfi srfi-9 gnu)
>    #:use-module (srfi srfi-26)
>    #:use-module (ice-9 match)
>    #:export (stream-null stream-cons stream? stream-null? stream-pair?
> @@ -148,7 +149,7 @@
>  
>  (define stream? stream-promise?)
>  
> -(define %stream-null '(stream . null))
> +(define %stream-null (cons 'stream 'null))

This change is not mentioned in the commit message.
Ideally, it should have been a separate commit, IMO.

>  (define stream-null (stream-eager %stream-null))
>  
>  (define (stream-null? obj)
> @@ -180,6 +181,26 @@
>  (define-syntax-rule (stream-lambda formals body0 body1 ...)
>    (lambda formals (stream-lazy (begin body0 body1 ...))))
>  
> +(set-record-type-printer! stream-promise
> +  (lambda (strm port)
> +    (display "#<stream" port)
> +    (let loop ((strm strm))
> +      (define value (stream-promise-val strm))
> +      (case (stream-value-tag value)
> +        ((eager)
> +         (let ((pare (stream-value-proc value)))
> +           (if (eq? pare %stream-null)
> +               (write-char #\> port)
> +               (let* ((kar (stream-kar pare))
> +                      (kar-value (stream-promise-val kar)))
> +                 (write-char #\space port)
> +                 (case (stream-value-tag kar-value)
> +                   ((eager) (write (stream-value-proc kar-value) port))
> +                   ((lazy)  (write-char #\? port)))
> +                 (loop (stream-kdr pare))))))
> +        ((lazy)
> +         (display " ...>" port))))))

The clarity of this code could greatly benefit from some helper
procedures.  One possibility would be a procedure that takes a promise
and two continuation arguments: one to call if the promise has already
been computed, and another to call if it has not yet been.  Another
possibility would be to simply have a predicate that tells whether a
promise has already been computed.

> diff --git a/module/srfi/srfi-45.scm b/module/srfi/srfi-45.scm
> index 5194770..ae08f9b 100644
> --- a/module/srfi/srfi-45.scm
> +++ b/module/srfi/srfi-45.scm
> @@ -39,7 +39,8 @@
>               eager
>               promise?)
>    #:replace (delay force promise?)
> -  #:use-module (srfi srfi-9))
> +  #:use-module (srfi srfi-9)
> +  #:use-module (srfi srfi-9 gnu))
>  
>  (cond-expand-provide (current-module) '(srfi-45))
>  
> @@ -76,3 +77,8 @@
>  ;; (*) These two lines re-fetch and check the original promise in case
>  ;;     the first line of the let* caused it to be forced.  For an example
>  ;;     where this happens, see reentrancy test 3 below.
> +
> +(set-record-type-printer! promise
> +  (lambda (promise port)
> +    (define content (promise-val promise))
> +    (format port "#<~a ~s>" (value-tag content) (value-proc content))))

I'd like to hear opinions on how to print promises.  Here are mine:

* I don't like #<eager ...> and #<lazy #<procedure ...>>.

* Both forms should start with #<promise ...>, because from a user's
  point of view that is the type.

* We should avoid printing "#<procedure" in there.  Some kind of
  location tag would be useful, but let's make it more compact.

* I suspect 'eager' and 'lazy' are likely to confuse most users, because
  they don't coincide with their own uses of 'lazy' and 'eager' as found
  in the public API.

What do other people think?

      Mark

Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9

[Chris K. Jester-Young]

Hi Mark,

On Fri, Apr 05, 2013 at 02:48:29AM -0400, Mark H Weaver wrote:
> Thanks for working on this, but in the future, please post your proposed
> patches for review before pushing to stable-2.0 or master.

You're right, that was a lapse of judgement on my part. Sorry.

> > -(define %stream-null '(stream . null))
> > +(define %stream-null (cons 'stream 'null))
> 
> This change is not mentioned in the commit message.
> Ideally, it should have been a separate commit, IMO.

Fair enough. If you prefer, I can revert this last commit and then
make this a separate commit. That will make the git blame a bit
tidier, at the expense of having a revert commit in the log.

> The clarity of this code could greatly benefit from some helper
> procedures.  One possibility would be a procedure that takes a promise
> and two continuation arguments: one to call if the promise has already
> been computed, and another to call if it has not yet been.  Another
> possibility would be to simply have a predicate that tells whether a
> promise has already been computed.

I was actually mimicking the style used in the SRFI 45 reference
implementation of delay. If we change this here, we should also
correspondingly change delay. The two continuations thing is probably
worth trying.

> I'd like to hear opinions on how to print promises.  Here are mine:
> 
> * I don't like #<eager ...> and #<lazy #<procedure ...>>.
> 
> * Both forms should start with #<promise ...>, because from a user's
>   point of view that is the type.

Fair enough. I was recalling your comment about how we should make SRFI
45 promises print slightly differently to core promises, so that was the
most succinct way I could imagine.

> * We should avoid printing "#<procedure" in there.  Some kind of
>   location tag would be useful, but let's make it more compact.

Right, and if we fix that here, we should do the same for core promises
(which does indeed print "#<procedure").

> * I suspect 'eager' and 'lazy' are likely to confuse most users, because
>   they don't coincide with their own uses of 'lazy' and 'eager' as found
>   in the public API.

If we don't print "eager" or "lazy" in the display, do we still need a
way to unambiguously identify between the two states? Right now, with
core promises, you can never tell: if it shows a procedure, that could
be the delayed thunk, or the result of calling such.

Thanks for your feedback! I look forward to hearing about which approach
we should take to your concerns outlined above, and implementing them.

Cheers,
Chris.

Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9

[Chris K. Jester-Young]

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

On Fri, Apr 05, 2013 at 08:36:51AM -0400, Chris K. Jester-Young wrote:
> > The clarity of this code could greatly benefit from some helper
> > procedures.  One possibility would be a procedure that takes a promise
> > and two continuation arguments: one to call if the promise has already
> > been computed, and another to call if it has not yet been.  Another
> > possibility would be to simply have a predicate that tells whether a
> > promise has already been computed.
> 
> I was actually mimicking the style used in the SRFI 45 reference
> implementation of delay. If we change this here, we should also
> correspondingly change delay. The two continuations thing is probably
> worth trying.

Attached is a patch for implementing this. Whether this results in
"greatly benefitted clarity" is debateable, but it was worth a try. :-)

Cheers,
Chris.

[-- Attachment #2: 0001-Implement-stream-promise-visit.patch --]
[-- Type: message/rfc822, Size: 3895 bytes --]

From: Chris K. Jester-Young <cky944@gmail.com>
Subject: [PATCH] Implement stream-promise-visit.
Date: Fri, 5 Apr 2013 09:08:51 -0400

* module/srfi/srfi-41.scm (stream-promise-visit): New procedure for
  cleanly visiting a promise based on whether its value is materialised
  or not. Based on feedback from Mark H Weaver.
  (stream-force, <stream printer>): Use stream-promise-visit.
---
 module/srfi/srfi-41.scm |   60 +++++++++++++++++++++++++---------------------
 1 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/module/srfi/srfi-41.scm b/module/srfi/srfi-41.scm
index 243bd44..108592f 100644
--- a/module/srfi/srfi-41.scm
+++ b/module/srfi/srfi-41.scm
@@ -127,19 +127,25 @@
 (define-syntax-rule (stream-delay exp)
   (stream-lazy (stream-eager exp)))
 
+(define (stream-promise-visit promise on-eager on-lazy)
+  (define content (stream-promise-val promise))
+  (case (stream-value-tag content)
+    ((eager) (on-eager (stream-value-proc content)))
+    ((lazy)  (on-lazy (stream-value-proc content)))))
+
 (define (stream-force promise)
-  (let ((content (stream-promise-val promise)))
-    (case (stream-value-tag content)
-      ((eager) (stream-value-proc content))
-      ((lazy)  (let* ((promise* ((stream-value-proc content)))
-                      (content  (stream-promise-val promise)))
-                 (if (not (eqv? (stream-value-tag content) 'eager))
-                     (begin (stream-value-tag-set! content
-                                                   (stream-value-tag (stream-promise-val promise*)))
-                            (stream-value-proc-set! content
-                                                    (stream-value-proc (stream-promise-val promise*)))
-                            (stream-promise-val-set! promise* content)))
-                 (stream-force promise))))))
+  (stream-promise-visit promise
+    values
+    (lambda (proc)
+      (let* ((promise* (proc))
+             (content  (stream-promise-val promise)))
+        (if (not (eqv? (stream-value-tag content) 'eager))
+            (begin (stream-value-tag-set! content
+                                          (stream-value-tag (stream-promise-val promise*)))
+                   (stream-value-proc-set! content
+                                           (stream-value-proc (stream-promise-val promise*)))
+                   (stream-promise-val-set! promise* content)))
+        (stream-force promise)))))
 
 ;;
 ;; End of the copy of the code from srfi-45.scm
@@ -185,21 +191,21 @@
   (lambda (strm port)
     (display "#<stream" port)
     (let loop ((strm strm))
-      (define value (stream-promise-val strm))
-      (case (stream-value-tag value)
-        ((eager)
-         (let ((pare (stream-value-proc value)))
-           (if (eq? pare %stream-null)
-               (write-char #\> port)
-               (let* ((kar (stream-kar pare))
-                      (kar-value (stream-promise-val kar)))
-                 (write-char #\space port)
-                 (case (stream-value-tag kar-value)
-                   ((eager) (write (stream-value-proc kar-value) port))
-                   ((lazy)  (write-char #\? port)))
-                 (loop (stream-kdr pare))))))
-        ((lazy)
-         (display " ...>" port))))))
+      (stream-promise-visit strm
+        ;; eager
+        (lambda (pare)
+          (if (eq? pare %stream-null)
+              (write-char #\> port)
+              (begin
+                (write-char #\space port)
+                (stream-promise-visit (stream-kar pare)
+                  (cut write <> port)           ; eager
+                  (lambda (_)                   ; lazy
+                    (write-char #\? port)))
+                (loop (stream-kdr pare)))))
+        ;; lazy
+        (lambda (_)
+          (display " ...>" port))))))
 
 ;;; Derived stream functions and macros: (streams derived)
 
-- 
1.7.2.5

Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9

[Ludovic Courtès]

Mark H Weaver <mhw@netris.org> skribis:

> I'd like to hear opinions on how to print promises.  Here are mine:
>
> * I don't like #<eager ...> and #<lazy #<procedure ...>>.
>
> * Both forms should start with #<promise ...>, because from a user's
>   point of view that is the type.

+1

> * We should avoid printing "#<procedure" in there.  Some kind of
>   location tag would be useful, but let's make it more compact.

Yeah, (number->string (object-address x) 16) should be enough.

Ludo’.