Skip to content

Add support for roaring_bitmap_contains_range #31

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

Closed
lemire opened this issue Jun 15, 2018 · 5 comments
Closed

Add support for roaring_bitmap_contains_range #31

lemire opened this issue Jun 15, 2018 · 5 comments

Comments

@lemire
Copy link
Member

lemire commented Jun 15, 2018

See PR RoaringBitmap/CRoaring#145

@saulius
Copy link
Member

saulius commented Jun 18, 2018

Hello @lemire,

I'm only able to make roaring_bitmap_contains_range work with the following changes to roaring.h file (using v0.2.47):

diff --git a/roaring.h b/roaring.h
index 5c4c0d6..d01a4b7 100644
--- a/roaring.h
+++ b/roaring.h
@@ -1468,7 +1468,7 @@ static inline bool array_container_remove(array_container_t *arr,
 }
 
 /* Check whether x is present.  */
-inline bool array_container_contains(const array_container_t *arr,
+bool array_container_contains(const array_container_t *arr,
                                      uint16_t pos) {
     //    return binarySearch(arr->array, arr->cardinality, pos) >= 0;
     // binary search with fallback to linear search for short ranges
@@ -1500,7 +1500,7 @@ inline bool array_container_contains(const array_container_t *arr,
 }
 
 //* Check whether a range of values from range_start (included) to range_end (excluded) is present. */
-static inline bool array_container_contains_range(const array_container_t *arr,
+bool array_container_contains_range(const array_container_t *arr,
                                                     uint32_t range_start, uint32_t range_end) {
 
     const uint16_t rs_included = range_start;
@@ -1806,7 +1806,7 @@ inline bool bitset_container_contains(const bitset_container_t *bitset,
 * Check whether a range of bits from position `pos_start' (included) to `pos_end' (excluded)
 * is present in `bitset'.  Calls bitset_container_get_all.
 */
-static inline bool bitset_container_contains_range(const bitset_container_t *bitset,
+inline bool bitset_container_contains_range(const bitset_container_t *bitset,
 					uint32_t pos_start, uint32_t pos_end) {
     return bitset_container_get_range(bitset, pos_start, pos_end);
 }
@@ -2295,7 +2295,7 @@ inline bool run_container_contains(const run_container_t *run, uint16_t pos) {
 * Check whether all positions in a range of positions from pos_start (included)
 * to pos_end (excluded) is present in `run'.
 */
-static inline bool run_container_contains_range(const run_container_t *run,
+bool run_container_contains_range(const run_container_t *run,
                                                 uint32_t pos_start, uint32_t pos_end) {
     uint32_t count = 0;
     int32_t index = interleavedBinarySearch(run->runs, run->n_runs, pos_start);
@@ -3981,7 +3981,7 @@ inline bool container_contains(const void *container, uint16_t val,
  * Check whether a range of values from range_start (included) to range_end (excluded)
  * is in a container, requires a typecode
  */
-static inline bool container_contains_range(const void *container, uint32_t range_start,
+bool container_contains_range(const void *container, uint32_t range_start,
 					uint32_t range_end, uint8_t typecode) {
     container = container_unwrap_shared(container, &typecode);
     switch (typecode) {
@@ -6482,7 +6482,7 @@ inline bool roaring_bitmap_contains(const roaring_bitmap_t *r, uint32_t val) {
 /**
  * Check whether a range of values from range_start (included) to range_end (excluded) is present
  */
-static inline bool roaring_bitmap_contains_range(const roaring_bitmap_t *r, uint64_t range_start, uint64_t range_end) {
+bool roaring_bitmap_contains_range(const roaring_bitmap_t *r, uint64_t range_start, uint64_t range_end) {
     if(range_end >= UINT64_C(0x100000000)) {
         range_end = UINT64_C(0x100000000);
     }

I'm not a C expert but do these functions necessarily need to be static inline? It might be related to this issue of Rust's bindgen, but I think this is the first set of such functions I found in CRoaring, so do we need to have them declared like that?

Thanks

@lemire
Copy link
Member Author

lemire commented Jun 18, 2018

I'm not a C expert but do these functions necessarily need to be static inline?

These have no business being static inline, you are correct. I consider this a small bug.

Can you update to 0.2.48? They are now conventional C function with dual define/declare (.c/.h) so it should work.

There is one "inline" that remains (for the "contains" function), but it is coupled with an "extern inline". I want client to have the option of inlining "contains". But it is a dual function (inline/extern inline).

The bug report you refer to alludes to "static inline" functions... we no longer have those. If there is a problem with dual inline/extern inline functions, then I would like to know.

@saulius
Copy link
Member

saulius commented Jun 18, 2018

Thanks! It works great now.

@lemire
Copy link
Member Author

lemire commented Jun 18, 2018

I recommend you use CRoaring version 0.2.49 or better.

@saulius
Copy link
Member

saulius commented Jun 18, 2018

I bumped to 0.2.49 now. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants