[Guile-commits] 06/27: Simplify interfaces to scm_vector_elements and scm_vector_writable_elements

[Daniel Llorens]

lloda pushed a commit to branch wip-vector-cleanup
in repository guile.

commit eb2d2c1a41f3cbdee62b2b68f2fd3c9efaf5ca15
Author: Daniel Llorens <address@hidden>
AuthorDate: Mon Feb 3 16:18:41 2020 +0100

    Simplify interfaces to scm_vector_elements and scm_vector_writable_elements
    
    * vectors.h: As stated. Don't include array-handle.h.
    * vectors.c (scm_vector_elements, scm_vector_writable_elements):
      Reimplement.
      (vector-move-right!, vector-move-left!): Replace uses of
      scm_vector_elements and scm_vector_writable_elements on array types.
    * doc/ref/api-data.texi: Fix doc for scm_vector_elements and
      scm_vector_writable_elements.
    * libguile/sort.c (stable-sort!): Fix use of
      scm_vector_writable_elements.
---
 ...-array-fixes.txt => NEWS-wip-vector-cleanup.txt |  28 +++-
 doc/ref/api-data.texi                              |  47 +++---
 libguile/sort.c                                    |   4 +-
 libguile/vectors.c                                 | 180 ++++++++++++---------
 libguile/vectors.h                                 |  10 +-
 5 files changed, 142 insertions(+), 127 deletions(-)

diff --git a/NEWS-array-fixes.txt b/NEWS-wip-vector-cleanup.txt
similarity index 70%
rename from NEWS-array-fixes.txt
rename to NEWS-wip-vector-cleanup.txt
index 3cc1c97..c492668 100644
--- a/NEWS-array-fixes.txt
+++ b/NEWS-wip-vector-cleanup.txt
@@ -1,4 +1,4 @@
-(vector-array-handle-plan for Guile 3.0) -*- coding: utf-8; mode: org; -*-
+(wip-vector-cleanup for Guile 3.0) -*- coding: utf-8; mode: org; -*-
 
 TBA to NEWS for this branch.
 
@@ -12,11 +12,6 @@ Guile and continue working with older versions, at least 
back to 2.2.
 
 Use array->list and array-copy (from (ice-9 arrays)) on general arrays.
 
-** scm_vector_elements / scm_vector_writable_elements require a true vector 
argument.
-
-Use scm_array_get_handle and scm_array_handle_elements /
-scm_array_handle_writable_elements on general arrays.
-
 ** scm_is_simple_vector has been removed.
 
 Use scm_is_vector instead.
@@ -37,15 +32,32 @@ calls.
 
 Use SCM_VECTOR_REF, SCM_VECTOR_SET and SCM_VECTOR_LENGTH instead.
 
+** scm_vector_elements / scm_vector_writable_elements take a single argument 
that must be a vector.
+
+Use scm_array_get_handle and scm_array_handle_elements /
+scm_array_handle_writable_elements on general arrays.
+
+Use scm_c_vector_length to get the length of a vector.
+
 
 * Rationale / TODO
 
-** Fix API for scm_vector_elements scm_vector_writable_elements to [(SCM vec) 
-> pointer]
+** Fix API for scm_VECTOR_TYPE_elements scm_VECTOR_TYPE_writable_elements to 
[(SCM v) -> pointer]
 
   - incp is always one on these types, so it isn't needed
-  - no need to acquire/release an array handle for VECTOR_TYPE
+  - no need to release an array handle for VECTOR_TYPE
   - remove the dependence of vector.c bitvector.c bytevector.c etc. on 
array-handle.h
 
+** Fix API for scm_vector_elements scm_vector_writable_elements to [(SCM v) -> 
pointer]
+
+  - the matching functions for srfi-4 VECTOR_TYPEs have never allowed passing
+    non-VECTOR-TYPE rank-1 arrays. There is no reason for these to allow it; if
+    v is an array, use the array interface.
+
+** Fix API for scm_bitvector_elements scm_bitvector_writable_elements to [(SCM 
v) -> pointer]
+
+   - same as for scm_vector_elements scm_vector_writable_elements.
+
 ** Remove scm_is_simple_vector, fix scm_is_array_p, rename SIMPLE_VECTOR  to 
VECTOR, etc.
 
   - just cleaning house
diff --git a/doc/ref/api-data.texi b/doc/ref/api-data.texi
index 805bb36..3b4d047 100644
--- a/doc/ref/api-data.texi
+++ b/doc/ref/api-data.texi
@@ -6441,32 +6441,23 @@ Sets the element at position @var{idx} in the vector 
@var{vec} to
 @var{val}.  No type or range checking is done.
 @end deftypefn
 
