Skip to content

Add RoaringTreemap::contains_range and range_cardinality#351

Merged
Kerollmops merged 2 commits into
RoaringBitmap:mainfrom
dchristle:dchristle/range-cardinality
Apr 23, 2026
Merged

Add RoaringTreemap::contains_range and range_cardinality#351
Kerollmops merged 2 commits into
RoaringBitmap:mainfrom
dchristle:dchristle/range-cardinality

Conversation

@dchristle
Copy link
Copy Markdown
Contributor

RoaringBitmap has contains_range and range_cardinality; RoaringTreemap does not.

Empty-range conventions match RoaringBitmap: contains_range returns true, range_cardinality returns 0.

New test file treemap_range_checks.rs mirrors range_checks.rs.

RoaringBitmap has had both methods; this closes the same gap on
RoaringTreemap using BTreeMap::range to iterate only the relevant
high-word buckets.
Copy link
Copy Markdown
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR. I see that Clippy may have been auto-updated and is triggering an error. Can you fix it quickly, please?

Overall, the PR looks good. I just have a small improvement proposal. Can you address it?

Comment thread roaring/src/treemap/inherent.rs Outdated
Copy link
Copy Markdown
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

The CI issue doesn't seems related to your code so I'll merge it. Thanks for the help!

@Kerollmops Kerollmops added this pull request to the merge queue Apr 23, 2026
Merged via the queue into RoaringBitmap:main with commit 01919af Apr 23, 2026
14 of 15 checks passed
@dchristle dchristle deleted the dchristle/range-cardinality branch April 23, 2026 17:34
@dchristle
Copy link
Copy Markdown
Contributor Author

Thanks for the review and merge. Yes, the Clippy warning was on existing code this PR didn’t touch, so I think a newer Clippy version likely started flagging it.

FWIW I checked the for loop vs map(...).sum() with a local --release disassembly, and for this case they compiled equivalently on the tested target, so the difference looks stylistic rather than performance-driven. BTreeMap::range(...) already makes the loop iterator-driven, and the reduction is already obvious to the compiler.

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

Successfully merging this pull request may close these issues.

2 participants