guile-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Guile-commits] 12/23: Remove generalized vector support for vector-move


From: Daniel Llorens
Subject: [Guile-commits] 12/23: Remove generalized vector support for vector-move-right!, vector-move-left!
Date: Mon, 10 Feb 2020 03:57:01 -0500 (EST)

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

commit cbf360ed7c3e53ab938be67acf89222b454facd2
Author: Daniel Llorens <address@hidden>
AuthorDate: Wed Feb 5 16:35:05 2020 +0100

    Remove generalized vector support for vector-move-right!, vector-move-left!
    
    This support was buggy and not advertised, so it has been removed.
---
 NEWS-wip-vector-cleanup.txt |  41 +++++++++------
 libguile/bitvectors.c       |   1 -
 libguile/vectors.c          | 122 +++++++++++---------------------------------
 3 files changed, 54 insertions(+), 110 deletions(-)

diff --git a/NEWS-wip-vector-cleanup.txt b/NEWS-wip-vector-cleanup.txt
index 1eb40ff..c0cbaf2 100644
--- a/NEWS-wip-vector-cleanup.txt
+++ b/NEWS-wip-vector-cleanup.txt
@@ -1,7 +1,6 @@
 (wip-vector-cleanup for Guile 3.0) -*- coding: utf-8; mode: org; -*-
 
 TBA to NEWS for this branch.
-
  
 * Forward incompatible changes
 
@@ -42,33 +41,41 @@ Use scm_array_get_handle and scm_array_handle_bit_elements 
/ scm_array_handle_bi
 Use scm_array1_bit_elements (NEW) / scm_array1_bit_writable_elements (NEW) on 
rank-1 bit arrays.
 
 
+* Bugfixes introducing incompatibility
+
+** vector-move-right! vector-move-left! require true vector arguments
+
+These functions weren't advertised to work on non-vector arrays. They did try 
to, but only incorrectly. For example:
+
+  (define a (vector 1 2 3 4 5))
+  (define b (make-array 0 '(1 5)))
+  (vector-move-right! a 0 2 b 2)
+  b
+  => #1@1(0 0 1 2 0)
+
+instead of the correct result #1@1(0 1 2 0 0). This buggy support has been 
removed.
+
+
 * Rationale / TODO
 
 ** Status as of 3.0.0
   
-  - The _elements functions require the array handle interface even for true 
vectors, even though the handle is unnecessary. This creates a burden (having 
to declare, release, etc).
+  - The _elements functions require the array handle interface even for true 
vectors, when all of handle, inc and off are unnecessary. This creates a burden 
(having to declare & release handles, etc).
   - The srfi-4 _elements functions don't accept arbitrary rank-1 xxxarray even 
though they require the array handle interface (inc, handle are superfluous).
 
-** Plan
+** Plan [3/7]
 
-  - Provide scm_VTYPE_(writable_)elements with signature [(SCM) -> pointer] 
for all vector types.
-  - Provide scm_array1_VTYPE_(writable_)elements with signature [(SCM, 
&handle, ...) -> pointer] for all vector types. These replace the old 
scm_VTYPE_(writable_)elements but will be available on the array API and not on 
the xxxvector APIs.
-  - remove the dependence of vector.c bitvector.c bytevector.c etc. on 
array-handle.h
-  - remove the double dependence of array-ref -> VECTOR-TYPE-ref -> 
array_handle
-    use seen in scm_array_get_handle.
-  - bug: scm_bitvector_elements doesn't select for type = bit
-  - bug: bit-set*! already requires second arg to be true bitvector
-  - bug: setaffinity in posix.c failed to release the mask handle
+  - [X] Provide scm_VTYPE_(writable_)elements with signature [(SCM) -> 
pointer] for all vector types.
+  - [ ] Provide scm_array1_VTYPE_(writable_)elements with signature [(SCM, 
&handle, ...) -> pointer] for all vector types. These replace the old 
scm_VTYPE_(writable_)elements but will be available on the array API and not on 
the xxxvector APIs.
+  - [ ] Remove the dependence of vector.c bitvector.c srfi-4.c etc. on 
array-handle.h
+  - [X] Remove the dependence of VECTOR-TYPE-ref -> array_handle use seen in 
scm_array_get_handle.
+  - [X] Bug: setaffinity in posix.c failed to release the mask handle
+  - [ ] Bug: scm_bitvector_elements doesn't select for type = bit
+  - [ ] Bug: bit-set*! already requires second arg to be true bitvector
 
 
 * Other
 
-** vector-move-right!, vector-move-left!, either
-
-  - work only on vector type, use array-copy! and make-shared-array (or future 
slicing facility) for general arrays
-  - rename to array-move... and make type generic
-  - remove entirely and just use array-copy!
-
 ** basic rank-1 ops on VECTOR_TYPEs need (len begin inc) instead of (begin end)
 
   - look at CBLAS/BLIS
diff --git a/libguile/bitvectors.c b/libguile/bitvectors.c
index 2b41f6d..fa549bb 100644
--- a/libguile/bitvectors.c
+++ b/libguile/bitvectors.c
@@ -689,7 +689,6 @@ SCM_DEFINE (scm_bit_count_star, "bit-count*", 3, 0, 0,
               count++;
        }
     }
-  /* FIXME This requires a true u32vector so handle, inc, etc. are superfluous 
*/
   else if (scm_is_true (scm_u32vector_p (kv)))
     {
       size_t kv_len;
diff --git a/libguile/vectors.c b/libguile/vectors.c
index 90c8ab1..09f270b 100644
--- a/libguile/vectors.c
+++ b/libguile/vectors.c
@@ -345,51 +345,20 @@ 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;
-  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));
+  size_t len1, len2;
+  const SCM *elts1 = scm_vector_elements (vec1, &len1);
+  SCM *elts2 = scm_vector_writable_elements (vec2, &len2);
+
+  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);
-        }
-    }
+  for (; i < e; ++i, ++j)
+    elts2[j] = elts1[i];
+
   return SCM_UNSPECIFIED;
 }
 #undef FUNC_NAME
@@ -405,57 +374,26 @@ 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;
-  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));
+  size_t len1, len2;
+  const SCM *elts1 = scm_vector_elements (vec1, &len1);
+  SCM *elts2 = scm_vector_writable_elements (vec2, &len2);
+
+  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];
-            }
-
-          scm_array_handle_release (&handle2);
-          scm_array_handle_release (&handle1);
-        }
+  while (i < e)
+    {
+      --e;
+      --j;
+      elts2[j] = elts1[e];
     }
+
   return SCM_UNSPECIFIED;
 }
 #undef FUNC_NAME



reply via email to

[Prev in Thread] Current Thread [Next in Thread]