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 <address@hidden>
> 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