How do folks feel about migrating to C++ 20?

[mbasmanova]

It would be cool to get access to coroutines available since C++ 20. How do folks feel about upgrading?

CC: @majetideepak @pedroerp @Yuhta @rui-mo @FelixYBW

[mbasmanova]

CC: @amitkdutta

[FelixYBW]

c++20 is supported since GCC8, Gluten use GCC9 at least. So it's not a block issues to migrate.

Should we step more to C++23? It's supported GCC11.

[pedroerp]

C++20 also gives us std::chrono::time_zone, so we could get rid of the external/date stuff that has been sort of problematic.

I think the main question is where compilers (and distros) are with C++20 support. What is the default compiler shipping with last version of distros like Ubuntu, Fedora, but also MacOS? Do they fully support C++20?

[FelixYBW]

GCC8 supports c++20
GCC8

we need some tests on OSes

[majetideepak]

Moving to C++20 should be good if we can benefit from coroutines and std::chrono::time_zone
CC: @czentgr

[czentgr]

Here are the current usages:

• CentOS 9 - default is GCC-11 but setups scripts installs and uses GCC-12
• Ubuntu 22.04 - default is GCC-11
• Ubuntu 24.04 - default is GCC-13
• MacOS - Apple Clang-15

There is a PR (#10363) that also allows the setup with Clang-15 on Centos9 and Ubuntu (22.04 and 24.04). The idea is to eventually standardize on Clang on all platforms.

Clang support for C++20 and C++23 PARTIAL

From that perspective Clang 15 supports C++20 and C++23 partially: https://clang.llvm.org/cxx_status.html

For coroutines the doc says:

Fully supported on all targets except Windows, which still has some stability and ABI issues.

There doesn't seem to be info on the chrono::time_zone type that is mentioned in the Clang docs and probably would need a test.

GCC-11 support for C++20 looks COMPLETE, C++23 PARTIAL

GCC support for the standards
https://gcc.gnu.org/projects/cxx-status.html

GCC-11 looks to have complete support for C++20 and partial support for C++23 (a small set).
Also not mentioned in C++20 list is time_zone.

[pedroerp]

So that means that standadizing in GCC 11+ and Clang 15+ would be a fair trade off today?

Btw, Fedora 38 and 39 seem to also come with GCC 13. Fedora 40 has an even newer version.

[czentgr]

I would suggest GCC-12 and Clang-15 for now. For GCC-13 there might still be some issues. Though some have been cleaned up since. So not sure how many remain. Maybe something to look at also. But focus was getting Clang to work. Clang-17 would also work (but is a problem because CentOS9 moved from default Clang-17 to default Clang-18 in a recent update - which caused issues again).

As minimum, yes, GCC-11+ and Clang-15+ would be a good base.

[mbasmanova]

Folks, it seems there is consensus on the benefits and feasibility of moving to C++ 20. If that's the case, can we start the migration? Anyone interested in driving this effort?

[majetideepak]

@mbasmanova Masha: @czentgr and I should be able to help with this. Christian added support for clang15 recently.

[mbasmanova]

Thank you @majetideepak and @czentgr !

[pedroerp]

Assuming this is already supported by the default compilers (gcc and clang) shipping with the latest version of major distros, I'm +1.

[czentgr]

The initial investigation using gcc12 showed only few issues. No major issues. I can get it to compile by ignoring deprecated errors.
We'll see how it affects Clang 15 and also need to give it a try with gcc11.

There are 3 issues that are resolved by ignoring the warnings and we should fix them. The first one is likely the one with most work.

1. Using volatile for atomic access. This might need a replacement with std::atomic types.

In file included from /root/oss/velox/velox/common/base/tests/Memcpy.cpp:17:
/root/oss/velox/./velox/common/base/Semaphore.h: In member function ‘void facebook::velox::Semaphore::acquire()’:
/root/oss/velox/./velox/common/base/Semaphore.h:40:9: error: ‘--’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]
   40 |       --count_;
      |         ^~~~~~
/root/oss/velox/./velox/common/base/Semaphore.h:42:9: error: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]
   42 |       ++numWaiting_;
      |         ^~~~~~~~~~~
