unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: "Chris K. Jester-Young" <cky944@gmail.com>
Cc: guile-devel@gnu.org
Subject: Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9
Date: Fri, 05 Apr 2013 02:48:29 -0400	[thread overview]
Message-ID: <871uapwk02.fsf@tines.lan> (raw)
In-Reply-To: <E1UNwJ4-0003EO-Dd@vcs.savannah.gnu.org> (Chris K. Jester-Young's message of "Fri, 05 Apr 2013 02:23:10 +0000")

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



       reply	other threads:[~2013-04-05  6:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <E1UNwJ4-0003EO-Dd@vcs.savannah.gnu.org>
2013-04-05  6:48 ` Mark H Weaver [this message]
2013-04-05 12:36   ` [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 Chris K. Jester-Young
2013-04-05 13:19     ` Chris K. Jester-Young
2013-04-06 22:49   ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871uapwk02.fsf@tines.lan \
    --to=mhw@netris.org \
    --cc=cky944@gmail.com \
    --cc=guile-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).