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

Previous Next

Full log

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,

Previous Next

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