r64 iterator: allow moving from beyond the start / end (#578)

Previously calling `roaring64_iterator_previous` after
`roaring64_iterator_advance` returned false would not move the iterator. With
this commit, this behavior and its inverse are both possible.

I chose to implement this by adding a new field in the iterator to indicate
saturation. Another option would be to allow the ART iterator to allow moving
from beyond the start / end, but that would require changing the index to a
signed integer and thus take up more space.

src/roaring64.c

@@ -48,6 +48,12 @@ typedef struct roaring64_iterator_s {
 
     uint64_t value;
     bool has_value;
+
+    // If has_value is false, then the iterator is saturated. This field
+    // indicates the direction of saturation. If true, there are no more values
+    // in the forward direction. If false, there are no more values in the
+    // backward direction.
+    bool saturated_forward;
 } roaring64_iterator_t;
 
 // Splits the given uint64 key into high 48 bit and low 16 bit components.
@@ -126,6 +132,8 @@ static inline roaring64_iterator_t *roaring64_iterator_init_at(
         } else {
             roaring64_iterator_init_at_leaf_last(it);
         }
+    } else {
+        it->saturated_forward = first;
     }
     return it;
 }
@@ -1955,7 +1963,11 @@ uint64_t roaring64_iterator_value(const roaring64_iterator_t *it) {
 
 bool roaring64_iterator_advance(roaring64_iterator_t *it) {
     if (it->art_it.value == NULL) {
-        return (it->has_value = false);
+        if (it->saturated_forward) {
+            return (it->has_value = false);
+        }
+        roaring64_iterator_init_at(it->parent, it, /*first=*/true);
+        return it->has_value;
     }
     leaf_t *leaf = (leaf_t *)it->art_it.value;
     uint16_t low16 = (uint16_t)it->value;
@@ -1964,15 +1976,21 @@ bool roaring64_iterator_advance(roaring64_iterator_t *it) {
         it->value = it->high48 | low16;
         return (it->has_value = true);
     }
-    if (!art_iterator_next(&it->art_it)) {
-        return (it->has_value = false);
+    if (art_iterator_next(&it->art_it)) {
+        return roaring64_iterator_init_at_leaf_first(it);
     }
-    return roaring64_iterator_init_at_leaf_first(it);
+    it->saturated_forward = true;
+    return (it->has_value = false);
 }
 
 bool roaring64_iterator_previous(roaring64_iterator_t *it) {
     if (it->art_it.value == NULL) {
-        return (it->has_value = false);
+        if (!it->saturated_forward) {
+            // Saturated backward.
+            return (it->has_value = false);
+        }
+        roaring64_iterator_init_at(it->parent, it, /*first=*/false);
+        return it->has_value;
     }
     leaf_t *leaf = (leaf_t *)it->art_it.value;
     uint16_t low16 = (uint16_t)it->value;
@@ -1981,10 +1999,11 @@ bool roaring64_iterator_previous(roaring64_iterator_t *it) {
         it->value = it->high48 | low16;
         return (it->has_value = true);
     }
-    if (!art_iterator_prev(&it->art_it)) {
-        return (it->has_value = false);
+    if (art_iterator_prev(&it->art_it)) {
+        return roaring64_iterator_init_at_leaf_last(it);
     }
-    return roaring64_iterator_init_at_leaf_last(it);
+    it->saturated_forward = false;  // Saturated backward.
+    return (it->has_value = false);
 }
 
 bool roaring64_iterator_move_equalorlarger(roaring64_iterator_t *it,
@@ -2000,6 +2019,7 @@ bool roaring64_iterator_move_equalorlarger(roaring64_iterator_t *it,
         // move to a leaf with a key equal or greater.
         if (!art_iterator_lower_bound(&it->art_it, val_high48)) {
             // Only smaller keys found.
+            it->saturated_forward = true;
             return (it->has_value = false);
         }
         it->high48 = combine_key(it->art_it.key, 0);
@@ -2019,6 +2039,7 @@ bool roaring64_iterator_move_equalorlarger(roaring64_iterator_t *it,
         }
         // Only smaller entries in this container, move to the next.
         if (!art_iterator_next(&it->art_it)) {
+            it->saturated_forward = true;
             return (it->has_value = false);
         }
     }

tests/roaring64_unit.cpp

@@ -1388,6 +1388,15 @@ DEFINE_TEST(test_iterator_advance) {
         i++;
     } while (roaring64_iterator_advance(it));
     assert_int_equal(i, values.size());
+
+    // Check that we can move backward from after the last entry.
+    assert_true(roaring64_iterator_previous(it));
+    i--;
+    assert_int_equal(roaring64_iterator_value(it), values[i]);
+
+    // Check that we can't move forward again.
+    assert_false(roaring64_iterator_advance(it));
+
     roaring64_iterator_free(it);
     roaring64_bitmap_free(r);
 }
@@ -1402,13 +1411,22 @@ DEFINE_TEST(test_iterator_previous) {
         values.push_back(v);
         roaring64_bitmap_add_bulk(r, &context, v);
     }
-    size_t i = values.size();
+    int i = ((int)values.size()) - 1;
     roaring64_iterator_t* it = roaring64_iterator_create_last(r);
     do {
-        i--;
         assert_int_equal(roaring64_iterator_value(it), values[i]);
+        i--;
     } while (roaring64_iterator_previous(it));
-    assert_int_equal(i, 0);
+    assert_int_equal(i, -1);
+
+    // Check that we can move forward from before the first entry.
+    assert_true(roaring64_iterator_advance(it));
+    i++;
+    assert_int_equal(roaring64_iterator_value(it), values[i]);
+
+    // Check that we can't move backward again.
+    assert_false(roaring64_iterator_previous(it));
+
     roaring64_iterator_free(it);
     roaring64_bitmap_free(r);
 }
@@ -1448,6 +1466,10 @@ DEFINE_TEST(test_iterator_move_equalorlarger) {
     assert_false(roaring64_iterator_move_equalorlarger(it, (1ULL << 36) + 1));
     assert_false(roaring64_iterator_has_value(it));
 
+    // Check that we can move backward from after the last entry.
+    assert_true(roaring64_iterator_previous(it));
+    assert_int_equal(roaring64_iterator_value(it), (1ULL << 36));
+
     roaring64_iterator_free(it);
     roaring64_bitmap_free(r);
 }