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

Add doublechecked::Roaring64Map + tests, fix issues found by it #387

Merged
merged 5 commits into from
Sep 7, 2022

Conversation

SLieve
Copy link
Contributor

@SLieve SLieve commented Sep 5, 2022

The implementation of both doublechecked::Roaring64Map and the tests are mostly copied from the 32-bit version. I ran clang-format on the newly added code in tests/cpp_random_unit.cpp. I also added a statement to print the random seed, to reproduce failures only reproduced by specific seeds.

Adding this random test surfaced a few issues:

  1. Roaring64Map::isSubset did not work correctly when a Roaring value was empty. It would return false if the key for that value was not found in the other Roaring64Map. The correct behavior is to return true, just as empty Roarings return true for all isSubset calls.
  2. Roaring64Map::flip did not check for a valid range.
  3. convert_to_bitset_or_array_container would loop forever if run_end == uint16 max.

@lemire
Copy link
Member

lemire commented Sep 5, 2022

This looks very good to me.

@lemire lemire merged commit 697c014 into RoaringBitmap:master Sep 7, 2022
@SLieve SLieve deleted the 64_random branch September 7, 2022 15:55
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