From f3fa8ae1250a47fbe62655db8f2db5cc88668c70 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 15 Jan 2025 16:06:01 -0500 Subject: [PATCH] [libc++] Don't implement before C++23 https://github.com/llvm/llvm-project/pull/95498 implemented a libc++ extension where would forward to even before C++23. Unfortunately, this was found to be a breaking change (with fairly widespread impact) since that changes whether _Atomic(T) is a C style atomic or std::atomic. In principle, this can even be an ABI break. We generally don't implement extensions in libc++ because they cause so many problems, and that extension had been accepted because it was deemed pretty small and only a quality of life improvement. Since it has widespread impact on valid C++20 (and before) code, this patch removes the extension before we ship it in any public release. --- libcxx/include/atomic | 4 +++ libcxx/include/stdatomic.h | 7 ++++-- ...compatible_with_stdatomic.compile.pass.cpp | 11 ++++---- .../incompatible_with_stdatomic.verify.cpp | 22 ++++++++++++++++ .../dont_hijack_header.compile.pass.cpp | 24 ++++++++++++++++++ .../dont_hijack_header.cxx23.compile.pass.cpp | 25 +++++++++++++++++++ 6 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 libcxx/test/libcxx/atomics/atomics.syn/incompatible_with_stdatomic.verify.cpp create mode 100644 libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.compile.pass.cpp create mode 100644 libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.cxx23.compile.pass.cpp diff --git a/libcxx/include/atomic b/libcxx/include/atomic index 80f9e437bfaab..0c641c7a68b58 100644 --- a/libcxx/include/atomic +++ b/libcxx/include/atomic @@ -592,6 +592,10 @@ template #else # include <__config> +# if _LIBCPP_STD_VER < 23 && defined(_LIBCPP_STDATOMIC_H) +# error is incompatible with before C++23. Please compile with -std=c++23. +# endif + # include <__atomic/aliases.h> # include <__atomic/atomic.h> # include <__atomic/atomic_flag.h> diff --git a/libcxx/include/stdatomic.h b/libcxx/include/stdatomic.h index a0b46e3b7bc17..ea57a1fb06702 100644 --- a/libcxx/include/stdatomic.h +++ b/libcxx/include/stdatomic.h @@ -126,7 +126,7 @@ using std::atomic_signal_fence // see below # pragma GCC system_header # endif -# if defined(__cplusplus) +# if defined(__cplusplus) && _LIBCPP_STD_VER >= 23 # include # include @@ -233,11 +233,14 @@ using std::atomic_thread_fence _LIBCPP_USING_IF_EXISTS; # else +// Before C++23, we include the next on the path to avoid hijacking +// the header, since Clang and some platforms have been providing this header for +// a long time and users rely on it. # if __has_include_next() # include_next # endif -# endif // defined(__cplusplus) +# endif // defined(__cplusplus) && _LIBCPP_STD_VER >= 23 #endif // defined(__cplusplus) && __cplusplus < 201103L && defined(_LIBCPP_USE_FROZEN_CXX03_HEADERS) #endif // _LIBCPP_STDATOMIC_H diff --git a/libcxx/test/libcxx/atomics/atomics.syn/compatible_with_stdatomic.compile.pass.cpp b/libcxx/test/libcxx/atomics/atomics.syn/compatible_with_stdatomic.compile.pass.cpp index 323072da14463..30e9672a25683 100644 --- a/libcxx/test/libcxx/atomics/atomics.syn/compatible_with_stdatomic.compile.pass.cpp +++ b/libcxx/test/libcxx/atomics/atomics.syn/compatible_with_stdatomic.compile.pass.cpp @@ -7,15 +7,16 @@ //===----------------------------------------------------------------------===// // UNSUPPORTED: no-threads +// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20 // XFAIL: FROZEN-CXX03-HEADERS-FIXME -// This test verifies that redirects to . As an extension, -// libc++ enables this redirection even before C++23. +// This test verifies that redirects to . -// Ordinarily, can be included after , but including it -// first doesn't work because its macros break . Verify that -// can be included first. +// Before C++23, can be included after , but including it +// first doesn't work because its macros break . Fixing that is the point +// of the C++23 change that added to C++. Thus, this test verifies +// that can be included first. #include #include diff --git a/libcxx/test/libcxx/atomics/atomics.syn/incompatible_with_stdatomic.verify.cpp b/libcxx/test/libcxx/atomics/atomics.syn/incompatible_with_stdatomic.verify.cpp new file mode 100644 index 0000000000000..ca092d9c60275 --- /dev/null +++ b/libcxx/test/libcxx/atomics/atomics.syn/incompatible_with_stdatomic.verify.cpp @@ -0,0 +1,22 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: no-threads +// REQUIRES: c++03 || c++11 || c++14 || c++17 || c++20 + +// This test ensures that we issue a reasonable diagnostic when including after +// has been included. Before C++23, this otherwise leads to obscure errors +// because may try to redefine things defined by . + +// Ignore additional weird errors that happen when the two headers are mixed. +// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error -Xclang -verify-ignore-unexpected=warning + +#include +#include + +// expected-error@*:* {{ is incompatible with before C++23.}} diff --git a/libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.compile.pass.cpp b/libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.compile.pass.cpp new file mode 100644 index 0000000000000..6df80daf9414e --- /dev/null +++ b/libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.compile.pass.cpp @@ -0,0 +1,24 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: no-threads + +// This test ensures that we don't hijack the header (e.g. by providing +// an empty header) even when compiling before C++23, since some users were using the +// Clang or platform provided header before libc++ added its own. + +// On GCC, the compiler-provided is not C++ friendly, so including +// doesn't work at all if we don't use the provided by libc++ in C++23 and above. +// XFAIL: (c++11 || c++14 || c++17 || c++20) && gcc + +#include + +void f() { + atomic_int i; // just make sure the header isn't empty + (void)i; +} diff --git a/libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.cxx23.compile.pass.cpp b/libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.cxx23.compile.pass.cpp new file mode 100644 index 0000000000000..9a30b439c96c9 --- /dev/null +++ b/libcxx/test/libcxx/atomics/stdatomic.h.syn/dont_hijack_header.cxx23.compile.pass.cpp @@ -0,0 +1,25 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: no-threads +// XFAIL: FROZEN-CXX03-HEADERS-FIXME + +// This test verifies that DOES NOT redirect to before C++23, +// since doing so is a breaking change. Several things can break when that happens, +// because the type of _Atomic(T) changes from _Atomic(T) to std::atomic. +// +// For example, redeclarations can become invalid depending on whether they +// have been declared with in scope or not. + +// REQUIRES: c++03 || c++11 || c++14 || c++17 || c++20 + +#include +#include +#include + +static_assert(!std::is_same<_Atomic(int), std::atomic >::value, "");