Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change map when copying Roaring64MapSetBitForwardIterator #590

Merged
merged 2 commits into from
Feb 11, 2024
Merged

Change map when copying Roaring64MapSetBitForwardIterator #590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6729016
Change map when copying Roaring64MapSetBitForwardIterator
tudor Feb 9, 2024
de9e63e
Review comments
tudor Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 72 additions & 104 deletions cpp/roaring64map.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@ namespace roaring {

using roaring::Roaring;

class Roaring64MapSetBitForwardIterator;
class Roaring64MapSetBitBiDirectionalIterator;

// For backwards compatibility; there used to be two kinds of iterators
// (forward and bidirectional) and now there's only one.
typedef Roaring64MapSetBitBiDirectionalIterator
SLieve marked this conversation as resolved.
Show resolved Hide resolved
Roaring64MapSetBitForwardIterator;

class Roaring64Map {
typedef api::roaring_bitmap_t roaring_bitmap_t;

Expand Down Expand Up @@ -1553,9 +1557,8 @@ class Roaring64Map {
return result;
}

friend class Roaring64MapSetBitForwardIterator;
friend class Roaring64MapSetBitBiDirectionalIterator;
typedef Roaring64MapSetBitForwardIterator const_iterator;
typedef Roaring64MapSetBitBiDirectionalIterator const_iterator;
typedef Roaring64MapSetBitBiDirectionalIterator
const_bidirectional_iterator;

Expand Down Expand Up @@ -1710,14 +1713,30 @@ class Roaring64Map {
/**
* Used to go through the set bits. Not optimally fast, but convenient.
*/
class Roaring64MapSetBitForwardIterator {
class Roaring64MapSetBitBiDirectionalIterator {
public:
typedef std::forward_iterator_tag iterator_category;
typedef std::bidirectional_iterator_tag iterator_category;
typedef uint64_t *pointer;
typedef uint64_t &reference;
typedef uint64_t value_type;
typedef int64_t difference_type;
typedef Roaring64MapSetBitForwardIterator type_of_iterator;
typedef Roaring64MapSetBitBiDirectionalIterator type_of_iterator;

Roaring64MapSetBitBiDirectionalIterator(const Roaring64Map &parent,
bool exhausted = false)
: p(&parent.roarings) {
if (exhausted || parent.roarings.empty()) {
map_iter = p->cend();
} else {
map_iter = parent.roarings.cbegin();
roaring_iterator_init(&map_iter->second.roaring, &i);
while (!i.has_value) {
map_iter++;
if (map_iter == p->cend()) return;
roaring_iterator_init(&map_iter->second.roaring, &i);
}
}
}

/**
* Provides the location of the set bit.
Expand All @@ -1727,179 +1746,128 @@ class Roaring64MapSetBitForwardIterator {
}

bool operator<(const type_of_iterator &o) const {
if (map_iter == map_end) return false;
if (o.map_iter == o.map_end) return true;
if (map_iter == p->cend()) return false;
if (o.map_iter == o.p->cend()) return true;
return **this < *o;
}

bool operator<=(const type_of_iterator &o) const {
if (o.map_iter == o.map_end) return true;
if (map_iter == map_end) return false;
if (o.map_iter == o.p->cend()) return true;
if (map_iter == p->cend()) return false;
return **this <= *o;
}

bool operator>(const type_of_iterator &o) const {
if (o.map_iter == o.map_end) return false;
if (map_iter == map_end) return true;
if (o.map_iter == o.p->cend()) return false;
if (map_iter == p->cend()) return true;
return **this > *o;
}

bool operator>=(const type_of_iterator &o) const {
if (map_iter == map_end) return true;
if (o.map_iter == o.map_end) return false;
if (map_iter == p->cend()) return true;
if (o.map_iter == o.p->cend()) return false;
return **this >= *o;
}

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++;
if (map_iter == map_end) return *this;
++map_iter;
if (map_iter == p->cend()) return *this;
roaring_iterator_init(&map_iter->second.roaring, &i);
}
return *this;
}

type_of_iterator operator++(int) { // i++, must return orig. value
Roaring64MapSetBitForwardIterator orig(*this);
Roaring64MapSetBitBiDirectionalIterator orig(*this);
roaring_uint32_iterator_advance(&i);
while (!i.has_value) {
map_iter++;
if (map_iter == map_end) return orig;
++map_iter;
if (map_iter == p->cend()) 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 != p->cend()) {
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++;
if (map_iter == map_end) return false;
++map_iter;
if (map_iter == p->cend()) return false;
roaring_iterator_init(&map_iter->second.roaring, &i);
}
return true;
}
return false;
}

bool operator==(const Roaring64MapSetBitForwardIterator &o) const {
if (map_iter == map_end && o.map_iter == o.map_end) return true;
if (o.map_iter == o.map_end) return false;
return **this == *o;
}

bool operator!=(const Roaring64MapSetBitForwardIterator &o) const {
if (map_iter == map_end && o.map_iter == o.map_end) return false;
if (o.map_iter == o.map_end) return true;
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()) {
if (exhausted || parent.roarings.empty()) {
map_iter = parent.roarings.cend();
} else {
map_iter = parent.roarings.cbegin();
roaring_iterator_init(&map_iter->second.roaring, &i);
while (!i.has_value) {
map_iter++;
if (map_iter == map_end) return;
roaring_iterator_init(&map_iter->second.roaring, &i);
}
}
}

protected:
const std::map<uint32_t, Roaring> &p;
std::map<uint32_t, Roaring>::const_iterator
map_iter{}; // The empty constructor silences warnings from pedantic
// static analyzers.
std::map<uint32_t, Roaring>::const_iterator
map_end{}; // The empty constructor silences warnings from pedantic
// static analyzers.
api::roaring_uint32_iterator_t
i{}; // The empty constructor silences warnings from pedantic static
// analyzers.
};

class Roaring64MapSetBitBiDirectionalIterator final
: public Roaring64MapSetBitForwardIterator {
public:
explicit Roaring64MapSetBitBiDirectionalIterator(const Roaring64Map &parent,
bool exhausted = false)
: Roaring64MapSetBitForwardIterator(parent, exhausted),
map_begin(parent.roarings.cbegin()) {}

Roaring64MapSetBitBiDirectionalIterator &operator=(
const Roaring64MapSetBitForwardIterator &r) {
*(Roaring64MapSetBitForwardIterator *)this = r;
return *this;
}

Roaring64MapSetBitBiDirectionalIterator &
operator--() { // --i, must return dec.value
if (map_iter == map_end) {
type_of_iterator &operator--() { // --i, must return dec.value
if (map_iter == p->cend()) {
--map_iter;
roaring_iterator_init_last(&map_iter->second.roaring, &i);
if (i.has_value) return *this;
}

roaring_uint32_iterator_previous(&i);
while (!i.has_value) {
if (map_iter == map_begin) return *this;
if (map_iter == p->cbegin()) return *this;
map_iter--;
roaring_iterator_init_last(&map_iter->second.roaring, &i);
}
return *this;
}

Roaring64MapSetBitBiDirectionalIterator operator--(
int) { // i--, must return orig. value
type_of_iterator operator--(int) { // i--, must return orig. value
Roaring64MapSetBitBiDirectionalIterator orig(*this);
if (map_iter == map_end) {
if (map_iter == p->cend()) {
--map_iter;
roaring_iterator_init_last(&map_iter->second.roaring, &i);
return orig;
}

roaring_uint32_iterator_previous(&i);
while (!i.has_value) {
if (map_iter == map_begin) return orig;
if (map_iter == p->cbegin()) return orig;
map_iter--;
roaring_iterator_init_last(&map_iter->second.roaring, &i);
}
return orig;
}

protected:
std::map<uint32_t, Roaring>::const_iterator map_begin;
bool operator==(const Roaring64MapSetBitBiDirectionalIterator &o) const {
if (map_iter == p->cend() && o.map_iter == o.p->cend()) return true;
if (o.map_iter == o.p->cend()) return false;
return **this == *o;
}

bool operator!=(const Roaring64MapSetBitBiDirectionalIterator &o) const {
if (map_iter == p->cend() && o.map_iter == o.p->cend()) return false;
if (o.map_iter == o.p->cend()) return true;
return **this != *o;
}

private:
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.
api::roaring_uint32_iterator_t
i{}; // The empty constructor silences warnings from pedantic static
// analyzers.
};

inline Roaring64MapSetBitForwardIterator Roaring64Map::begin() const {
return Roaring64MapSetBitForwardIterator(*this);
inline Roaring64MapSetBitBiDirectionalIterator Roaring64Map::begin() const {
return Roaring64MapSetBitBiDirectionalIterator(*this);
}

inline Roaring64MapSetBitForwardIterator Roaring64Map::end() const {
return Roaring64MapSetBitForwardIterator(*this, true);
inline Roaring64MapSetBitBiDirectionalIterator Roaring64Map::end() const {
return Roaring64MapSetBitBiDirectionalIterator(*this, true);
}

} // namespace roaring
Expand Down
15 changes: 15 additions & 0 deletions tests/cpp_unit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2146,6 +2146,20 @@ DEFINE_TEST(test_cpp_contains_range_interleaved_containers) {
roaring.containsRange(0x1FFFF, 0x2FFFF + 2);
}

// Test that it is pointed to the new map, see
// https://github.com/RoaringBitmap/CRoaring/issues/589
DEFINE_TEST(test_cpp_copy_map_iterator_to_different_map) {
SLieve marked this conversation as resolved.
Show resolved Hide resolved
Roaring64Map m1{1};
Roaring64Map m2{10, 20, 30, 40};
auto it = m1.begin();
it = m2.begin();
it.move(21);
int n = 0;
for (; it != m2.end(); ++it, ++n) {
}
assert_int_equal(2, n);
}

int main() {
roaring::misc::tellmeall();
const struct CMUnitTest tests[] = {
Expand Down Expand Up @@ -2222,6 +2236,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);
}
Loading