-@deftypefn {C Function} {const SCM *} scm_vector_elements (SCM vec, 
scm_t_array_handle *handle, size_t *lenp, ssize_t *incp)
-Acquire a handle for the vector @var{vec} and return a pointer to the
-elements of it.  This pointer can only be used to read the elements of
-@var{vec}.  When @var{vec} is not a vector, an error is signaled.  The
-handle must eventually be released with
-@code{scm_array_handle_release}.
+@deftypefn {C Function} {const SCM *} scm_vector_elements (SCM vec)
+Return a pointer to the elements of @var{vec}.  This pointer can only be
+used to read the elements of @var{vec}.  When @var{vec} is not a vector,
+an error is signaled.
 
-The variables pointed to by @var{lenp} and @var{incp} are filled with
-the number of elements of the vector and the increment (number of
-elements) between successive elements, respectively.  
+For use with general arrays, see @code{scm_array_handle_elements}.
 
 The following example shows the typical way to use this function.  It
 creates a list of all elements of @var{vec} (in reverse order).
 
 @example
-scm_t_array_handle handle;
-size_t i, len;
-ssize_t inc;
-const SCM *elt;
-SCM list;
-
-elt = scm_vector_elements (vec, &handle, &len, &inc);
-list = SCM_EOL;
-for (i = 0; i < len; i++, elt += inc)
-  list = scm_cons (*elt, list);
-scm_array_handle_release (&handle);
+size_t len = scm_c_vector_length (vec);
+const SCM *elt = scm_vector_elements (vec);
+
+SCM list = SCM_EOL;
+for (size_t i = 0; i < len; ++i)
+  list = scm_cons (elt[i], list);
 @end example
 
 @end deftypefn
@@ -6475,19 +6466,17 @@ scm_array_handle_release (&handle);
 Like @code{scm_vector_elements} but the pointer can be used to modify
 the vector.
 
+For use with general arrays, see @code{scm_array_handle_writable_elements}.
+
 The following example shows the typical way to use this function.  It
-fills a vector with @code{#t}.
+fills a vector with @code{#t} (but see @code{vector-fill!}).
 
 @example
-scm_t_array_handle handle;
-size_t i, len;
-ssize_t inc;
-SCM *elt;
+size_t len = scm_c_vector_length (vec);
+SCM *elt = scm_vector_writable_elements (vec);
 
-elt = scm_vector_writable_elements (vec, &handle, &len, &inc);
-for (i = 0; i < len; i++, elt += inc)
-  *elt = SCM_BOOL_T;
-scm_array_handle_release (&handle);
+for (size_t i = 0; i < len; ++i)
+  elt[i] = SCM_BOOL_T;
 @end example
 
 @end deftypefn
diff --git a/libguile/sort.c b/libguile/sort.c
index 0827ebe..977a0b6 100644
--- a/libguile/sort.c
+++ b/libguile/sort.c
@@ -577,12 +577,10 @@ SCM_DEFINE (scm_stable_sort_x, "stable-sort!", 2, 0, 0,
         }
 
       SCM temp = scm_c_make_vector (len, SCM_UNDEFINED);
-      scm_t_array_handle temp_handle;
-      SCM *temp_elts = scm_vector_writable_elements (temp, &temp_handle, NULL, 
NULL);
+      SCM *temp_elts = scm_vector_writable_elements (temp);
 
       scm_merge_vector_step (vec_elts, temp_elts, less, 0, len-1, inc);
 
-      scm_array_handle_release (&temp_handle);
       scm_array_handle_release (&vec_handle);
 
       return items;
diff --git a/libguile/vectors.c b/libguile/vectors.c
index 30af659..d481b14 100644
--- a/libguile/vectors.c
+++ b/libguile/vectors.c
@@ -24,7 +24,6 @@
 # include <config.h>
 #endif
 
-#include "array-handle.h"
 #include "bdw-gc.h"
 #include "boolean.h"
 #include "eq.h"
@@ -35,9 +34,10 @@
 #include "vectors.h"
 #include <string.h>
 
+// You're next
+#include "array-handle.h"
 #include "generalized-vectors.h"
 
-
 
 
 #define VECTOR_MAX_LENGTH (SCM_T_BITS_MAX >> 8)
@@ -56,42 +56,20 @@ scm_is_vector (SCM obj)
 }
 
 const SCM *
-scm_vector_elements (SCM vec, scm_t_array_handle *h,
-                    size_t *lenp, ssize_t *incp)
+scm_vector_elements (SCM vec)
 #define FUNC_NAME "scm_vector_elements"
 {
   SCM_VALIDATE_VECTOR (1, vec);
-  scm_array_get_handle (vec, h);
-  
-  if (lenp)
-    {
-      scm_t_array_dim *dim = scm_array_handle_dims (h);
-      *lenp = dim->ubnd - dim->lbnd + 1;
-    }
-  if (incp)
-    *incp = 1;
-  
-  return scm_array_handle_elements (h);
+  return SCM_I_VECTOR_ELTS (vec);
 }
 #undef FUNC_NAME
 
 SCM *
-scm_vector_writable_elements (SCM vec, scm_t_array_handle *h,
-                             size_t *lenp, ssize_t *incp)
+scm_vector_writable_elements (SCM vec)
 #define FUNC_NAME "scm_vector_writable_elements"
 {
   SCM_VALIDATE_MUTABLE_VECTOR (1, vec);
-  scm_array_get_handle (vec, h);
-  
-  if (lenp)
-    {
-      scm_t_array_dim *dim = scm_array_handle_dims (h);
-      *lenp = dim->ubnd - dim->lbnd + 1;
-    }
-  if (incp)
-    *incp = 1;
-  
-  return scm_array_handle_writable_elements (h);
+  return SCM_I_VECTOR_WELTS (vec);
 }
 #undef FUNC_NAME
 
@@ -353,6 +331,8 @@ scm_i_vector_equal_p (SCM x, SCM y)
   return SCM_BOOL_T;
 }
 
