* Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 [not found] <E1UNwJ4-0003EO-Dd@vcs.savannah.gnu.org> @ 2013-04-05 6:48 ` Mark H Weaver 2013-04-05 12:36 ` Chris K. Jester-Young 2013-04-06 22:49 ` Ludovic Courtès 0 siblings, 2 replies; 4+ messages in thread From: Mark H Weaver @ 2013-04-05 6:48 UTC (permalink / raw) To: Chris K. Jester-Young; +Cc: guile-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 2013-04-05 6:48 ` [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 Mark H Weaver @ 2013-04-05 12:36 ` Chris K. Jester-Young 2013-04-05 13:19 ` Chris K. Jester-Young 2013-04-06 22:49 ` Ludovic Courtès 1 sibling, 1 reply; 4+ messages in thread From: Chris K. Jester-Young @ 2013-04-05 12:36 UTC (permalink / raw) To: guile-devel 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 2013-04-05 12:36 ` Chris K. Jester-Young @ 2013-04-05 13:19 ` Chris K. Jester-Young 0 siblings, 0 replies; 4+ messages in thread From: Chris K. Jester-Young @ 2013-04-05 13:19 UTC (permalink / raw) To: guile-devel [-- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 2013-04-05 6:48 ` [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 Mark H Weaver 2013-04-05 12:36 ` Chris K. Jester-Young @ 2013-04-06 22:49 ` Ludovic Courtès 1 sibling, 0 replies; 4+ messages in thread From: Ludovic Courtès @ 2013-04-06 22:49 UTC (permalink / raw) To: guile-devel 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’. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-06 22:49 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <E1UNwJ4-0003EO-Dd@vcs.savannah.gnu.org> 2013-04-05 6:48 ` [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.7-291-g4a1cdc9 Mark H Weaver 2013-04-05 12:36 ` Chris K. Jester-Young 2013-04-05 13:19 ` Chris K. Jester-Young 2013-04-06 22:49 ` Ludovic Courtès
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).