Subject: bug#15912: Improve API of recently-added bool vector functions.

From: Paul Eggert <[email-masked]>
Date: Sun, 17 Nov 2013 12:17:32 -0800

Daniel Colascione wrote:

> I ... dislike count-leading as a name: it suggests that the function
> is useful only at the beginning of a vector. How about count-consecutive?

Sounds good.  Revised patch attached.

> bool-vector-count-matches is supposed to be evocative to CL's
> count-matches, which has a similar interface.

I can't find 'count-matches' in the online documentation for CL, here:

http://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node145.html

It appears that (bool-vector-count-population A) would
correspond to Common Lisp's (count t A), and that
(bool-vector-count-consecutive A B I) would correspond to
Common Lisp's (- (or (position (not B) A :start I) (length A)) I).
If so, Common Lisp's API is so far from the latter primitive
that its names don't seem to provide much guidance.
It might make sense to rename the former primitive's name
to bool-vector-count-t, if you'd prefer that.

(P.S. I don't use CL much, so if I'm missing out on some
commonly-used CL library somewhere, please bear with me.)

=== modified file 'etc/NEWS'
--- etc/NEWS	2013-11-11 05:18:53 +0000
+++ etc/NEWS	2013-11-17 19:36:35 +0000
@@ -728,8 +728,8 @@
 *** `bool-vector-set-difference'
 *** `bool-vector-not'
 *** `bool-vector-subsetp'
-*** `bool-vector-count-matches'
-*** `bool-vector-count-matches-at'
+*** `bool-vector-count-consecutive'
+*** `bool-vector-count-population'
 
 ** Comparison functions =, <, >, <=, >= now take many arguments.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-11-17 03:58:30 +0000
+++ src/ChangeLog	2013-11-17 20:15:12 +0000
@@ -1,5 +1,19 @@
 2013-11-17  Paul Eggert  <[email-masked]>
 
+	Improve API of recently-added bool vector functions (Bug#15912).
+	The old API had (bool-vector-count-matches A B)
+	and (bool-vector-count-matches-at A B I), which gave the
+	misleading impression that the two functions were variants, one
+	with a location I.  The new API has (bool-vector-count-population A)
+	and (bool-vector-count-consecutive A B I) to make the distinction
+	clearer.  The first function no longer has a B argument, since the
+	caller can easily determine the number of nils if the length and
+	number of ts is known.
+	* data.c (Fbool_vector_count_population): Rename from
+	bool_vector_count_matches, and accept just 1 argument.
+	(Fbool_vector_count_consecutive): Rename from
+	Fbool_vector_count_matches_at.
+
 	* lisp.h (DEBUGGER_SEES_C_MACROS): Remove.
 
 2013-11-16  Eli Zaretskii  <[email-masked]>

=== modified file 'src/data.c'
--- src/data.c	2013-11-15 18:01:04 +0000
+++ src/data.c	2013-11-17 19:36:35 +0000
@@ -3247,11 +3247,12 @@
   return b;
 }
 
-DEFUN ("bool-vector-count-matches", Fbool_vector_count_matches,
-       Sbool_vector_count_matches, 2, 2, 0,
-       doc: /* Count how many elements in A equal B.
-A must be a bool vector.  B is a generalized bool.  */)
-  (Lisp_Object a, Lisp_Object b)
+DEFUN ("bool-vector-count-population", Fbool_vector_count_population,
+       Sbool_vector_count_population, 1, 1, 0,
+       doc: /* Count how many elements in A are t.
+A is a bool vector.  To count A's nil elements, subtract the return
+value from A's length.  */)
+  (Lisp_Object a)
 {
   EMACS_INT count;
   EMACS_INT nr_bits;
@@ -3268,17 +3269,13 @@
   for (i = 0; i < nwords; i++)
     count += count_one_bits_word (adata[i]);
 
-  if (NILP (b))
-    count = nr_bits - count;
   return make_number (count);
 }
 
-DEFUN ("bool-vector-count-matches-at",
-       Fbool_vector_count_matches_at,
-       Sbool_vector_count_matches_at, 3, 3, 0,
-       doc: /* Count how many consecutive elements in A equal B at i.
-A must be a bool vector.  B is a generalized boolean.  i is an
-index into the vector.  */)
+DEFUN ("bool-vector-count-consecutive", Fbool_vector_count_consecutive,
+       Sbool_vector_count_consecutive, 3, 3, 0,
+       doc: /* Count how many consecutive elements in A equal B starting at I.
+A is a bool vector, B is t or nil, and I is an index into A.  */)
   (Lisp_Object a, Lisp_Object b, Lisp_Object i)
 {
   EMACS_INT count;
@@ -3622,8 +3619,8 @@
   defsubr (&Sbool_vector_set_difference);
   defsubr (&Sbool_vector_not);
   defsubr (&Sbool_vector_subsetp);
-  defsubr (&Sbool_vector_count_matches);
-  defsubr (&Sbool_vector_count_matches_at);
+  defsubr (&Sbool_vector_count_consecutive);
+  defsubr (&Sbool_vector_count_population);
 
   set_symbol_function (Qwholenump, XSYMBOL (Qnatnump)->function);

=== modified file 'test/ChangeLog'
--- test/ChangeLog	2013-11-16 11:46:37 +0000
+++ test/ChangeLog	2013-11-17 20:15:12 +0000
@@ -1,3 +1,8 @@
+2013-11-17  Paul Eggert  <[email-masked]>
+
+	Improve API of recently-added bool vector functions (Bug#15912).
+	* automated/data-tests.el: Adjust to API changes.
+
 2013-11-16  Michael Albinus  <[email-masked]>
 
 	* automated/tramp-tests.el (tramp-test07-file-exists-p)

=== modified file 'test/automated/data-tests.el'
--- test/automated/data-tests.el	2013-09-22 09:31:55 +0000
+++ test/automated/data-tests.el	2013-11-17 19:36:35 +0000
@@ -77,42 +77,40 @@
 ;; Bool vector tests.  Compactly represent bool vectors as hex
 ;; strings.
 
-(ert-deftest bool-vector-count-matches-all-0-nil ()
+(ert-deftest bool-vector-count-population-all-0-nil ()
   (cl-loop for sz in '(0 45 1 64 9 344)
            do (let* ((bv (make-bool-vector sz nil)))
                 (should
+                 (zerop
+                  (bool-vector-count-population bv))))))
+
+(ert-deftest bool-vector-count-population-all-1-t ()
+  (cl-loop for sz in '(0 45 1 64 9 344)
+           do (let* ((bv (make-bool-vector sz t)))
+                (should
                  (eql
-                  (bool-vector-count-matches bv nil)
+                  (bool-vector-count-population bv)
                   sz)))))
 
-(ert-deftest bool-vector-count-matches-all-0-t ()
-  (cl-loop for sz in '(0 45 1 64 9 344)
-           do (let* ((bv (make-bool-vector sz nil)))
-                (should
-                 (eql
-                  (bool-vector-count-matches bv t)
-                  0)))))
-
-(ert-deftest bool-vector-count-matches-1-nil ()
-  (let* ((bv (make-bool-vector 45 nil)))
-    (aset bv 40 t)
-    (aset bv 0 t)
-    (should
-     (eql
-      (bool-vector-count-matches bv t)
-      2)))
-  )
-
-(ert-deftest bool-vector-count-matches-1-t ()
-  (let* ((bv (make-bool-vector 45 nil)))
-    (aset bv 40 t)
-    (aset bv 0 t)
-    (should
-     (eql
-      (bool-vector-count-matches bv nil)
+(ert-deftest bool-vector-count-population-1-nil ()
+  (let* ((bv (make-bool-vector 45 nil)))
+    (aset bv 40 t)
+    (aset bv 0 t)
+    (should
+     (eql
+      (bool-vector-count-population bv)
+      2))))
+
+(ert-deftest bool-vector-count-population-1-t ()
+  (let* ((bv (make-bool-vector 45 t)))
+    (aset bv 40 nil)
+    (aset bv 0 nil)
+    (should
+     (eql
+      (bool-vector-count-population bv)
       43))))
 
-(defun mock-bool-vector-count-matches-at (a b i)
+(defun mock-bool-vector-count-consecutive (a b i)
   (loop for i from i below (length a)
         while (eq (aref a i) b)
         sum 1))
@@ -147,8 +145,8 @@
                (nreverse nibbles)
                "")))
 
-(defun test-bool-vector-count-matches-at-tc (desc)
-  "Run a test case for bool-vector-count-matches-at.
+(defun test-bool-vector-count-consecutive-tc (desc)
+  "Run a test case for bool-vector-count-consecutive.
 DESC is a string describing the test.  It is a sequence of
 hexadecimal digits describing the bool vector.  We exhaustively
 test all counts at all possible positions in the vector by
@@ -158,8 +156,8 @@
      for lf in '(nil t)
      do (loop
          for pos from 0 upto (length bv)
-         for cnt = (mock-bool-vector-count-matches-at bv lf pos)
-         for rcnt = (bool-vector-count-matches-at bv lf pos)
+         for cnt = (mock-bool-vector-count-consecutive bv lf pos)
+         for rcnt = (bool-vector-count-consecutive bv lf pos)
          unless (eql cnt rcnt)
          do (error "FAILED testcase %S %3S %3S %3S"
                    pos lf cnt rcnt)))))
@@ -182,8 +180,8 @@
   "0000000000000000000000000"
   "FFFFFFFFFFFFFFFF1"))
 
-(ert-deftest bool-vector-count-matches-at ()
-  (mapc #'test-bool-vector-count-matches-at-tc
+(ert-deftest bool-vector-count-consecutive ()
+  (mapc #'test-bool-vector-count-consecutive-tc
         bool-vector-test-vectors))
 
 (defun test-bool-vector-apply-mock-op (mock a b c)