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.

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

Improve consistency in the C++ interface #653

Merged
merged 4 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion benchmarks/containsmulti_benchmark.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "benchmark.h"
#include "numbersfromtextfiles.h"
#include "portability.h"
#include "random.h"

void contains_multi_via_contains(roaring_bitmap_t* bm, const uint32_t* values,
Expand All @@ -19,7 +20,7 @@ void contains_multi_via_contains(roaring_bitmap_t* bm, const uint32_t* values,

void contains_multi_bulk(roaring_bitmap_t* bm, const uint32_t* values,
bool* results, const size_t count) {
roaring_bulk_context_t context = {0, 0};
roaring_bulk_context_t context = CROARING_ZERO_INITIALIZER;
for (size_t i = 0; i < count; ++i) {
results[i] = roaring_bitmap_contains_bulk(bm, &context, values[i]);
}
Expand Down
234 changes: 133 additions & 101 deletions cpp/roaring.hh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ A C++ header for Roaring Bitmaps.
#include <algorithm>
#include <cstdarg>
#include <initializer_list>
#include <limits>
#include <new>
#include <stdexcept>
#include <string>
Expand Down Expand Up @@ -39,7 +40,10 @@ A C++ header for Roaring Bitmaps.

namespace roaring {

class RoaringSetBitForwardIterator;
class RoaringSetBitBiDirectionalIterator;

/** DEPRECATED, use `RoaringSetBitBiDirectionalIterator`. */
using RoaringSetBitForwardIterator = RoaringSetBitBiDirectionalIterator;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use CROARING_DEPRECATED?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to, but:

  1. currently, the Roaring64MapSetBitForwardIterator is not marked as deprecated.
  2. CROARING_DEPRECATED expands to __attribute__((deprecated)) for GNUC and Clang, so using xxx CROARING_DEPRECATED = xxx; works. However, for MSVC, it expands to __declspec(deprecated), which won't work in this format.


/**
* A bit of context usable with `*Bulk()` functions.
Expand Down Expand Up @@ -91,6 +95,16 @@ class Roaring {
addMany(l.size(), l.begin());
}

/**
* Construct a roaring object by taking control of a malloc()'d C struct.
cubicYYY marked this conversation as resolved.
Show resolved Hide resolved
*
* Passing a NULL pointer is unsafe.
* The pointer to the C struct will be invalid after the call.
*/
explicit Roaring(roaring_bitmap_t *s) noexcept : roaring(*s) {
roaring_free(s); // deallocate the passed-in pointer
}

/**
* Copy constructor.
* It may throw std::runtime_error if there is insufficient memory.
Expand Down Expand Up @@ -118,16 +132,6 @@ class Roaring {
api::roaring_bitmap_init_cleared(&r.roaring);
}

/**
* Construct a roaring object by taking control of a malloc()'d C struct.
*
* Passing a NULL pointer is unsafe.
* The pointer to the C struct will be invalid after the call.
*/
explicit Roaring(roaring_bitmap_t *s) noexcept : roaring(*s) {
roaring_free(s); // deallocate the passed-in pointer
}

/**
* Construct a bitmap from a list of uint32_t values.
*/
Expand All @@ -142,6 +146,44 @@ class Roaring {
return ans;
}

/**
* Copies the content of the provided bitmap, and
* discard the current content.
* It may throw std::runtime_error if there is insufficient memory.
*/
Roaring &operator=(const Roaring &r) {
if (!api::roaring_bitmap_overwrite(&roaring, &r.roaring)) {
ROARING_TERMINATE("failed memory alloc in assignment");
}
api::roaring_bitmap_set_copy_on_write(
&roaring, api::roaring_bitmap_get_copy_on_write(&r.roaring));
return *this;
}

/**
* Moves the content of the provided bitmap, and
* discard the current content.
*/
Roaring &operator=(Roaring &&r) noexcept {
api::roaring_bitmap_clear(&roaring); // free this class's allocations

// !!! See notes in the Move Constructor regarding roaring_bitmap_move()
//
roaring = r.roaring;
api::roaring_bitmap_init_cleared(&r.roaring);

return *this;
}

/**
* Assignment from an initializer list.
*/
Roaring &operator=(std::initializer_list<uint32_t> l) {
// Delegate to move assignment operator
*this = Roaring(l);
return *this;
}

/**
* Construct a bitmap from a list of uint32_t values.
* E.g., bitmapOfList({1,2,3}).
Expand Down Expand Up @@ -242,6 +284,11 @@ class Roaring {
return api::roaring_bitmap_remove_range_closed(&roaring, min, max);
}

/**
* Clears the bitmap.
*/
void clear() { api::roaring_bitmap_clear(&roaring); }

/**
* Return the largest value (if not empty)
*/
Expand Down Expand Up @@ -270,63 +317,9 @@ class Roaring {
return api::roaring_bitmap_contains_range(&roaring, x, y);
}

/**
* Destructor. By contract, calling roaring_bitmap_clear() is enough to
* release all auxiliary memory used by the structure.
*/
~Roaring() {
if (!(roaring.high_low_container.flags & ROARING_FLAG_FROZEN)) {
api::roaring_bitmap_clear(&roaring);
} else {
// The roaring member variable copies the `roaring_bitmap_t` and
// nested `roaring_array_t` structures by value and is freed in the
// constructor, however the underlying memory arena used for the
// container data is not freed with it. Here we derive the arena
// pointer from the second arena allocation in
// `roaring_bitmap_frozen_view` and free it as well.
roaring_bitmap_free(
(roaring_bitmap_t *)((char *)
roaring.high_low_container.containers -
sizeof(roaring_bitmap_t)));
}
}

/**
* Copies the content of the provided bitmap, and
* discard the current content.
* It may throw std::runtime_error if there is insufficient memory.
*/
Roaring &operator=(const Roaring &r) {
if (!api::roaring_bitmap_overwrite(&roaring, &r.roaring)) {
ROARING_TERMINATE("failed memory alloc in assignment");
}
api::roaring_bitmap_set_copy_on_write(
&roaring, api::roaring_bitmap_get_copy_on_write(&r.roaring));
return *this;
}

/**
* Moves the content of the provided bitmap, and
* discard the current content.
*/
Roaring &operator=(Roaring &&r) noexcept {
api::roaring_bitmap_clear(&roaring); // free this class's allocations

// !!! See notes in the Move Constructor regarding roaring_bitmap_move()
//
roaring = r.roaring;
api::roaring_bitmap_init_cleared(&r.roaring);

return *this;
}

/**
* Assignment from an initializer list.
*/
Roaring &operator=(std::initializer_list<uint32_t> l) {
// Delegate to move assignment operator
*this = Roaring(l);
return *this;
bool containsRangeClosed(const uint32_t x,
const uint32_t y) const noexcept {
return api::roaring_bitmap_contains_range_closed(&roaring, x, y);
}

/**
Expand Down Expand Up @@ -393,6 +386,16 @@ class Roaring {
return api::roaring_bitmap_is_empty(&roaring);
}

/**
* Returns true if the bitmap is full (cardinality is uint32_t max + 1).
* we put std::numeric_limits<>::max/min in parentheses
* to avoid a clash with the Windows.h header under Windows.
*/
bool isFull() const noexcept {
return api::roaring_bitmap_get_cardinality(&roaring) ==
((uint64_t)(std::numeric_limits<uint32_t>::max)()) + 1;
}

/**
* Returns true if the bitmap is subset of the other.
*/
Expand Down Expand Up @@ -443,8 +446,8 @@ class Roaring {
* [range_start, range_end]. Areas outside the interval are unchanged.
*/
void flipClosed(uint32_t range_start, uint32_t range_end) noexcept {
api::roaring_bitmap_flip_inplace(&roaring, range_start,
uint64_t(range_end) + 1);
api::roaring_bitmap_flip_inplace_closed(&roaring, range_start,
range_end);
}

/**
Expand Down Expand Up @@ -868,7 +871,30 @@ class Roaring {
return ans;
}

typedef RoaringSetBitForwardIterator const_iterator;
/**
* Destructor. By contract, calling roaring_bitmap_clear() is enough to
* release all auxiliary memory used by the structure.
*/
~Roaring() {
if (!(roaring.high_low_container.flags & ROARING_FLAG_FROZEN)) {
api::roaring_bitmap_clear(&roaring);
} else {
// The roaring member variable copies the `roaring_bitmap_t` and
// nested `roaring_array_t` structures by value and is freed in the
// constructor, however the underlying memory arena used for the
// container data is not freed with it. Here we derive the arena
// pointer from the second arena allocation in
// `roaring_bitmap_frozen_view` and free it as well.
roaring_bitmap_free(
(roaring_bitmap_t *)((char *)
roaring.high_low_container.containers -
sizeof(roaring_bitmap_t)));
}
}

friend class RoaringSetBitBiDirectionalIterator;
typedef RoaringSetBitBiDirectionalIterator const_iterator;
typedef RoaringSetBitBiDirectionalIterator const_bidirectional_iterator;

/**
* Returns an iterator that can be used to access the position of the set
Expand All @@ -893,14 +919,26 @@ class Roaring {
/**
* Used to go through the set bits. Not optimally fast, but convenient.
*/
class RoaringSetBitForwardIterator final {
class RoaringSetBitBiDirectionalIterator final {
public:
typedef std::forward_iterator_tag iterator_category;
typedef std::bidirectional_iterator_tag iterator_category;
typedef uint32_t *pointer;
typedef uint32_t &reference_type;
typedef uint32_t value_type;
typedef int32_t difference_type;
typedef RoaringSetBitForwardIterator type_of_iterator;
typedef RoaringSetBitBiDirectionalIterator type_of_iterator;

explicit RoaringSetBitBiDirectionalIterator(const Roaring &parent,
bool exhausted = false) {
if (exhausted) {
i.parent = &parent.roaring;
i.container_index = INT32_MAX;
i.has_value = false;
i.current_value = UINT32_MAX;
} else {
api::roaring_iterator_init(&parent.roaring, &i);
}
}

/**
* Provides the location of the set bit.
Expand Down Expand Up @@ -931,66 +969,60 @@ class RoaringSetBitForwardIterator final {
return i.current_value >= *o;
}

/**
* Move the iterator to the first value >= val.
*/
void equalorlarger(uint32_t val) {
api::roaring_uint32_iterator_move_equalorlarger(&i, val);
}

type_of_iterator &operator++() { // ++i, must returned inc. value
api::roaring_uint32_iterator_advance(&i);
return *this;
}

type_of_iterator operator++(int) { // i++, must return orig. value
RoaringSetBitForwardIterator orig(*this);
RoaringSetBitBiDirectionalIterator orig(*this);
api::roaring_uint32_iterator_advance(&i);
return orig;
}

/**
* Move the iterator to the first value >= val.
* Return true if there is such a value.
*/
bool move_equalorlarger(value_type val) {
return api::roaring_uint32_iterator_move_equalorlarger(&i, val);
}

/** DEPRECATED, use `move_equalorlarger`.*/
CROARING_DEPRECATED void equalorlarger(uint32_t val) {
api::roaring_uint32_iterator_move_equalorlarger(&i, val);
}

type_of_iterator &operator--() { // prefix --
api::roaring_uint32_iterator_previous(&i);
return *this;
}

type_of_iterator operator--(int) { // postfix --
RoaringSetBitForwardIterator orig(*this);
RoaringSetBitBiDirectionalIterator orig(*this);
api::roaring_uint32_iterator_previous(&i);
return orig;
}

bool operator==(const RoaringSetBitForwardIterator &o) const {
bool operator==(const RoaringSetBitBiDirectionalIterator &o) const {
return i.current_value == *o && i.has_value == o.i.has_value;
}

bool operator!=(const RoaringSetBitForwardIterator &o) const {
bool operator!=(const RoaringSetBitBiDirectionalIterator &o) const {
return i.current_value != *o || i.has_value != o.i.has_value;
}

explicit RoaringSetBitForwardIterator(const Roaring &parent,
bool exhausted = false) {
if (exhausted) {
i.parent = &parent.roaring;
i.container_index = INT32_MAX;
i.has_value = false;
i.current_value = UINT32_MAX;
} else {
api::roaring_iterator_init(&parent.roaring, &i);
}
}

api::roaring_uint32_iterator_t
i{}; // The empty constructor silences warnings from pedantic static
// analyzers.
};

inline RoaringSetBitForwardIterator Roaring::begin() const {
return RoaringSetBitForwardIterator(*this);
inline RoaringSetBitBiDirectionalIterator Roaring::begin() const {
return RoaringSetBitBiDirectionalIterator(*this);
}

inline RoaringSetBitForwardIterator &Roaring::end() const {
static RoaringSetBitForwardIterator e(*this, true);
inline RoaringSetBitBiDirectionalIterator &Roaring::end() const {
static RoaringSetBitBiDirectionalIterator e(*this, true);
return e;
}

Expand Down
Loading