unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: nly@disroot.org
Cc: guile-devel@gnu.org
Subject: Re: [PATCH] add SRFI: srfi-121; generators
Date: Mon, 01 Jul 2019 01:06:02 -0400	[thread overview]
Message-ID: <871rzaibt1.fsf@netris.org> (raw)
In-Reply-To: <3fb6ac24483457821130185b0e1f277c@disroot.org> (nly's message of "Mon, 01 Jul 2019 00:09:08 +0000")

Hi Amar,

> SRFI-121 Generators.
>
> All tests(49/49) are passing in my testing. I am not sure if the tests
> file is put in the correct place.
>
> From 0352f9be13aba1e8acc9a8f700f3673334d48d28 Mon Sep 17 00:00:00 2001
> From: Amar Singh <nly@disroot.org>
> Date: Mon, 1 Jul 2019 05:14:53 +0530
> Subject: [PATCH] add SRFI: srfi-121; generators

Thank you for this, but I'm sorry to say that I'm not inclined to add
this implementation of SRFI-121 to Guile.  The first sentence of
SRFI-121's "Rationale" section states:

  The main purpose of generators is high performance.

and I agree.  In fact, I would strongly discourage its use except in
cases where efficiency demands it.  Like hash tables, generators are
fundamentally imperative constructs, and they will tend to force code
built with them to be written in an imperative style.

With this in mind, if SRFI-121 is to be added to Guile, it should be a
high performance implementation.  The implementation that you provided,
which I guess is primarily taken from the sample implementation, is far
too inefficient, at least on Guile.

Also, the provided implementations of 'generator-find', 'generator-any'
and 'generator-every' are incorrect:

> +;; generator-find
> +(define (generator-find pred g)
> +  (let loop ((v (g)))
> +   ; A literal interpretation might say it only terminates on #eof if (pred #eof) but I think this makes more sense...
> +   (if (or (pred v) (eof-object? v))
> +     v
> +     (loop (g)))))

The SRFI says:

  Returns the first item from the generator gen that satisfies the
  predicate pred, or #f if no such item is found before gen is
  exhausted.

This specification is quite clear that #f should be returned if EOF is
encountered (i.e. "gen is exhausted").  Here, you are returning EOF in
that case, which is incorrect.

Also, I think that 'pred' should never be applied to EOF.

> +;; generator-any
> +(define (generator-any pred g)
> +  (let loop ((v (g)))
> +   (if (eof-object? v)
> +     #f
> +     (if (pred v)
> +       #t
> +       (loop (g))))))

This is incorrect.  If (pred v) returns a true value, the value returned
by (pred v) should be returned, not #t.

> +;; generator-every
> +(define (generator-every pred g)
> +  (let loop ((v (g)))
> +   (if (eof-object? v)
> +     #t
> +     (if (pred v)
> +       (loop (g))
> +       #f ; the spec would have me return #f, but I think it must simply be wrong...
> +       ))))

I can't make sense of the comment above.  Anyway, this is also
incorrect.  The specification is clear that if the generator is
exhausted after returning some items, (pred LAST-ITEM) should be
returned.  The only case where #t is returned is if the generator was
exhausted before calling 'generator-every'.

I might write my own implementation of SRFI-121 from scratch and add it
to Guile.

     Regards,
       Mark



  reply	other threads:[~2019-07-01  5:06 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-01  0:09 [PATCH] add SRFI: srfi-121; generators nly
2019-07-01  5:06 ` Mark H Weaver [this message]
2019-07-01  6:00   ` Mark H Weaver
2019-07-01  6:21     ` Amar Singh
  -- strict thread matches above, loose matches on Subject: below --
2020-08-01  3:42 John Cowan
2020-08-02 22:39 ` Mark H Weaver
     [not found] <mailman.61.1596470427.10776.guile-devel@gnu.org>
2020-08-03 19:41 ` Marc Nieper-Wißkirchen
2020-08-04 12:48   ` Marc Nieper-Wißkirchen
2020-08-04 15:24   ` John Cowan
2020-08-04 15:58     ` Marc Nieper-Wißkirchen
2020-08-04 17:24       ` Dr. Arne Babenhauserheide
2021-01-21 18:39 John Cowan
2021-01-23  0:58 ` Mark H Weaver
2021-01-23  2:14   ` Shiro Kawai
2021-01-23  2:18     ` Arthur A. Gleckler
2021-01-23  6:37       ` Mark H Weaver
2021-01-26  3:29         ` John Cowan
2021-01-26  6:48           ` Linus Björnstam
2021-01-26  6:49             ` Linus Björnstam
2021-01-26  7:14             ` Marc Nieper-Wißkirchen
2021-01-26 11:54               ` Linus Björnstam
2021-04-08 15:53                 ` Arthur A. Gleckler
2021-04-11  6:52                   ` Linus Björnstam
2021-04-11 16:17                     ` Arthur A. Gleckler

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=871rzaibt1.fsf@netris.org \
    --to=mhw@netris.org \
    --cc=guile-devel@gnu.org \
    --cc=nly@disroot.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).