RoaringBitmap / CRoaring

Roaring bitmaps in C (and C++), with SIMD (AVX2, AVX-512 and NEON) optimizations: used by Apache Doris, ClickHouse, and StarRocks
http://roaringbitmap.org/
Other
1.55k stars 265 forks source link

roaring_bitmap_contains is incorrect after roaring_bitmap_create #81

Closed KingJiangNet closed 7 years ago

KingJiangNet commented 7 years ago

After calling roaring_bitmap_create(), roaring_bitmap_contains(0) returns true.

lemire commented 7 years ago

Can you provide a complete unit test? I cannot reproduce the problem:

https://github.com/RoaringBitmap/CRoaring/commit/259da686cf4ae9325c106c848c5dedddddc1f106

KingJiangNet commented 7 years ago

It was found after I forked it from: Brandon Smith wrote a C# wrapper available at https://github.com/RogueException/CRoaring.Net (works for Windows and Linux under x64 processors)

3 tests failed: TestAdd(), TestNot() and TestRemove(). I checked the TestAdd(), it is caused by the reason as I mentioned. I'm not sure if it is caused by the Clang compiler in VS2015. I have uploaded the compiled dlls and pdbs from my PC: https://github.com/KingJiangNet/CRoaring.Net/releases/download/0.2.20/CRoaring_0.2.20_alpha.7z

lemire commented 7 years ago

@KingJiangNet First thing first, if you compile the code under Visual Studio using this approach: https://github.com/RoaringBitmap/CRoaring#building-visual-studio-under-windows Does it work for you? Does it pass all unit tests? (You should be able to do this in a couple of minutes.)

If it does pass the unit tests, then the problem is probably elsewhere.

If we cannot reproduce the problem, we have no chance of ever solving it. Can you provide us with steps to follow to reproduce the problem?

KingJiangNet commented 7 years ago

@lemire The reason has been found. It is caused by marshaling the 1 byte bool from C to C#, I have corrected it in the C# wrapper by using [return: MarshalAs(UnmanagedType.I1)] on the pinvoke methods which return bool. Thanks for your help.

lemire commented 7 years ago

c.c. @RogueException