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

Fix C++ amalgamation errors #547

Merged
merged 2 commits into from
Jan 10, 2024
Merged

Conversation

SLieve
Copy link
Contributor

@SLieve SLieve commented Jan 9, 2024

The C++ amalgamation example was broken, see #545 (comment). This PR fixes the two errors: explicit casting and redefining htobe64.

I've moved the big endian conversion macros into portability.h, as that seemed like a better home anyway. In the process I've renamed them to avoid naming conflicts (e.g. with <endian.h>).

Tested:

$ ./amalgamation.sh
We are about to amalgamate all CRoaring files into one source file.
For rationale, see: https://www.sqlite.org/amalgamation.html
and: https://en.wikipedia.org/wiki/Single_Compilation_Unit

Creating roaring.h...
Creating roaring.c...
Creating amalgamation_demo.c...
Creating roaring.hh...
Creating amalgamation_demo.cpp...

Files have been written to current directory: /home/soerian/repos/CRoaring
-rw-rw-r-- 1 soerian soerian    825 Jan  9 23:05 amalgamation_demo.c
-rw-rw-r-- 1 soerian soerian   2526 Jan  9 23:05 amalgamation_demo.cpp
-rw-rw-r-- 1 soerian soerian 951043 Jan  9 23:05 roaring.c
-rw-rw-r-- 1 soerian soerian  87349 Jan  9 23:05 roaring.h
-rw-rw-r-- 1 soerian soerian 107166 Jan  9 23:05 roaring.hh

The interface is found in the file 'include/roaring/roaring.h'.

For C, try:
cc -O3 -std=c11  -o amalgamation_demo amalgamation_demo.c  && ./amalgamation_demo

For C++, try:
c++ -O3 -std=c++11 -o amalgamation_demo amalgamation_demo.cpp  && ./amalgamation_demo

You can build a shared library with the following command:
cc -O3 -std=c11 -shared -o libroaring.so -fPIC roaring.c

$ cc -O3 -std=c11  -o amalgamation_demo amalgamation_demo.c  && ./amalgamation_demo
cardinality = 900
cardinality (64-bit) = 900
1000

$ c++ -O3 -std=c++11 -o amalgamation_demo amalgamation_demo.cpp  && ./amalgamation_demo

cardinality = 900
cardinality = 900

Copy link
Member

@Dr-Emann Dr-Emann left a comment

Choose a reason for hiding this comment

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

Nice, I do think portability.h is the right place for those macros anyway.

Also define a unique macro name to avoid conflicts with endian.h
@SLieve SLieve force-pushed the r64_amalgamation_fix branch from aaa47ae to 9058881 Compare January 9, 2024 22:18
@lemire
Copy link
Member

lemire commented Jan 10, 2024

Merging.

@lemire lemire merged commit 4230b8a into RoaringBitmap:master Jan 10, 2024
25 checks passed
@SLieve SLieve deleted the r64_amalgamation_fix branch January 22, 2024 21:07
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.

3 participants