Re: [PATCH] Add SRFI-25 implementation

[Ludovic Courtès]

Mark H Weaver <address@hidden> skribis:

> address@hidden (Ludovic Courtès) writes:
>
>> address@hidden (Ludovic Courtès) skribis:
>>
>>> Andreas Rottmann <address@hidden> skribis:
>>>
>>>> Adds an implementation of SRFI 25 on top of Guile's native arrays. The
>>>> implementation does not introduce a disjoint type; Guile arrays and
>>>> SRFI-25 arrays can be used interchangably, though with different, partly
>>>> conflicting APIs.
>>>>
>>>> * NEWS: Add preliminary, incomplete section on 2.0.12, noting the
>>>>   addition of SRFI-25.
>>>> * doc/ref/srfi-modules.texi (SRFI-25): New node.
>>>> * module/srfi/srfi-25.scm: New file.
>>>> * test-suite/tests/srfi-25.test: New file.
>>>> * module/Makefile.am:
>>>> * test-suite/Makefile.am: Add new files.
>>>> ---
>>>>  NEWS                          |  15 +-
>>>>  doc/ref/srfi-modules.texi     | 240 +++++++++++++++++++++-
>>>>  module/Makefile.am            |   1 +
>>>>  module/srfi/srfi-25.scm       | 159 +++++++++++++++
>>>>  test-suite/Makefile.am        |   3 +-
>>>>  test-suite/tests/srfi-25.test | 461 
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>  6 files changed, 876 insertions(+), 3 deletions(-)
>>>>  create mode 100644 module/srfi/srfi-25.scm
>>>>  create mode 100644 test-suite/tests/srfi-25.test
>>>
>>> If there are no objections, you can push this patch within a day or two
>>> to ‘stable-2.0’ (or I’ll do it if you happen to be unavailable.)
>>
>> Done in aaea5b2 with minor modifications in srfi-modules.texi to make it
>> more consistent with the rest of the manual.
>
> This commit broke the build, because it forgot to add srfi-25.scm.
>
> For now, I reverted the commit instead of fixing it, because there are a
> few other problems with srfi-25.scm which I had told Andreas about over
> IRC but never posted about.  Sorry about that.  I guess maybe that's why
> Andreas didn't respond more quickly to your earlier email.
>
> In a couple of places, it has code like this:
>
> +     (cond ((array? i)
> +            (apply array-ref array (array->index-list i)))
> +           ((vector? i)
> +            (apply array-ref array (vector->list i)))
>
> Since every vector is also considered an array, the second clause of the
> 'cond' is dead code.
>
> Also, it would be good to avoid consing in common cases in 'array-ref'
> and 'array-set!', by adding special cases for 1 and 2 dimensional array
> accesses, at least when the indices are given as direct arguments.
>
> Finally, the following line should be put somewhere in the module:
>
> (cond-expand-provide (current-module) '(srfi-25))

Too much asynchrony here. ;-)  I hope Andreas or you can look into it.

Ludo’.