Change map when copying Roaring64MapSetBitForwardIterator

rockset · Feb 9, 2024 · cd25509 · cd25509
1 parent 04c9af2
commit cd25509
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 20 deletions.
diff --git a/cpp/roaring64map.hh b/cpp/roaring64map.hh
@@ -1753,7 +1753,7 @@ class Roaring64MapSetBitForwardIterator {
     type_of_iterator &operator++() {  // ++i, must returned inc. value
         if (i.has_value == true) roaring_uint32_iterator_advance(&i);
         while (!i.has_value) {
-            map_iter++;
+            ++map_iter;
             if (map_iter == map_end) return *this;
             roaring_iterator_init(&map_iter->second.roaring, &i);
         }
@@ -1764,22 +1764,22 @@ class Roaring64MapSetBitForwardIterator {
         Roaring64MapSetBitForwardIterator orig(*this);
         roaring_uint32_iterator_advance(&i);
         while (!i.has_value) {
-            map_iter++;
+            ++map_iter;
             if (map_iter == map_end) return orig;
             roaring_iterator_init(&map_iter->second.roaring, &i);
         }
         return orig;
     }
 
     bool move(const value_type &x) {
-        map_iter = p.lower_bound(Roaring64Map::highBytes(x));
-        if (map_iter != p.cend()) {
+        map_iter = p->lower_bound(Roaring64Map::highBytes(x));
+        if (map_iter != map_end) {
             roaring_iterator_init(&map_iter->second.roaring, &i);
             if (map_iter->first == Roaring64Map::highBytes(x)) {
                 if (roaring_uint32_iterator_move_equalorlarger(
                         &i, Roaring64Map::lowBytes(x)))
                     return true;
-                map_iter++;
+                ++map_iter;
                 if (map_iter == map_end) return false;
                 roaring_iterator_init(&map_iter->second.roaring, &i);
             }
@@ -1800,23 +1800,11 @@ class Roaring64MapSetBitForwardIterator {
         return **this != *o;
     }
 
-    Roaring64MapSetBitForwardIterator &operator=(
-        const Roaring64MapSetBitForwardIterator &r) {
-        map_iter = r.map_iter;
-        map_end = r.map_end;
-        i = r.i;
-        return *this;
-    }
-
-    Roaring64MapSetBitForwardIterator(
-        const Roaring64MapSetBitForwardIterator &r)
-        : p(r.p), map_iter(r.map_iter), map_end(r.map_end), i(r.i) {}
-
     Roaring64MapSetBitForwardIterator(const Roaring64Map &parent,
                                       bool exhausted = false)
-        : p(parent.roarings), map_end(parent.roarings.cend()) {
+        : p(&parent.roarings), map_end(parent.roarings.cend()) {
         if (exhausted || parent.roarings.empty()) {
-            map_iter = parent.roarings.cend();
+            map_iter = map_end;
         } else {
             map_iter = parent.roarings.cbegin();
             roaring_iterator_init(&map_iter->second.roaring, &i);
@@ -1829,7 +1817,7 @@ class Roaring64MapSetBitForwardIterator {
     }
 
    protected:
-    const std::map<uint32_t, Roaring> &p;
+    const std::map<uint32_t, Roaring> *p{nullptr};
     std::map<uint32_t, Roaring>::const_iterator
         map_iter{};  // The empty constructor silences warnings from pedantic
                      // static analyzers.

diff --git a/tests/cpp_unit.cpp b/tests/cpp_unit.cpp
@@ -2146,6 +2146,16 @@ DEFINE_TEST(test_cpp_contains_range_interleaved_containers) {
     roaring.containsRange(0x1FFFF, 0x2FFFF + 2);
 }
 
+DEFINE_TEST(test_cpp_copy_map_iterator_to_different_map) {
+    Roaring64Map m1{1};
+    Roaring64Map m2{2};
+    auto  it = m1.begin();
+    int n = 0;
+    for (it = m2.begin(); it != m2.end(); ++it, ++n) {}
+    assert_int_equal(1, n);
+}
+
+
 int main() {
     roaring::misc::tellmeall();
     const struct CMUnitTest tests[] = {
@@ -2222,6 +2232,7 @@ int main() {
         cmocka_unit_test(test_cpp_to_string),
         cmocka_unit_test(test_cpp_remove_run_compression),
         cmocka_unit_test(test_cpp_contains_range_interleaved_containers),
+        cmocka_unit_test(test_cpp_copy_map_iterator_to_different_map),
     };
     return cmocka_run_group_tests(tests, NULL, NULL);
 }