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

Bindings CRoaring 3.0, including 64 bit bitmaps #125

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

Dr-Emann
Copy link
Member

@Dr-Emann Dr-Emann commented Jan 10, 2024

@Dr-Emann
Copy link
Member Author

Man I love differential fuzzing. Finding some issues in roaring64, and our treemap: #126, RoaringBitmap/CRoaring#552, RoaringBitmap/CRoaring#550, RoaringBitmap/CRoaring#551

@Dr-Emann Dr-Emann force-pushed the roaring64 branch 2 times, most recently from c9867b4 to dea654b Compare January 20, 2024 09:06
@Dr-Emann Dr-Emann force-pushed the roaring64 branch 10 times, most recently from 1a05364 to d7cf56c Compare January 30, 2024 04:50
@Dr-Emann Dr-Emann force-pushed the roaring64 branch 2 times, most recently from e2dd6a8 to b112e7e Compare February 13, 2024 05:14
@Dr-Emann Dr-Emann force-pushed the roaring64 branch 3 times, most recently from 97b75be to 8779205 Compare March 22, 2024 04:49
@Dr-Emann Dr-Emann marked this pull request as ready for review March 22, 2024 04:56
@Dr-Emann Dr-Emann changed the title Bindings to roaring64 bitmaps Bindings CRoaring 3.0, including 64 bit bitmaps Mar 22, 2024
@Dr-Emann
Copy link
Member Author

@saulius Believe this is ready 🎉

@saulius
Copy link
Member

saulius commented Mar 29, 2024

Woha, this is fascinating @Dr-Emann, great work!

/// assert_eq!(first_over_50, ControlFlow::Break(100));
/// ```
#[inline]
pub fn for_each<F, O>(&self, f: F) -> ControlFlow<O>
Copy link
Member

Choose a reason for hiding this comment

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

Nice use of ControlFlow, very elegant!

@saulius
Copy link
Member

saulius commented Mar 29, 2024

Looks great! Given this introduces changes to Bitmap, in particular to BitmapIterator, I think it would be safe to release this as 2.x. Do you agree?

the fuzz_ops64 target includes deserializing, and performing actions on the
bitmap too
croaring-sys definitely requires a major version bump, but croaring-rs
shouldn't have any breaking changes, and cargo-semver-checks agrees
@Dr-Emann
Copy link
Member Author

Dr-Emann commented Apr 7, 2024

I believe we don't actually need a major version bump, I don't think there's any breaking changes (and cargo-semver-checks agrees).

That said, I'm not against a 2.0, especially since croaring just did a major version bump.

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.

None yet

2 participants