[libc++] Drop support for the C++20 Synchronization Library before C++20 #82008
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesWhen we initially implemented the C++20 synchronization library, we reluctantly accepted for the implementation to be backported to C++03 upon request from the person who provided the patch. This was when we were only starting to have experience with the issues this can create, so we flinched. Nowadays, we have a much stricter stance about not backporting features to previous standards. We have recently started fixing several bugs (and near bugs) in our implementation of the synchronization library. A recurring theme during these reviews has been how difficult to undertand the current code is, and upon inspection it becomes clear that being able to use a few recent C++ features (in particular lambdas) would help a great deal. The code would still be pretty intricate, but it would be a lot easier to reason about the flow of callbacks through things like __thread_poll_with_backoff. As a result, this patch drops support for the synchronization library before C++20. This makes us more strictly conforming and opens the door to major simplifications, in particular around atomic_wait which was supported all the way to C++03. This change will probably have some impact on downstream users, however since the C++20 synchronization library was added only in LLVM 10 (~3 years ago) and it's quite a niche feature, the set of people trying to use this part of the library before C++20 should be reasonably small. Full diff: https://github.com/llvm/llvm-project/pull/82008.diff 8 Files Affected:
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index 908d46b711a5a6..7f9eb8e28e8efc 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -50,6 +50,10 @@ Improvements and New Features
Deprecations and Removals
-------------------------
+- Support for the C++20 synchronization library (``<barrier>``, ``<latch>``, ``atomic::wait``, etc.) has been
+ removed in language modes prior to C++20. If you are using these features prior to C++20, you will need to
+ update to ``-std=c++20``.
+
- TODO: The ``LIBCXX_EXECUTOR`` CMake variables have been removed.
- TODO: The ``LIBCXX_ENABLE_ASSERTIONS`` CMake variable that was used to enable the safe mode has been deprecated and setting
diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index 3dfb6937d0325e..bcea21f5ce2e17 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -429,6 +429,8 @@ _LIBCPP_HIDE_FROM_ABI bool atomic_compare_exchange_strong_explicit(
return __o->compare_exchange_strong(*__e, __d, __s, __f);
}
+#if _LIBCPP_STD_VER >= 20
+
// atomic_wait
template <class _Tp>
@@ -481,6 +483,8 @@ _LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void atomic_notify_all(atomic<_T
__o->notify_all();
}
+#endif // _LIBCPP_STD_VER >= 20
+
// atomic_fetch_add
template <class _Tp>
diff --git a/libcxx/include/__atomic/atomic_base.h b/libcxx/include/__atomic/atomic_base.h
index 3ad3b562c59805..41f6920692a35a 100644
--- a/libcxx/include/__atomic/atomic_base.h
+++ b/libcxx/include/__atomic/atomic_base.h
@@ -102,6 +102,7 @@ struct __atomic_base // false
return std::__cxx_atomic_compare_exchange_strong(std::addressof(__a_), std::addressof(__e), __d, __m, __m);
}
+#if _LIBCPP_STD_VER >= 20
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void wait(_Tp __v, memory_order __m = memory_order_seq_cst) const
volatile _NOEXCEPT {
std::__cxx_atomic_wait(std::addressof(__a_), __v, __m);
@@ -122,6 +123,7 @@ struct __atomic_base // false
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void notify_all() _NOEXCEPT {
std::__cxx_atomic_notify_all(std::addressof(__a_));
}
+#endif // _LIBCPP_STD_VER >= 20
#if _LIBCPP_STD_VER >= 20
_LIBCPP_HIDE_FROM_ABI constexpr __atomic_base() noexcept(is_nothrow_default_constructible_v<_Tp>) : __a_(_Tp()) {}
diff --git a/libcxx/include/__atomic/atomic_flag.h b/libcxx/include/__atomic/atomic_flag.h
index a45a7183547726..b5ee2c272aee40 100644
--- a/libcxx/include/__atomic/atomic_flag.h
+++ b/libcxx/include/__atomic/atomic_flag.h
@@ -47,6 +47,7 @@ struct atomic_flag {
__cxx_atomic_store(&__a_, _LIBCPP_ATOMIC_FLAG_TYPE(false), __m);
}
+#if _LIBCPP_STD_VER >= 20
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void wait(bool __v, memory_order __m = memory_order_seq_cst) const
volatile _NOEXCEPT {
__cxx_atomic_wait(&__a_, _LIBCPP_ATOMIC_FLAG_TYPE(__v), __m);
@@ -63,6 +64,7 @@ struct atomic_flag {
__cxx_atomic_notify_all(&__a_);
}
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void notify_all() _NOEXCEPT { __cxx_atomic_notify_all(&__a_); }
+#endif
#if _LIBCPP_STD_VER >= 20
_LIBCPP_HIDE_FROM_ABI constexpr atomic_flag() _NOEXCEPT : __a_(false) {}
@@ -117,6 +119,7 @@ inline _LIBCPP_HIDE_FROM_ABI void atomic_flag_clear_explicit(atomic_flag* __o, m
__o->clear(__m);
}
+#if _LIBCPP_STD_VER >= 20
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_SYNC void
atomic_flag_wait(const volatile atomic_flag* __o, bool __v) _NOEXCEPT {
__o->wait(__v);
@@ -154,6 +157,7 @@ atomic_flag_notify_all(volatile atomic_flag* __o) _NOEXCEPT {
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_SYNC void atomic_flag_notify_all(atomic_flag* __o) _NOEXCEPT {
__o->notify_all();
}
+#endif // _LIBCPP_STD_VER >= 20
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/atomic b/libcxx/include/atomic
index 2e8f5b521a55eb..a95d745c1fe593 100644
--- a/libcxx/include/atomic
+++ b/libcxx/include/atomic
@@ -101,12 +101,12 @@ struct atomic
bool compare_exchange_strong(T& expc, T desr,
memory_order m = memory_order_seq_cst) noexcept;
- void wait(T, memory_order = memory_order::seq_cst) const volatile noexcept;
- void wait(T, memory_order = memory_order::seq_cst) const noexcept;
- void notify_one() volatile noexcept;
- void notify_one() noexcept;
- void notify_all() volatile noexcept;
- void notify_all() noexcept;
+ void wait(T, memory_order = memory_order::seq_cst) const volatile noexcept; // since C++20
+ void wait(T, memory_order = memory_order::seq_cst) const noexcept; // since C++20
+ void notify_one() volatile noexcept; // since C++20
+ void notify_one() noexcept; // since C++20
+ void notify_all() volatile noexcept; // since C++20
+ void notify_all() noexcept; // since C++20
};
template <>
@@ -184,12 +184,12 @@ struct atomic<integral>
integral operator^=(integral op) volatile noexcept;
integral operator^=(integral op) noexcept;
- void wait(integral, memory_order = memory_order::seq_cst) const volatile noexcept;
- void wait(integral, memory_order = memory_order::seq_cst) const noexcept;
- void notify_one() volatile noexcept;
- void notify_one() noexcept;
- void notify_all() volatile noexcept;
- void notify_all() noexcept;
+ void wait(integral, memory_order = memory_order::seq_cst) const volatile noexcept; // since C++20
+ void wait(integral, memory_order = memory_order::seq_cst) const noexcept; // since C++20
+ void notify_one() volatile noexcept; // since C++20
+ void notify_one() noexcept; // since C++20
+ void notify_all() volatile noexcept; // since C++20
+ void notify_all() noexcept; // since C++20
};
template <class T>
@@ -254,12 +254,12 @@ struct atomic<T*>
T* operator-=(ptrdiff_t op) volatile noexcept;
T* operator-=(ptrdiff_t op) noexcept;
- void wait(T*, memory_order = memory_order::seq_cst) const volatile noexcept;
- void wait(T*, memory_order = memory_order::seq_cst) const noexcept;
- void notify_one() volatile noexcept;
- void notify_one() noexcept;
- void notify_all() volatile noexcept;
- void notify_all() noexcept;
+ void wait(T*, memory_order = memory_order::seq_cst) const volatile noexcept; // since C++20
+ void wait(T*, memory_order = memory_order::seq_cst) const noexcept; // since C++20
+ void notify_one() volatile noexcept; // since C++20
+ void notify_one() noexcept; // since C++20
+ void notify_all() volatile noexcept; // since C++20
+ void notify_all() noexcept; // since C++20
};
template<>
@@ -321,12 +321,12 @@ struct atomic<floating-point-type> { // since C++20
floating-point-type operator-=(floating-point-type) volatile noexcept;
floating-point-type operator-=(floating-point-type) noexcept;
- void wait(floating-point-type, memory_order = memory_order::seq_cst) const volatile noexcept;
- void wait(floating-point-type, memory_order = memory_order::seq_cst) const noexcept;
- void notify_one() volatile noexcept;
- void notify_one() noexcept;
- void notify_all() volatile noexcept;
- void notify_all() noexcept;
+ void wait(floating-point-type, memory_order = memory_order::seq_cst) const volatile noexcept; // since C++20
+ void wait(floating-point-type, memory_order = memory_order::seq_cst) const noexcept; // since C++20
+ void notify_one() volatile noexcept; // since C++20
+ void notify_one() noexcept; // since C++20
+ void notify_all() volatile noexcept; // since C++20
+ void notify_all() noexcept; // since C++20
};
// [atomics.nonmembers], non-member functions
@@ -443,23 +443,23 @@ template<class T>
memory_order) noexcept;
template<class T>
- void atomic_wait(const volatile atomic<T>*, atomic<T>::value_type) noexcept;
+ void atomic_wait(const volatile atomic<T>*, atomic<T>::value_type) noexcept; // since C++20
template<class T>
- void atomic_wait(const atomic<T>*, atomic<T>::value_type) noexcept;
+ void atomic_wait(const atomic<T>*, atomic<T>::value_type) noexcept; // since C++20
template<class T>
- void atomic_wait_explicit(const volatile atomic<T>*, atomic<T>::value_type,
+ void atomic_wait_explicit(const volatile atomic<T>*, atomic<T>::value_type, // since C++20
memory_order) noexcept;
template<class T>
- void atomic_wait_explicit(const atomic<T>*, atomic<T>::value_type,
+ void atomic_wait_explicit(const atomic<T>*, atomic<T>::value_type, // since C++20
memory_order) noexcept;
template<class T>
- void atomic_notify_one(volatile atomic<T>*) noexcept;
+ void atomic_notify_one(volatile atomic<T>*) noexcept; // since C++20
template<class T>
- void atomic_notify_one(atomic<T>*) noexcept;
+ void atomic_notify_one(atomic<T>*) noexcept; // since C++20
template<class T>
- void atomic_notify_all(volatile atomic<T>*) noexcept;
+ void atomic_notify_all(volatile atomic<T>*) noexcept; // since C++20
template<class T>
- void atomic_notify_all(atomic<T>*) noexcept;
+ void atomic_notify_all(atomic<T>*) noexcept; // since C++20
// Atomics for standard typedef types
@@ -534,12 +534,12 @@ typedef struct atomic_flag
void clear(memory_order m = memory_order_seq_cst) volatile noexcept;
void clear(memory_order m = memory_order_seq_cst) noexcept;
- void wait(bool, memory_order = memory_order::seq_cst) const volatile noexcept;
- void wait(bool, memory_order = memory_order::seq_cst) const noexcept;
- void notify_one() volatile noexcept;
- void notify_one() noexcept;
- void notify_all() volatile noexcept;
- void notify_all() noexcept;
+ void wait(bool, memory_order = memory_order::seq_cst) const volatile noexcept; // since C++20
+ void wait(bool, memory_order = memory_order::seq_cst) const noexcept; // since C++20
+ void notify_one() volatile noexcept; // since C++20
+ void notify_one() noexcept; // since C++20
+ void notify_all() volatile noexcept; // since C++20
+ void notify_all() noexcept; // since C++20
} atomic_flag;
bool atomic_flag_test(volatile atomic_flag* obj) noexcept;
@@ -557,14 +557,14 @@ void atomic_flag_clear(atomic_flag* obj) noexcept;
void atomic_flag_clear_explicit(volatile atomic_flag* obj, memory_order m) noexcept;
void atomic_flag_clear_explicit(atomic_flag* obj, memory_order m) noexcept;
-void atomic_wait(const volatile atomic_flag* obj, T old) noexcept;
-void atomic_wait(const atomic_flag* obj, T old) noexcept;
-void atomic_wait_explicit(const volatile atomic_flag* obj, T old, memory_order m) noexcept;
-void atomic_wait_explicit(const atomic_flag* obj, T old, memory_order m) noexcept;
-void atomic_one(volatile atomic_flag* obj) noexcept;
-void atomic_one(atomic_flag* obj) noexcept;
-void atomic_all(volatile atomic_flag* obj) noexcept;
-void atomic_all(atomic_flag* obj) noexcept;
+void atomic_wait(const volatile atomic_flag* obj, T old) noexcept; // since C++20
+void atomic_wait(const atomic_flag* obj, T old) noexcept; // since C++20
+void atomic_wait_explicit(const volatile atomic_flag* obj, T old, memory_order m) noexcept; // since C++20
+void atomic_wait_explicit(const atomic_flag* obj, T old, memory_order m) noexcept; // since C++20
+void atomic_one(volatile atomic_flag* obj) noexcept; // since C++20
+void atomic_one(atomic_flag* obj) noexcept; // since C++20
+void atomic_all(volatile atomic_flag* obj) noexcept; // since C++20
+void atomic_all(atomic_flag* obj) noexcept; // since C++20
// fences
diff --git a/libcxx/include/barrier b/libcxx/include/barrier
index f91452c8d0064c..de94bd638a1f88 100644
--- a/libcxx/include/barrier
+++ b/libcxx/include/barrier
@@ -17,7 +17,7 @@ namespace std
{
template<class CompletionFunction = see below>
- class barrier
+ class barrier // since C++20
{
public:
using arrival_token = see below;
@@ -71,7 +71,7 @@ namespace std
_LIBCPP_PUSH_MACROS
#include <__undef_macros>
-#if _LIBCPP_STD_VER >= 14
+#if _LIBCPP_STD_VER >= 20
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -293,7 +293,7 @@ public:
_LIBCPP_END_NAMESPACE_STD
-#endif // _LIBCPP_STD_VER >= 14
+#endif // _LIBCPP_STD_VER >= 20
_LIBCPP_POP_MACROS
@@ -306,4 +306,4 @@ _LIBCPP_POP_MACROS
# include <variant>
#endif
-#endif //_LIBCPP_BARRIER
+#endif // !_LIBCPP_BARRIER
diff --git a/libcxx/include/latch b/libcxx/include/latch
index ad7b35579913b3..2c26de4eb92a53 100644
--- a/libcxx/include/latch
+++ b/libcxx/include/latch
@@ -16,7 +16,7 @@
namespace std
{
- class latch
+ class latch // since C++20
{
public:
static constexpr ptrdiff_t max() noexcept;
@@ -62,7 +62,7 @@ namespace std
_LIBCPP_PUSH_MACROS
#include <__undef_macros>
-#if _LIBCPP_STD_VER >= 14
+#if _LIBCPP_STD_VER >= 20
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -112,7 +112,7 @@ public:
_LIBCPP_END_NAMESPACE_STD
-#endif // _LIBCPP_STD_VER >= 14
+#endif // _LIBCPP_STD_VER >= 20
_LIBCPP_POP_MACROS
@@ -120,4 +120,4 @@ _LIBCPP_POP_MACROS
# include <atomic>
#endif
-#endif //_LIBCPP_LATCH
+#endif // !_LIBCPP_LATCH
diff --git a/libcxx/include/semaphore b/libcxx/include/semaphore
index 5235d720bf6fee..3c0e04e326af9a 100644
--- a/libcxx/include/semaphore
+++ b/libcxx/include/semaphore
@@ -16,7 +16,7 @@
namespace std {
template<ptrdiff_t least_max_value = implementation-defined>
-class counting_semaphore
+class counting_semaphore // since C++20
{
public:
static constexpr ptrdiff_t max() noexcept;
@@ -39,7 +39,7 @@ private:
ptrdiff_t counter; // exposition only
};
-using binary_semaphore = counting_semaphore<1>;
+using binary_semaphore = counting_semaphore<1>; // since C++20
}
@@ -72,7 +72,7 @@ using binary_semaphore = counting_semaphore<1>;
_LIBCPP_PUSH_MACROS
#include <__undef_macros>
-#if _LIBCPP_STD_VER >= 14
+#if _LIBCPP_STD_VER >= 20
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -186,7 +186,7 @@ using binary_semaphore = counting_semaphore<1>;
_LIBCPP_END_NAMESPACE_STD
-#endif // _LIBCPP_STD_VER >= 14
+#endif // _LIBCPP_STD_VER >= 20
_LIBCPP_POP_MACROS
@@ -194,4 +194,4 @@ _LIBCPP_POP_MACROS
# include <atomic>
#endif
-#endif //_LIBCPP_SEMAPHORE
+#endif // !_LIBCPP_SEMAPHORE
|
philnik777
approved these changes
Feb 16, 2024
|
+1, although I can only speak from the perspective of someone who is a bit familiar with this part of the code and doesn't know about the reasons it was backported for C++03. It also makes sense in light of the proposed additions to |
|
Do we really need to drop support for all versions prior to C++20? For example I've seen the If possible I would prefer the keep it in C++11 and if wanted use our 2 release deprecation. Note this is not an objection, we have done this in the past when extensions became a maintenance issue. If we want to do this we should create a separate patch to mention this in the LLVM-18 release, we still have time for that. |
ldionne
force-pushed
the
review/synchronization-library-cxx20-only
branch
from
05c86f8 to
280a780
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
I would definitely drop it in all versions before C++20. Since we're going through the trouble of this deprecation + removal, I'd remove it in all standard modes it shouldn't be in. Since LLVM 18 has shipped, what I suggest is to mark the library as deprecated < C++20 in LLVM 19, and to remove it in LLVM 20. I'll create another PR for the deprecation. |
ldionne
force-pushed
the
review/synchronization-library-cxx20-only
branch
from
280a780 to
ab2f186
Compare
When we initially implemented the C++20 synchronization library, we reluctantly accepted for the implementation to be backported to C++03 upon request from the person who provided the patch. This was when we were only starting to have experience with the issues this can create, so we flinched. Nowadays, we have a much stricter stance about not backporting features to previous standards. We have recently started fixing several bugs (and near bugs) in our implementation of the synchronization library. A recurring theme during these reviews has been how difficult to undertand the current code is, and upon inspection it becomes clear that being able to use a few recent C++ features (in particular lambdas) would help a great deal. The code would still be pretty intricate, but it would be a lot easier to reason about the flow of callbacks through things like __thread_poll_with_backoff. As a result, this patch drops support for the synchronization library before C++20. This makes us more strictly conforming and opens the door to major simplifications, in particular around atomic_wait which was supported all the way to C++03. This change will probably have some impact on downstream users, however since the C++20 synchronization library was added only in LLVM 10 (~3 years ago) and it's quite a niche feature, the set of people trying to use this part of the library before C++20 should be reasonably small.
ldionne
force-pushed
the
review/synchronization-library-cxx20-only
branch
from
ab2f186 to
438487e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we initially implemented the C++20 synchronization library, we reluctantly accepted for the implementation to be backported to C++03 upon request from the person who provided the patch. This was when we were only starting to have experience with the issues this can create, so we flinched. Nowadays, we have a much stricter stance about not backporting features to previous standards.
We have recently started fixing several bugs (and near bugs) in our implementation of the synchronization library. A recurring theme during these reviews has been how difficult to understand the current code is, and upon inspection it becomes clear that being able to use a few recent C++ features (in particular lambdas) would help a great deal. The code would still be pretty intricate, but it would be a lot easier to reason about the flow of callbacks through things like __thread_poll_with_backoff.
As a result, this patch drops support for the synchronization library before C++20. This makes us more strictly conforming and opens the door to major simplifications, in particular around atomic_wait which was supported all the way to C++03.
This change will probably have some impact on downstream users, however since the C++20 synchronization library was added only in LLVM 10 (~3 years ago) and it's quite a niche feature, the set of people trying to use this part of the library before C++20 should be reasonably small.