/root/oss/velox/./velox/common/base/Semaphore.h:44:9: error: ‘--’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]
   44 |       --numWaiting_;
      |         ^~~~~~~~~~~
/root/oss/velox/./velox/common/base/Semaphore.h:45:9: error: ‘--’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]
   45 |       --count_;
      |         ^~~~~~
/root/oss/velox/./velox/common/base/Semaphore.h: In member function ‘void facebook::velox::Semaphore::release()’:
/root/oss/velox/./velox/common/base/Semaphore.h:52:7: error: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]
   52 |     ++count_;
      |       ^~~~~~
/root/oss/velox/velox/common/base/tests/Memcpy.cpp: In function ‘int main(int, char**)’:
/root/oss/velox/velox/common/base/tests/Memcpy.cpp:82:18: error: compound assignment with ‘volatile’-qualified left operand is deprecated [-Werror=volatile]
   82 |         totalSum +=
      |         ~~~~~~~~~^~
   83 |         sum(reinterpret_cast<uint64_t*>(other), bytes / sizeof(int64_t)));
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/oss/velox/velox/common/base/tests/Memcpy.cpp:82:18: error: using value of assignment with ‘volatile’-qualified left operand is deprecated [-Werror=volatile]
cc1plus: all warnings being treated as errors

2. This is a simple change to replace the capture with [=, this]. There is some discussion on why this occurred.

In file included from /root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h:23,
                 from /root/oss/velox/velox/serializers/tests/UnsafeRowSerializerTest.cpp:20:
/root/oss/velox/./velox/vector/tests/utils/VectorMaker.h: In lambda function:
/root/oss/velox/./velox/vector/tests/utils/VectorMaker.h:148:46: error: implicit capture of ‘this’ via ‘[=]’ is deprecated in C++20 [-Werror=deprecated]
  148 |         std::make_unique<SimpleVectorLoader>([=](RowSet rowSet) {
      |                                              ^
/root/oss/velox/./velox/vector/tests/utils/VectorMaker.h:148:46: note: add explicit ‘this’ or ‘*this’ capture
/root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h: In lambda function:
/root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h:784:46: error: implicit capture of ‘this’ via ‘[=]’ is deprecated in C++20 [-Werror=deprecated]
  784 |         std::make_unique<SimpleVectorLoader>([=](RowSet rows) {
      |                                              ^
/root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h:784:46: note: add explicit ‘this’ or ‘*this’ capture
/root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h: In lambda function:
/root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h:802:46: error: implicit capture of ‘this’ via ‘[=]’ is deprecated in C++20 [-Werror=deprecated]
  802 |         std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
      |                                              ^
/root/oss/velox/./velox/vector/tests/utils/VectorTestBase.h:802:46: note: add explicit ‘this’ or ‘*this’ capture
cc1plus: all warnings being treated as errors

3. std::is_pod_v is deprecated. Only a single use of this. This can be replace it with std::is_standard_layout_v which tightens the check and I believe is what the intent was at

   velox/velox/dwio/common/SelectiveColumnReader.h

   Line 342 in 3bf1a74

   std::is_pod_v<T>,

   .
   Reference: https://en.cppreference.com/w/cpp/types/is_pod

---

As a result of updating the C++ standard we should make gcc11 (and clang15) the minimum supported version. We also should drop support for Ubuntu 20.04 (and gcc9).
gcc9 does not support coroutines and gcc10 requires and extra compile flag.

[majetideepak]

Let's enforce minimum compiler versions and not Linux distros. Can we install gcc11 on Ubuntu 20.04?

[czentgr]

Yes, you can. It is a bit more work because you need to add a toolchain repo and install it from there. And then just as with gcc12 on Centos9 need to switch to it. Details are here: https://stackoverflow.com/questions/67298443/when-gcc-11-will-appear-in-ubuntu-repositories

[czentgr]

Ubuntu 20.04 is officially supported until April 2025 so I suppose it is still good to go and we shouldn't drop support just now.

[pedroerp]

Maybe we should move this to an Issue?

[czentgr]

I ceated issue #10953 for this.