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

[Freja Nordsiek]

Mark,


On Mon, May 29, 2017 at 1:12 AM, Mark H Weaver <address@hidden> wrote:
> Hi Freja,
>
> Freja Nordsiek <address@hidden> 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

Mark


Wow, that is a lot I missed. Then, obviously, what I wrote needs to go
to the proverbial bit-bucket.

As far as splitting it into parts and discarding the scheme modules
and keeping the documentation, that sounds like a good idea. I just
did a quick perusal of the r7rs-wip branch and it does not seem to
have any R7RS unit tests. Did I miss any? If not, the test code, as
limited as it is, might also be useful.

As for the question/puzzlement of why I wrote all of this, that is
complicated, and kind of silly in retrospect. The r7rs-wip branch
looked like it was most of the way to complete but was three years
behind the master branch and thus seemed like it was possibly dead for
unknown reasons (obviously, in retrospect, I should have just emailed
you and the others who had worked on it and asked). So, I figured I
would try to aim for adding just the R7RS libraries to master branch
with a minimal patch (add as little as possible) and not change
anything else in Guile (and thus not get the rest of the R7RS features
and syntax) to make it very easy to include. As for why not taking the
code directly from r7rs-wip except in a couple places, it was hard to
tell what did not depend on any changes to the rest of Guile and what
did without going through the documentation each generated and compare
them line by line or going through each different commit one by one
(would be necessary with them being 3 years out of sync), which was a
task too big to undertake. Honestly, I should have just emailed the
list and what not and asked about the status of the r7rs-wip branch
and why it stalled, and then go from there (e.g. write the
documentation and possibly tests). I ended up duplicating a lot of
effort in a sloppy way.

I will split the documentation and possibly the tests out into their
own patches and modify them to work with r7rs-wip branch instead of
master branch.


Freja