Re: [PATCH] add SRFI: srfi-121; generators

[Mark H Weaver]

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