Re: [PATCH] Add preliminary versions of the R7RS libraries along with documentation and tests

[Mark H Weaver <mhw@netris.org>]

Hi Freja,

Freja Nordsiek <fnordsie@gmail.com> writes:
> I went through the patch and cleaned up a lot of little things (line
> length in documentation, two spaces between sentences in
> documentation, trailing spaces, etc.), made a proper commit
> comment/log, and replaced the very ugly bytevector output port hack
> with a cleaner and simpler hack (just used string output ports with
> get-output-bytevector reading the input with the ISO-8859-1 encoding
> and then converting to a bytevector).

I very much appreciate the documentation work you've done here, but I'm
puzzled why you would rewrite most of the code I wrote for the r7rs-wip
branch.  Now that you've done this, whose code would you propose should
get dumped into the proverbial bit bucket?

A quick perusal of your code suggests that you did not take the same
care as I did to account for the sometimes subtle differences in
specified functionality of procedures of the same name.

For example, the R7RS 'map' procedure is specified to terminate when the
shortest list runs out, and also to allow circular lists as long as
there is at least one finite list among the arguments.  In my code, I
made sure that the 'map' in (scheme base) matches this specification.
Your code simply re-exports Guile's native 'map', which requires that
lists are finite and of the same length.

Your 'string->vector', 'vector->string', 'string-map', 'string-for-each'
and many similar procedures are written in a very simple way that
involves a lot of needless allocation of intermediate data structures
such as 'rest' argument lists, and lists of characters from a string,
whereas my versions are written to be reasonably efficient.

Your 'finite?', 'infinite?' and 'nan?' procedures are simply re-exported
from Guile's core, although the semantics are different.  The guile
versions are only defined on real numbers, but the R7RS variants are
defined on all complex numbers.  

R7RS 'bytevector-copy' can accept up to three arguments (bv start end),
but you simply reuse the one from R6RS which only accepts one argument.

Your 'vector-map' and 'vector-for-each' are simply wrong.  You re-export
the same procedures from SRFI-43, but in fact they differ from the R7RS
semantics in a crucial respect: the SRFI-43 versions of 'vector-map' and
'vector-for-each' call the provided procedure 'f' with the 'index' as an
additional argument, whereas the R7RS versions do not.

I could go on.

Having said all of this, I'm grateful for your documentation.  How would
you feel about preparing a patch to add your documentation to the
existing r7rs-wip branch?

     Regards,
       Mark