Skip to content

Commit

Permalink
[libc++] Don't implement <stdatomic.h> before C++23
Browse files Browse the repository at this point in the history
llvm#95498 implemented a libc++
extension where <stdatomic.h> would forward to <atomic> 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<T>. 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.
  • Loading branch information
ldionne committed Jan 15, 2025
1 parent f09db6a commit f3fa8ae
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 7 deletions.
4 changes: 4 additions & 0 deletions libcxx/include/atomic
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,10 @@ template <class T>
#else
# include <__config>

# if _LIBCPP_STD_VER < 23 && defined(_LIBCPP_STDATOMIC_H)
# error <atomic> is incompatible with <stdatomic.h> before C++23. Please compile with -std=c++23.
# endif

# include <__atomic/aliases.h>
# include <__atomic/atomic.h>
# include <__atomic/atomic_flag.h>
Expand Down
7 changes: 5 additions & 2 deletions libcxx/include/stdatomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <atomic>
# include <version>
Expand Down Expand Up @@ -233,11 +233,14 @@ using std::atomic_thread_fence _LIBCPP_USING_IF_EXISTS;

# else

// Before C++23, we include the next <stdatomic.h> 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(<stdatomic.h>)
# include_next <stdatomic.h>
# 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
Original file line number Diff line number Diff line change
Expand Up @@ -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 <stdatomic.h> redirects to <atomic>. As an extension,
// libc++ enables this redirection even before C++23.
// This test verifies that <stdatomic.h> redirects to <atomic>.

// Ordinarily, <stdatomic.h> can be included after <atomic>, but including it
// first doesn't work because its macros break <atomic>. Verify that
// <stdatomic.h> can be included first.
// Before C++23, <stdatomic.h> can be included after <atomic>, but including it
// first doesn't work because its macros break <atomic>. Fixing that is the point
// of the C++23 change that added <stdatomic.h> to C++. Thus, this test verifies
// that <stdatomic.h> can be included first.
#include <stdatomic.h>
#include <atomic>

Expand Down
Original file line number Diff line number Diff line change
@@ -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 <atomic> after
// <stdatomic.h> has been included. Before C++23, this otherwise leads to obscure errors
// because <atomic> may try to redefine things defined by <stdatomic.h>.

// 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 <stdatomic.h>
#include <atomic>

// expected-error@*:* {{<atomic> is incompatible with <stdatomic.h> before C++23.}}
Original file line number Diff line number Diff line change
@@ -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 <stdatomic.h> 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 <stdatomic.h> is not C++ friendly, so including <stdatomic.h>
// doesn't work at all if we don't use the <stdatomic.h> provided by libc++ in C++23 and above.
// XFAIL: (c++11 || c++14 || c++17 || c++20) && gcc

#include <stdatomic.h>

void f() {
atomic_int i; // just make sure the header isn't empty
(void)i;
}
Original file line number Diff line number Diff line change
@@ -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 <stdatomic.h> DOES NOT redirect to <atomic> 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<T>.
//
// For example, redeclarations can become invalid depending on whether they
// have been declared with <stdatomic.h> in scope or not.

// REQUIRES: c++03 || c++11 || c++14 || c++17 || c++20

#include <atomic>
#include <stdatomic.h>
#include <type_traits>

static_assert(!std::is_same<_Atomic(int), std::atomic<int> >::value, "");

0 comments on commit f3fa8ae

Please sign in to comment.