Github Rework the insert/remove_range functions with RangeBounds by Kerollmops ·...
source link: https://github.com/RoaringBitmap/roaring-rs/pull/92
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Copy link
Urhengulas left a comment
Brief hello from this-week-in-rust 👋🏾
Bound::Excluded(value) => match value.checked_add(1) {
Some(value) => value,
None => return 0,
},
Comment on lines
+64 to +67
Bound::Included(value) => match value.checked_add(1) {
Some(value) => value,
None => return 0,
},
Comment on lines
+72 to +75
tomjw64 yesterday •
Another hello from This Week in Rust!
I find it a bit strange that of these three hypothetical test cases, the first two pass and the third does not:
#[test]
fn test_insert_max_u32() {
let mut b = RoaringBitmap::new();
let inserted = b.insert(u32::MAX);
// We are allowed to add u32::MAX
assert!(inserted);
}
#[test]
fn test_insert_range_zero_inclusive() {
let mut b = RoaringBitmap::new();
let inserted = b.insert_range(0..=0);
// `insert_range(value..=value)` appears equivalent to `insert(value)`
assert_eq!(inserted, 1);
assert!(b.contains(0));
}
#[test]
fn test_insert_range_max_u32_inclusive() {
let mut b = RoaringBitmap::new();
let inserted = b.insert_range(u32::MAX..=u32::MAX);
// But not equivalent for u32::MAX
assert_eq!(inserted, 1); // Fails - left: 0, right: 1
assert!(b.contains(u32::MAX));
}tomjw64 yesterday •
To add onto this, since it does not seem possible to add u32::MAX with insert_range (any Range argument with an end_bound of Inclusive(u32::MAX) will insert 0 elements), if this is intended behavior, wouldn't this function be able to return a u32? Maybe I'm missing something basic. e.g.
#[test]
fn test_insert_all_u32() {
let mut b = RoaringBitmap::new();
let inserted = b.insert_range(0..u32::MAX); // Largest possible range seemingly allowed
assert_eq!(inserted, u32::MAX as u64); // Still not bigger than u32::MAX
}Kerollmops yesterday
Author
Member
Hello 
@tomjw64
Hum... Thank you very much, I borrowed your tests and will investigate this bug.
Indeed, this is not correct, we should be able to insert up to u32::MAX by using an inclusive bound.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK