[libc] raise CMAKE_CXX_STANDARD to C++20 #87020
Conversation
This is supported by clang-10+ and gcc-10+. Our minimum supported versions are currently clang-11+, gcc-12+. We have a few places where we were already using C++20. libc++ is currently using C++23. We can likely join them, but let's jump to C++20 first (and ensure our buildbots can support C++23).
|
@llvm/pr-subscribers-libc Author: Nick Desaulniers (nickdesaulniers) ChangesThis is supported by clang-10+ and gcc-10+. Our minimum supported versions are We have a few places where we were already using C++20. libc++ is currently using C++23. We can likely join them, but let's jump to Full diff: https://github.com/llvm/llvm-project/pull/87020.diff 6 Files Affected:
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index a0d79858a896ad..1517d698915510 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -26,8 +26,8 @@ if (CMAKE_BUILD_TYPE STREQUAL "Debug")
add_definitions("-D_DEBUG")
endif()
-# Default to C++17
-set(CMAKE_CXX_STANDARD 17)
+# Default to C++20
+set(CMAKE_CXX_STANDARD 20)
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules")
diff --git a/libc/config/linux/app.h b/libc/config/linux/app.h
index 766cd49e88f6f7..0e29176a18d018 100644
--- a/libc/config/linux/app.h
+++ b/libc/config/linux/app.h
@@ -108,8 +108,6 @@ struct TLSDescriptor {
// Note that, dependending the target architecture ABI, it can be the
// same as |addr| or something else.
uintptr_t tp = 0;
-
- constexpr TLSDescriptor() = default;
};
// Create and initialize the TLS area for the current thread. Should not
diff --git a/libc/src/__support/threads/fork_callbacks.cpp b/libc/src/__support/threads/fork_callbacks.cpp
index 54fda676f281ed..c019337afc829e 100644
--- a/libc/src/__support/threads/fork_callbacks.cpp
+++ b/libc/src/__support/threads/fork_callbacks.cpp
@@ -20,7 +20,6 @@ struct ForkCallbackTriple {
ForkCallback *prepare = nullptr;
ForkCallback *parent = nullptr;
ForkCallback *child = nullptr;
- constexpr ForkCallbackTriple() = default;
};
class AtForkCallbackManager {
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index e526ba040befb7..2a6bda804ea4e5 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -411,8 +411,6 @@ add_entrypoint_object(
atexit.cpp
HDRS
atexit.h
- CXX_STANDARD
- 20 # For constinit of the atexit callback list.
DEPENDS
libc.src.__support.CPP.new
libc.src.__support.OSUtil.osutil
diff --git a/libc/test/src/network/CMakeLists.txt b/libc/test/src/network/CMakeLists.txt
index 222205dfe247a6..1cda9289c2beec 100644
--- a/libc/test/src/network/CMakeLists.txt
+++ b/libc/test/src/network/CMakeLists.txt
@@ -6,8 +6,6 @@ add_libc_unittest(
libc_network_unittests
SRCS
htonl_test.cpp
- CXX_STANDARD
- 20
DEPENDS
libc.src.network.htonl
libc.src.network.ntohl
@@ -19,8 +17,6 @@ add_libc_unittest(
libc_network_unittests
SRCS
htons_test.cpp
- CXX_STANDARD
- 20
DEPENDS
libc.src.network.htons
libc.src.network.ntohs
@@ -32,8 +28,6 @@ add_libc_unittest(
libc_network_unittests
SRCS
ntohl_test.cpp
- CXX_STANDARD
- 20
DEPENDS
libc.src.network.htonl
libc.src.network.ntohl
@@ -45,8 +39,6 @@ add_libc_unittest(
libc_network_unittests
SRCS
ntohs_test.cpp
- CXX_STANDARD
- 20
DEPENDS
libc.src.network.htons
libc.src.network.ntohs
diff --git a/libc/test/src/time/CMakeLists.txt b/libc/test/src/time/CMakeLists.txt
index 51cacef0a62fe8..32cad8cf6c5784 100644
--- a/libc/test/src/time/CMakeLists.txt
+++ b/libc/test/src/time/CMakeLists.txt
@@ -9,8 +9,6 @@ add_libc_unittest(
HDRS
TmHelper.h
TmMatcher.h
- CXX_STANDARD
- 20
DEPENDS
libc.src.time.asctime
)
@@ -24,8 +22,6 @@ add_libc_unittest(
HDRS
TmHelper.h
TmMatcher.h
- CXX_STANDARD
- 20
DEPENDS
libc.src.time.asctime_r
)
@@ -95,8 +91,6 @@ add_libc_unittest(
HDRS
TmHelper.h
TmMatcher.h
- CXX_STANDARD
- 20
DEPENDS
libc.src.time.mktime
libc.src.__support.CPP.limits
|
|
Probably should mention that there was some change to C++20 from C++17 around aggregate initialization. |
|
@gchatelet should I be upgrading benchmarks, too? |
|
Just linking in all the goodies we get from C++20
|
For consistency yes |
|
(going to sit on this PR for at least a week to collect feedback RFC style) |
|
I do not recommend using C++20 until Clang 17, due to multiple serious bugs in codegen in previous versions. For LLVM, we also upped the minimum to 17.0.6 due to a late issue with the spaceship operator implementation. If you switch to C++20 for libc, you ought to raise the minimum version as well. See also https://discourse.llvm.org/t/rfc-clang-17-0-6-would-be-minimum-version-to-build-llvm-in-c-20/75345/8 |
|
@jyknight , thanks for speaking up and linking to that thread. I'll trust your experience to say we should avoid having to redebug problems @jyknight has already observed. Let's start by first revisiting our documented supported compiler versions. I'll start a new PR/RFC for that. Once we raise minimum compiler versions, then we can revisit raising the C++ language standard used for the project. Moving this to draft for now to help signal that it's on the back burner. |
|
Further discussions today; it sounds like libc++ was able to move away from the
This might be tricky for us because of host dependencies such as:
Though it sounds like a have a "bootstrap cross build" https://libc.llvm.org/full_cross_build.html#bootstrap-cross-build where this might not be a problem? cc @petrhosek |
This is supported by clang-10+ and gcc-10+. Our minimum supported versions are
currently clang-11+, gcc-12+.
We have a few places where we were already using C++20.
libc++ is currently using C++23. We can likely join them, but let's jump to
C++20 first (and ensure our buildbots can support C++23).