Re: [PATCH] Implement SRFI-43 Vector Library

[Ludovic Courtès]

Mark H Weaver <address@hidden> skribis:

> From 35662c15740a16346a7cc62ae8a87c2cd4c31079 Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <address@hidden>
> Date: Mon, 27 Jan 2014 17:17:23 -0500
> Subject: [PATCH] Implement SRFI-43 Vector Library.
>
> * module/srfi/srfi-43.scm: New file.
> * module/Makefile.am (SRFI_SOURCES): Add module/srfi/srfi-43.scm.
> * test-suite/tests/srfi-43.test: New file.
> * test-suite/Makefile.am (SCM_TESTS): Add test-suite/tests/srfi-43.test.
> * doc/ref/srfi-modules.texi (SRFI-43, SRFI-43 Constructors)
>   (SRFI-43 Predicates, SRFI-43 Selectors, SRFI-43 Iteration)
>   (SRFI-43 Searching, SRFI-43 Mutators, SRFI-43 Conversion): New nodes.

Awesome!

Nothing to say overall, just a little bit of nitpicking:

> +(define-module (srfi srfi-43)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-8)
> +  #:re-export (make-vector vector vector? vector-ref vector-set!
> +                           vector-length)

I think ‘vector-length’ should be aligned with ‘make-vector’.

> +  #:replace (vector-copy vector-fill! list->vector vector->list)
> +  #:export (vector-empty? vector= vector-unfold vector-unfold-right
> +                          vector-reverse-copy

Likewise.

> +(define vector-unfold
> +  (case-lambda
> +    "(vector-unfold f length initial-seed ...) -> vector
> +
> +The fundamental vector constructor.  Create a vector whose length is
> +LENGTH and iterates across each index k from 0 up to LENGTH - 1,
> +applying F at each iteration to the current index and current seeds,
> +in that order, to receive n + 1 values: first, the element to put in
> +the kth slot of the new vector and n new seeds for the next iteration.
> +It is an error for the number of seeds to vary between iterations."

I think it’s not necessary to repeat the procedure prototype in the
docstring: we never do that, and the documentation system would print it
anyway.

(BTW, I recently learned that Texinfo in docstring *is* actually
interpreted if (texinfo reflection) is loaded.  Tip of the day.  ;-))

Nice that you used ‘case-lambda’ to specialize the zero-seed and
one-seed cases; I wonder if it’s really necessary to specialize the
two-seed case, though, as it’s less common, I think.  WDYT?

Thanks!

Ludo’.