LUCENE-5272: OpenBitSet.ensureCapacity does not modify numBits

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1532093 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -141,6 +141,9 @@ Bug Fixes
 * LUCENE-5269: Fix bug in NGramTokenFilter where it would sometimes count
   unicode characters incorrectly. (Mike McCandless, Robert Muir)
 
+* LUCENE-5272: OpenBitSet.ensureCapacity did not modify numBits, causing 
+  false assertion errors in fastSet. (Shai Erera)
+
 API Changes:
 
 * LUCENE-5222: Add SortField.needsScores(). Previously it was not possible

lucene/core/src/java/org/apache/lucene/util/OpenBitSet.java

@@ -81,32 +81,35 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
   // Used only for assert:
   private long numBits;
 
-  /** Constructs an OpenBitSet large enough to hold <code>numBits</code>.
-   */
+  /** Constructs an OpenBitSet large enough to hold {@code numBits}. */
   public OpenBitSet(long numBits) {
     this.numBits = numBits;
     bits = new long[bits2words(numBits)];
     wlen = bits.length;
   }
 
+  /** Constructor: allocates enough space for 64 bits. */
   public OpenBitSet() {
     this(64);
   }
 
-  /** Constructs an OpenBitSet from an existing long[].
-   * <br/>
-   * The first 64 bits are in long[0],
-   * with bit index 0 at the least significant bit, and bit index 63 at the most significant.
-   * Given a bit index,
-   * the word containing it is long[index/64], and it is at bit number index%64 within that word.
+  /**
+   * Constructs an OpenBitSet from an existing long[].
    * <p>
-   * numWords are the number of elements in the array that contain
-   * set bits (non-zero longs).
-   * numWords should be &lt= bits.length, and
-   * any existing words in the array at position &gt= numWords should be zero.
-   *
+   * The first 64 bits are in long[0], with bit index 0 at the least significant
+   * bit, and bit index 63 at the most significant. Given a bit index, the word
+   * containing it is long[index/64], and it is at bit number index%64 within
+   * that word.
+   * <p>
+   * numWords are the number of elements in the array that contain set bits
+   * (non-zero longs). numWords should be &lt= bits.length, and any existing
+   * words in the array at position &gt= numWords should be zero.
+   * 
    */
   public OpenBitSet(long[] bits, int numWords) {
+    if (numWords > bits.length) {
+      throw new IllegalArgumentException("numWords cannot exceed bits.length");
+    }
     this.bits = bits;
     this.wlen = numWords;
     this.numBits = wlen * 64;
@@ -150,17 +153,9 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
   /** Expert: returns the long[] storing the bits */
   public long[] getBits() { return bits; }
 
-  /** Expert: sets a new long[] to use as the bit storage */
-  public void setBits(long[] bits) { this.bits = bits; }
-
   /** Expert: gets the number of longs in the array that are in use */
   public int getNumWords() { return wlen; }
 
-  /** Expert: sets the number of longs in the array that are in use */
-  public void setNumWords(int nWords) { this.wlen=nWords; }
-
-
-
   /** Returns true or false for the specified bit index. */
   @Override
   public boolean get(int index) {
@@ -287,7 +282,7 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
 
     // since endIndex is one past the end, this is index of the last
     // word to be changed.
-    int endWord   = expandingWordNum(endIndex-1);
+    int endWord = expandingWordNum(endIndex-1);
 
     long startmask = -1L << startIndex;
     long endmask = -1L >>> -endIndex;  // 64-(endIndex&0x3f) is the same as -endIndex due to wrap
@@ -302,19 +297,14 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
     bits[endWord] |= endmask;
   }
 
-
-
   protected int expandingWordNum(long index) {
     int wordNum = (int)(index >> 6);
-    if (wordNum>=wlen) {
-      ensureCapacity(index+1);
-      wlen = wordNum+1;
+    if (wordNum >= wlen) {
+      ensureCapacity(index + 1);
     }
-    assert (numBits = Math.max(numBits, index+1)) >= 0;
     return wordNum;
   }
 
-
   /** clears a bit.
    * The index should be less than the OpenBitSet size.
    */
@@ -519,7 +509,7 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
 
     // since endIndex is one past the end, this is index of the last
     // word to be changed.
-    int endWord   = expandingWordNum(endIndex-1);
+    int endWord = expandingWordNum(endIndex-1);
 
     /*** Grrr, java shifting wraps around so -1L>>>64 == -1
      * for that reason, make sure not to use endmask if the bits to flip will
@@ -833,22 +823,22 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
     return false;
   }
 
-
-
-  /** Expand the long[] with the size given as a number of words (64 bit longs).
-   * getNumWords() is unchanged by this call.
-   */
+  /** Expand the long[] with the size given as a number of words (64 bit longs). */
   public void ensureCapacityWords(int numWords) {
-    if (bits.length < numWords) {
-      bits = ArrayUtil.grow(bits, numWords);
-    }
+    bits = ArrayUtil.grow(bits, numWords);
+    wlen = numWords;
+    assert (this.numBits = Math.max(this.numBits, numWords << 6)) >= 0;
   }
 
-  /** Ensure that the long[] is big enough to hold numBits, expanding it if necessary.
-   * getNumWords() is unchanged by this call.
+  /**
+   * Ensure that the long[] is big enough to hold numBits, expanding it if
+   * necessary.
    */
   public void ensureCapacity(long numBits) {
     ensureCapacityWords(bits2words(numBits));
+    // ensureCapacityWords sets numBits to a multiple of 64, but we want to set
+    // it to exactly what the app asked.
+    assert (this.numBits = Math.max(this.numBits, numBits)) >= 0;
   }
 
   /** Lowers numWords, the number of words in use,
@@ -862,10 +852,9 @@ public class OpenBitSet extends DocIdSet implements Bits, Cloneable {
 
   /** returns the number of 64 bit words it would take to hold numBits */
   public static int bits2words(long numBits) {
-   return (int)(((numBits-1)>>>6)+1);
+    return (int)(((numBits-1)>>>6)+1);
   }
 
-
   /** returns true if both sets have the same bits set */
   @Override
   public boolean equals(Object o) {

lucene/core/src/test/org/apache/lucene/util/TestOpenBitSet.java

@@ -331,6 +331,38 @@ public class TestOpenBitSet extends BaseDocIdSetTestCase<OpenBitSet> {
     checkPrevSetBitArray(new int[] {0,2});
   }
 
+  public void testEnsureCapacity() {
+    OpenBitSet bits = new OpenBitSet(1);
+    int bit = random().nextInt(100) + 10;
+    bits.ensureCapacity(bit); // make room for more bits
+    bits.fastSet(bit);
+    assertTrue(bits.fastGet(bit));
+    bits.ensureCapacity(bit + 1);
+    bits.fastSet(bit + 1);
+    assertTrue(bits.fastGet(bit + 1));
+    bits.ensureCapacity(3); // should not change numBits nor grow the array
+    bits.fastSet(3);
+    assertTrue(bits.fastGet(3));
+    bits.fastSet(bit-1);
+    assertTrue(bits.fastGet(bit-1));
+
+    // test ensureCapacityWords
+    int numWords = random().nextInt(10) + 2; // make sure we grow the array (at least 128 bits)
+    bits.ensureCapacityWords(numWords);
+    bit = _TestUtil.nextInt(random(), 128, numWords << 6); // pick a higher bit than 128, but still within range
+    bits.fastSet(bit);
+    assertTrue(bits.fastGet(bit));
+    bits.fastClear(bit);
+    assertFalse(bits.fastGet(bit));
+    bits.fastFlip(bit);
+    assertTrue(bits.fastGet(bit));
+    bits.ensureCapacityWords(2); // should not change numBits nor grow the array
+    bits.fastSet(3);
+    assertTrue(bits.fastGet(3));
+    bits.fastSet(bit-1);
+    assertTrue(bits.fastGet(bit-1));
+  }
+  
 }