GNU bug report logs - #15912
Improve API of recently-added bool vector functions.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15912 in the body.
You can then email your comments to 15912 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-emacs <bug-gnu-emacs <at> gnu.org>
Cc: Daniel Colascione <dancol <at> dancol.org>
Subject: Improve API of recently-added bool vector functions.
Date: Sun, 17 Nov 2013 00:39:06 -0800

[Message part 1 (text/plain, inline)]

Tags: patch

Here's a proposed patch to improve the API of the recently-added
bool vector functions.  I'm CC'ing this to Daniel Colascione
to give him a heads-up, since he added the functions.
I figure we should get this resolved soon, before the functions
are frozen for 24.4.

[boolvec.diff (text/x-patch, attachment)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Daniel Colascione <dancol <at> dancol.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, bug-emacs <bug-gnu-emacs <at> gnu.org>
Subject: Re: Improve API of recently-added bool vector functions.
Date: Sun, 17 Nov 2013 03:07:08 -0800

On 11/17/2013 12:39 AM, Paul Eggert wrote:
> Tags: patch
>
> Here's a proposed patch to improve the API of the recently-added
> bool vector functions.  I'm CC'ing this to Daniel Colascione
> to give him a heads-up, since he added the functions.
> I figure we should get this resolved soon, before the functions
> are frozen for 24.4.
>

The functions _are_ variants. They both count bits, albeit in slightly 
different ways. bool-vector-count-matches is supposed to be evocative to 
CL's count-matches, which has a similar interface. I also dislike 
count-leading as a name: it suggests that the function is useful only at 
the beginning of a vector. How about count-consecutive?

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Daniel Colascione <dancol <at> dancol.org>, bug-emacs <bug-gnu-emacs <at> gnu.org>
Subject: Re: Improve API of recently-added bool vector functions.
Date: Sun, 17 Nov 2013 12:17:32 -0800

[Message part 1 (text/plain, inline)]

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.)

[boolvec.diff (text/x-patch, attachment)]

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Daniel Colascione <dancol <at> dancol.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, bug-emacs <bug-gnu-emacs <at> gnu.org>
Subject: Re: Improve API of recently-added bool vector functions.
Date: Sun, 17 Nov 2013 12:28:09 -0800

Thanks!

On 11/17/2013 12:17 PM, Paul Eggert wrote:
> 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:

I was mistaken. (Note the original message timestamp.) Sorry about 
sending you on a wild goose chase through CLTL --- the revised patch is 
fine.

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Daniel Colascione <dancol <at> dancol.org>, bug-emacs <bug-gnu-emacs <at> gnu.org>
Subject: Re: Improve API of recently-added bool vector functions.
Date: Mon, 18 Nov 2013 11:44:56 -0800

On 11/17/2013 12:28 PM, Daniel Colascione wrote:
> the revised patch is fine

Thanks, installed as trunk bzr 15912.

I noticed another problem.  bool-vector-subset-p is backwards
of what I'd normally expect, i.e., (bool-vector-subsetp A B)
tests whether B is a subset of A, not whether A is a subset of B.
Another thing: it might be more intuitive if this function returns
a boolean.  Also, this function isn't documented.  One possibility
is to remove the function if nobody's using it; another possibility
would be the following patch.

=== modified file 'src/ChangeLog'
--- src/ChangeLog    2013-11-18 19:31:05 +0000
+++ src/ChangeLog    2013-11-18 19:43:41 +0000
@@ -1,5 +1,10 @@
 2013-11-18  Paul Eggert  <eggert <at> cs.ucla.edu>
 
+    bool-vector-subsetp is now the normal direction and returns a boolean.
+    * data.c (bool_vector_binop_driver): When implementing subsetp,
+    test whether the first argument is a subset of the second one,
+    and return t or nil.  Tune.
+
     * data.c (bool_vector_binop_driver): Rename locals for sanity's sake.
     The old names predated the API change that put destination at end.
 

=== modified file 'src/data.c'
--- src/data.c    2013-11-18 19:31:05 +0000
+++ src/data.c    2013-11-18 19:43:41 +0000
@@ -3060,6 +3060,12 @@
       break;
 
     case bool_vector_subsetp:
+      eassert (bdata == destdata);
+      while (! (adata[i] & ~bdata[i]))
+        if (! (++i < nr_words))
+          return Qt;
+      return Qnil;
+
     case bool_vector_union:
       while (destdata[i] == (adata[i] | bdata[i]))
         if (! (++i < nr_words))
@@ -3088,9 +3094,6 @@
       while (++i < nr_words);
       break;
 
-    case bool_vector_subsetp:
-      break;
-
     case bool_vector_union:
       do
     destdata[i] = adata[i] | bdata[i];
@@ -3108,6 +3111,9 @@
     destdata[i] = adata[i] &~ bdata[i];
       while (++i < nr_words);
       break;
+
+    default:
+      eassume (0);
     }
 
   return dest;
@@ -3234,11 +3240,11 @@
 
 DEFUN ("bool-vector-subsetp", Fbool_vector_subsetp,
        Sbool_vector_subsetp, 2, 2, 0,
-       doc: )
+       doc: /* Return t if every t value in A is also t in B, nil otherwise.
+A and B must be bool vectors of the same length.  */)
   (Lisp_Object a, Lisp_Object b)
 {
-  /* Like bool_vector_union, but doesn't modify b.  */
-  return bool_vector_binop_driver (b, a, b, bool_vector_subsetp);
+  return bool_vector_binop_driver (a, b, b, bool_vector_subsetp);
 }
 
 DEFUN ("bool-vector-not", Fbool_vector_not,

Message #22 received at 15912-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 15912-done <at> debbugs.gnu.org
Subject: Re: Improve API of recently-added bool vector functions.
Date: Mon, 25 Nov 2013 15:26:40 -0800

On 11/18/2013 11:44 AM, Paul Eggert wrote:
> One possibility
> is to remove the function if nobody's using it; another possibility
> would be the following patch.

No further comment, so I merged the patch as trunk bzr 115236
and am marking this as done.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.