+// These functions are used by vector-copy!
+// FIXME split into vector- and array- (?)
 
 SCM_DEFINE (scm_vector_move_left_x, "vector-move-left!", 5, 0, 0, 
             (SCM vec1, SCM start1, SCM end1, SCM vec2, SCM start2),
@@ -365,31 +345,51 @@ SCM_DEFINE (scm_vector_move_left_x, "vector-move-left!", 
5, 0, 0,
            "@var{start1} is greater than @var{start2}.")
 #define FUNC_NAME s_scm_vector_move_left_x
 {
-  scm_t_array_handle handle1, handle2;
-  const SCM *elts1;
-  SCM *elts2;
-  size_t len1, len2;
-  ssize_t inc1, inc2;
-  size_t i, j, e;
-  
-  elts1 = scm_vector_elements (vec1, &handle1, &len1, &inc1);
-  elts2 = scm_vector_writable_elements (vec2, &handle2, &len2, &inc2);
-
-  i = scm_to_unsigned_integer (start1, 0, len1);
-  e = scm_to_unsigned_integer (end1, i, len1);
-  SCM_ASSERT_RANGE (SCM_ARG3, end1, (e-i) <= len2);
-  j = scm_to_unsigned_integer (start2, 0, len2);
-  SCM_ASSERT_RANGE (SCM_ARG5, start2, j <= len2 - (e - i));
+  scm_t_array_handle handle1;
+  scm_array_get_handle (vec1, &handle1);
+  if (1 != scm_array_handle_rank (&handle1))
+    {
+      scm_array_handle_release (&handle1);
+      SCM_WRONG_TYPE_ARG (1, vec1);
+    }
+  else
+    {
+      scm_t_array_handle handle2;
+      scm_array_get_handle (vec2, &handle2);
+      if (1 != scm_array_handle_rank (&handle2))
+        {
+          scm_array_handle_release (&handle1);
+          scm_array_handle_release (&handle2);
+          SCM_WRONG_TYPE_ARG (4, vec2);
+        }
+      else
+        {
+          const SCM *elts1 = scm_array_handle_elements (&handle1);
+          SCM *elts2 = scm_array_handle_writable_elements (&handle2);
+          scm_t_array_dim *dims1 = scm_array_handle_dims (&handle1);
+          scm_t_array_dim *dims2 = scm_array_handle_dims (&handle2);
+          size_t len1 = dims1->ubnd + 1 - dims1->lbnd;
+          size_t len2 = dims2->ubnd + 1 - dims2->lbnd;
+          ssize_t inc1 = dims1->inc;
+          ssize_t inc2 = dims2->inc;
+
+          size_t i, j, e;
+          i = scm_to_unsigned_integer (start1, 0, len1);
+          e = scm_to_unsigned_integer (end1, i, len1);
+          SCM_ASSERT_RANGE (SCM_ARG3, end1, (e-i) <= len2);
+          j = scm_to_unsigned_integer (start2, 0, len2);
+          SCM_ASSERT_RANGE (SCM_ARG5, start2, j <= len2 - (e - i));
   
-  i *= inc1;
-  e *= inc1;
-  j *= inc2;
-  for (; i < e; i += inc1, j += inc2)
-    elts2[j] = elts1[i];
-
-  scm_array_handle_release (&handle2);
-  scm_array_handle_release (&handle1);
-
+          i *= inc1;
+          e *= inc1;
+          j *= inc2;
+          for (; i < e; i += inc1, j += inc2)
+            elts2[j] = elts1[i];
+
+          scm_array_handle_release (&handle2);
+          scm_array_handle_release (&handle1);
+        }
+    }
   return SCM_UNSPECIFIED;
 }
 #undef FUNC_NAME
@@ -405,37 +405,57 @@ SCM_DEFINE (scm_vector_move_right_x, 
"vector-move-right!", 5, 0, 0,
            "@var{start1} is less than @var{start2}.")
 #define FUNC_NAME s_scm_vector_move_right_x
 {
-  scm_t_array_handle handle1, handle2;
-  const SCM *elts1;
-  SCM *elts2;
-  size_t len1, len2;
-  ssize_t inc1, inc2;
-  size_t i, j, e;
-  
-  elts1 = scm_vector_elements (vec1, &handle1, &len1, &inc1);
-  elts2 = scm_vector_writable_elements (vec2, &handle2, &len2, &inc2);
-
-  i = scm_to_unsigned_integer (start1, 0, len1);
-  e = scm_to_unsigned_integer (end1, i, len1);
-  SCM_ASSERT_RANGE (SCM_ARG3, end1, (e-i) <= len2);
-  j = scm_to_unsigned_integer (start2, 0, len2);
-  SCM_ASSERT_RANGE (SCM_ARG5, start2, j <= len2 - (e - i));
+  scm_t_array_handle handle1;
+  scm_array_get_handle (vec1, &handle1);
+  if (1 != scm_array_handle_rank (&handle1))
+    {
+      scm_array_handle_release (&handle1);
+      SCM_WRONG_TYPE_ARG (1, vec1);
+    }
+  else
+    {
+      scm_t_array_handle handle2;
+      scm_array_get_handle (vec2, &handle2);
+      if (1 != scm_array_handle_rank (&handle2))
+        {
+          scm_array_handle_release (&handle1);
+          scm_array_handle_release (&handle2);
+          SCM_WRONG_TYPE_ARG (4, vec2);
+        }
+      else
+        {
+          const SCM *elts1 = scm_array_handle_elements (&handle1);
+          SCM *elts2 = scm_array_handle_writable_elements (&handle2);
+          scm_t_array_dim *dims1 = scm_array_handle_dims (&handle1);
+          scm_t_array_dim *dims2 = scm_array_handle_dims (&handle2);
+          size_t len1 = dims1->ubnd + 1 - dims1->lbnd;
+          size_t len2 = dims2->ubnd + 1 - dims2->lbnd;
+          ssize_t inc1 = dims1->inc;
+          ssize_t inc2 = dims2->inc;
+
+          size_t i, j, e;
+          i = scm_to_unsigned_integer (start1, 0, len1);
+          e = scm_to_unsigned_integer (end1, i, len1);
+          SCM_ASSERT_RANGE (SCM_ARG3, end1, (e-i) <= len2);
+          j = scm_to_unsigned_integer (start2, 0, len2);
+          SCM_ASSERT_RANGE (SCM_ARG5, start2, j <= len2 - (e - i));
   
-  j += (e - i);
+          j += (e - i);
   
-  i *= inc1;
-  e *= inc1;
-  j *= inc2;
-  while (i < e)
-    {
-      e -= inc1;
-      j -= inc2;
-      elts2[j] = elts1[e];
+          i *= inc1;
+          e *= inc1;
+          j *= inc2;
+          while (i < e)
+            {
+              e -= inc1;
+              j -= inc2;
+              elts2[j] = elts1[e];
+            }
+
+          scm_array_handle_release (&handle2);
+          scm_array_handle_release (&handle1);
+        }
     }
-
-  scm_array_handle_release (&handle2);
-  scm_array_handle_release (&handle1);
-
   return SCM_UNSPECIFIED;
 }
 #undef FUNC_NAME
diff --git a/libguile/vectors.h b/libguile/vectors.h
index 48647a8..0fa320b 100644
--- a/libguile/vectors.h
+++ b/libguile/vectors.h
@@ -22,9 +22,9 @@
 
 
 
-#include "libguile/array-handle.h"
 #include <libguile/error.h>
 #include "libguile/gc.h"
+#include "numbers.h"
 
 
 
@@ -47,12 +47,8 @@ SCM_API SCM scm_c_make_vector (size_t len, SCM fill);
 SCM_API size_t scm_c_vector_length (SCM vec);
 SCM_API SCM scm_c_vector_ref (SCM vec, size_t k);
 SCM_API void scm_c_vector_set_x (SCM vec, size_t k, SCM obj);
-SCM_API const SCM *scm_vector_elements (SCM vec,
-                                       scm_t_array_handle *h,
-                                       size_t *lenp, ssize_t *incp);
-SCM_API SCM *scm_vector_writable_elements (SCM vec,
-                                          scm_t_array_handle *h,
-                                          size_t *lenp, ssize_t *incp);
+SCM_API const SCM *scm_vector_elements (SCM vec);
+SCM_API SCM *scm_vector_writable_elements (SCM vec);
 
 #define SCM_VALIDATE_VECTOR(pos, v) \
   do { \