Clang c++ compilation fixes #4739
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bazsi
force-pushed
the
clang-cxx-fixes
branch
from
a33b9c1
to
a141980
Compare
This was referenced Dec 3, 2023
|
RHEL 9 build still fails: You can find the full log at https://download.copr.fedorainfracloud.org/results/czanik/syslog-ng-githead/epel-9-x86_64/06722699-syslog-ng/builder-live.log.gz FreeBSD test will come soon. |
|
FreeBSD CLANG fails: Most likely because it has c++, but no libstdc++, according to the output of Next up: trying gcc on FreeBSD again. |
|
Using GCC it compiles, but some modules fail to load: |
|
OK. I'm not a programmer, so I might write something stupid :-) It seems that FreeBSD has clang and uses libc++, while gcc has libstdc++. They are not fully compatible. Software compiled with one cannot be properly linked with the other one. The following option could make sure that syslog-ng uses libc++ instead of libstdc++ |
|
OK. I did a bit of experimenting. Replacing all occurrences of All I did was: and So, it seems that I found the solution for my favorite OS, FreeBSD. However, a lot more users run RHEL, and RHEL 9 is getting popular... |
|
I was just wondering how we got that the g_pointer_atomic_*() functions support anything but pointers? Because we passing a gssize to them at the moment. @MrAnno do you have a recollection of that? |
|
We wanted 64-bit stats counters wherever they are available, so we implemented atomic_gssize on top of atomic pointers. There is a static assertion making sure that the size of gssize matches the size of a void pointer. |
|
The same is true for LogSource window trackers. We needed the first bit of the window to make sure that "forced suspending" (disk-buffer) remains atomic for real. |
bazsi
force-pushed
the
clang-cxx-fixes
branch
from
a141980
to
5b565fe
Compare
|
Just pushed an alternative solution. This should trigger a lot of compilation warnings but should let syslog-ng compile. Details: https://gitlab.gnome.org/GNOME/glib/-/issues/3193#note_1935964 @czanik can you please check if his indeed compiles on RHEL9? The FreeBSD issue is separate, if you compile with gcc the issue is that your protobuf libraries are compiled with clang and that causes the linker error. Actually you get the same issue with clang but it errors out instead of simply generating an invalid binary. The right solution is indeed to link against -lc++ instead of -lstdc++ but we would need to detect somehow which one is the right one. |
|
Build FAILURE |
|
@bazsi Thanks! I'm still waiting for the Fedora / RHEL builds, but (open)SUSE build results are already available. This patch breaks SLES 12 SP5: |
|
And it breaks RHEL 7 and RHEL 8 builds with the same error message. Still waiting for RHEL 9... |
|
@bazsi RHEL 9 compile finished successfully: https://download.copr.fedorainfracloud.org/results/czanik/syslog-ng-githead/epel-9-x86_64/06728673-syslog-ng/builder-live.log.gz I'll try to install it now. |
|
centos7 and sles sp5 breaks as they don't have glib 2.68 which this PR would start depending on. |
bazsi
force-pushed
the
clang-cxx-fixes
branch
from
5b565fe
to
fbc2dd5
Compare
|
I've tried to fix up centos7. let's see if that works. |
|
Build FAILURE |
bazsi
force-pushed
the
clang-cxx-fixes
branch
2 times, most recently
from
eb49322
to
4df8bd3
Compare
|
Build FAILURE |
bazsi
force-pushed
the
clang-cxx-fixes
branch
2 times, most recently
from
d9e8357
to
1172f43
Compare
|
Build FAILURE |
Yepp, we discussed this one once with @alltilla, not an easy fix as a bunch of build stuff depends on this (CI, PE) |
|
Yes the macOS filename case insensitivity is what tricks us here, I believe. |
I finally got a working build of GRPC dependant modules (so basically now all the c++ modules) on macOS as well 2 questions
|
no2. is solved by #4775 |
alltilla
force-pushed
the
clang-cxx-fixes
branch
from
f7062f3
to
d660f46
Compare
|
Rebased to master and resolved the merge conflicts. |
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…tring functions Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
cr_assert() would expect arguments to the format string but they were not supplied. clang warns about this. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
clang complained about an unreferenced local variable in the generated grammar. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Not all warnings are supported everywhere, so check their presence from configure.ac before using them in Makefile.am. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
This makes sure syslog-ng.h and glib.h are included from outside of extern "C" blocks. * Header ordering: * - syslog-ng.h needs to come first even in cpp/hpp files. You don't * need cpp-start/end wrapper around syslog-ng.h. This include can * happen indirectly, e.g. if you include any other header that should * take care of syslog-ng.h * * - syslog-ng.h will include glib too, which is also C++ safe. This is * needed as some constructs in glib only work if it is outside of an * extern "C" block. * * - Use the usual header ordering conventions, e.g. include the closest * header first (e.g. the one associated with your module) and then * iterate to furthest. This ensures that each header is actually * standalone and includes all its dependencies. * * - If you use a .hpp file, include .h from it and make sure the .h * includes syslog-ng.h and includes all other .h files by wrapping them * using cpp-start/end.h as needed. This takes care of the first rule. * * - In your .cpp files, include the .hpp file but not the .h file which is * already included. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…ecent glib This fixes clang++ compilation as explained here: https://gitlab.gnome.org/GNOME/glib/-/issues/3193#note_1935964 Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
…er associated with CC Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
alltilla
force-pushed
the
clang-cxx-fixes
branch
from
d660f46
to
46f8a25
Compare
|
Rebased to master. |
alltilla
approved these changes
Jan 9, 2024
alltilla
approved these changes
Jan 9, 2024
alltilla
approved these changes
Jan 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Let's see if this fixes
#4652