-
Notifications
You must be signed in to change notification settings - Fork 444
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
Switch compilation from C++17 to C++20. #4347
base: main
Are you sure you want to change the base?
Conversation
I started looking at this some time ago in #4297. I was probably too sneaky though. Still not complete, there is quite lot of things that break and I don't have much time for it now. The main problem is clashing operators. |
I can close this PR in favor of yours. I just wanted to see what needs to be done and it seems moving up is non-trivial. |
OK. Yes, it does not seem as easy as was the move to C++17. |
There is not much missing for C++20 support. It looks like there is just one small failure left. |
Hmm, the error is in boost, that might be hard to fix unless it is our bad usage of boost of course :-). Seems to be related to a particular version as it fails just in the sanitizers build. What are your thoughts on switching? Do you think it is time for that? We don't get many goodies until we bump minimal required GCC, but even some things from GCC 9 are nice. It would also unlock C++20 for downstreams that have newer compilers. On the other hand, we would be having a more experimental setup, with GCC that is far from stable on C++20. |
The boost version should be the same for Ubuntu 20.04 but this compiler is Clang. Maybe that is the reason.
I am happy to switch, there are some folks that have been waiting for us to upgrade to C++20 already. We can try to be conservative with the features we introduce and keep Ubuntu 20.04 as the minimum supported version until it hits EOL. |
Cherrypicking #4663 fixes the issue. There were two more small fixes required which I can fork into a separate PR. |
Personally I'm OK with the operator fixes to be there, but the boost should be separate. |
I've just notice that |
Yes, the changes were for testing purposes, I will move them into separate PRs. |
63d7c31
to
07beb28
Compare
It looks like there is very little that can be done to fix the compilation issue sans fixing boost. I am not sure why this pops up with clang only though. |
Signed-off-by: fruffy <[email protected]>
@asl Any idea how to workaround this Boost C++20 bug? https://stackoverflow.com/a/67702145 It's the only thing blocking compilation for C++20 for our CI. Otherwise, #4663 is required. |
Does this only show on the combination of versions of Clang and Boost on Ubuntu 20.04? Do we know why it does not show with GCC on the same OS? That sounds quite weird for me frankly as it seems like problem in interaction between standard library and boost, not compiler and boost (and both GCC and clang should presumably use the same version of libstdc++ on the system). If it was just Clang problem, I would suggest trying to move the sanitizers to Ubuntu 22.04. Ubuntu 20 is quite old now after all. That said, we claim to support Clang 6, that should probably be revisited too, we don't test with it. I doubt anyone would require Clang unless on macOS. We should keep it supported, we need the clang-related tools like sanitizers and tidy, but we can probably be a little bit more aggressive in dropping support for old versions. |
I tend to agree with @vlstill as it seems to be some bad interaction with standard library. Anyway, if boost uses things that were deprecated in C++17 and removed in C++20, then there is no way to workaround rather than patch / upgrade the sources. Or we can just get rid PS: There are reports about overheads (runtime and compile time) of |
Would be nice, but quite tough because of the difference in formatting... not even sure how you could start here.
What about fmt? Will also be much easier to replace with |
it is nice, yes. But probably we'd need to check / benchmark different solutions |
Hmm, I think at least for the usage of boost::format in the compiler most of the calls are used for debugging/warnings/errors, which are not that performance-critical, no? Going with the standard solution is likely fine here. |
The complains were both for compile time and for runtime IIRC. This is why I said we need to check :) |
Apparently boost 1.74 is the one to fix this: https://www.boost.org/users/history/version_1_74_0.html
So that means the one in Ubuntu 22.04 is OK (that is 1.74), but the one in 20.04 (1.71) is bad. |
The reason this fails in the sanitizer build has actually nothing to do with clang and everything to do with GCC 10, which is installed in that image, but not in the one used for normal Ubuntu 20.04 build. It is GCC 10 that first removes the overload that |
C++20 is now supported in most major distributions and compilers and there are several features that are useful for us to have. Let's switch to C++20.