Skip to content

Conversation

MitalAshok
Copy link
Contributor

Implements P0718R2.

This is quite a "dumb" implementation: It just reuses the now-deprecated functions in [depr.util.smartptr.shared.atomic] on a held std::{shared/weak}_ptr<T> member.

P1644R0 (wait/notify for atomic<shared_ptr<T>>) is not implemented. I will work on a subsequent patch, but we need atomic<shared_ptr> first.

I also noticed LWG3661 (constinit atomic<shared_ptr<T>> a(nullptr);) did not apply to atomic<weak_ptr<T>>, but I see no reason not to support it.

I also added forward declarations for the partial specializations in <atomic>. This shouldn't change the correctness of any code but should give a better error (that atomic<shared_ptr<T>> is incomplete, not that shared_ptr<T> is a non-trivial type in the primary template)

Also implemented LWG3661, LWG3893
@MitalAshok MitalAshok requested a review from a team as a code owner January 16, 2024 17:37
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2024

@llvm/pr-subscribers-libcxx

Author: Mital Ashok (MitalAshok)

Changes

Implements P0718R2.

This is quite a "dumb" implementation: It just reuses the now-deprecated functions in [depr.util.smartptr.shared.atomic] on a held std::{shared/weak}_ptr&lt;T&gt; member.

P1644R0 (wait/notify for atomic&lt;shared_ptr&lt;T&gt;&gt;) is not implemented. I will work on a subsequent patch, but we need atomic&lt;shared_ptr&gt; first.

I also noticed LWG3661 (constinit atomic&lt;shared_ptr&lt;T&gt;&gt; a(nullptr);) did not apply to atomic&lt;weak_ptr&lt;T&gt;&gt;, but I see no reason not to support it.

I also added forward declarations for the partial specializations in &lt;atomic&gt;. This shouldn't change the correctness of any code but should give a better error (that atomic&lt;shared_ptr&lt;T&gt;&gt; is incomplete, not that shared_ptr&lt;T&gt; is a non-trivial type in the primary template)


Patch is 33.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78317.diff

17 Files Affected:

  • (modified) libcxx/docs/FeatureTestMacroTable.rst (+1-1)
  • (modified) libcxx/docs/ReleaseNotes/18.rst (+1)
  • (modified) libcxx/docs/Status/Cxx20Papers.csv (+1-1)
  • (modified) libcxx/docs/Status/Cxx23Issues.csv (+1-1)
  • (modified) libcxx/docs/Status/Cxx2cIssues.csv (+1-1)
  • (modified) libcxx/include/CMakeLists.txt (+1)
  • (modified) libcxx/include/__atomic/atomic.h (+10)
  • (added) libcxx/include/__memory/atomic_shared_ptr.h (+233)
  • (modified) libcxx/include/__memory/shared_ptr.h (-104)
  • (modified) libcxx/include/memory (+83-10)
  • (modified) libcxx/include/module.modulemap.in (+1)
  • (modified) libcxx/include/version (+1-1)
  • (modified) libcxx/modules/std/memory.inc (+3)
  • (modified) libcxx/test/std/atomics/types.pass.cpp (+1-2)
  • (modified) libcxx/test/std/language.support/support.limits/support.limits.general/atomic.version.compile.pass.cpp (+15-33)
  • (modified) libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp (+15-33)
  • (modified) libcxx/utils/generate_feature_test_macro_components.py (-1)
diff --git a/libcxx/docs/FeatureTestMacroTable.rst b/libcxx/docs/FeatureTestMacroTable.rst
index 893a3b13ca06e02..ea411ee8d672776 100644
--- a/libcxx/docs/FeatureTestMacroTable.rst
+++ b/libcxx/docs/FeatureTestMacroTable.rst
@@ -180,7 +180,7 @@ Status
     --------------------------------------------------- -----------------
     ``__cpp_lib_atomic_ref``                            *unimplemented*
     --------------------------------------------------- -----------------
-    ``__cpp_lib_atomic_shared_ptr``                     *unimplemented*
+    ``__cpp_lib_atomic_shared_ptr``                     ``201711L``
     --------------------------------------------------- -----------------
     ``__cpp_lib_atomic_value_initialization``           ``201911L``
     --------------------------------------------------- -----------------
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 62a1fec627d0ca7..c6efadd4ba1e204 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -60,6 +60,7 @@ Implemented Papers
 - P0521R0 - Proposed Resolution for CA 14 (``shared_ptr`` ``use_count/unique``)
 - P1759R6 - Native handles and file streams
 - P2517R1 - Add a conditional ``noexcept`` specification to ``std::apply``
+- P0718R2 -  Revising ``atomic_shared_ptr`` for C++20
 
 
 Improvements and New Features
diff --git a/libcxx/docs/Status/Cxx20Papers.csv b/libcxx/docs/Status/Cxx20Papers.csv
index d73088687975c27..eae002a35d33454 100644
--- a/libcxx/docs/Status/Cxx20Papers.csv
+++ b/libcxx/docs/Status/Cxx20Papers.csv
@@ -12,7 +12,7 @@
 "`P0600R1 <https://wg21.link/P0600R1>`__","LWG","nodiscard in the Library","Albuquerque","|Complete|","16.0"
 "`P0616R0 <https://wg21.link/P0616R0>`__","LWG","de-pessimize legacy <numeric> algorithms with std::move","Albuquerque","|Complete|","12.0"
 "`P0653R2 <https://wg21.link/P0653R2>`__","LWG","Utility to convert a pointer to a raw pointer","Albuquerque","|Complete|","6.0"
-"`P0718R2 <https://wg21.link/P0718R2>`__","LWG","Atomic shared_ptr","Albuquerque","",""
+"`P0718R2 <https://wg21.link/P0718R2>`__","LWG","Atomic shared_ptr","Albuquerque","|Complete|","18.0"
 "`P0767R1 <https://wg21.link/P0767R1>`__","CWG","Deprecate POD","Albuquerque","|Complete|","7.0"
 "`P0768R1 <https://wg21.link/P0768R1>`__","CWG","Library Support for the Spaceship (Comparison) Operator","Albuquerque","|Complete|",""
 "`P0777R1 <https://wg21.link/P0777R1>`__","LWG","Treating Unnecessary ``decay``\ ","Albuquerque","|Complete|","7.0"
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index b24ecc5525a1497..597861409039181 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -157,7 +157,7 @@
 "`3654 <https://wg21.link/LWG3654>`__","``basic_format_context::arg(size_t)`` should be ``noexcept`` ","February 2022","|Complete|","15.0","|format|"
 "`3657 <https://wg21.link/LWG3657>`__","``std::hash<std::filesystem::path>`` is not enabled","February 2022","|Complete|","17.0"
 "`3660 <https://wg21.link/LWG3660>`__","``iterator_traits<common_iterator>::pointer`` should conform to §[iterator.traits]","February 2022","|Complete|","14.0","|ranges|"
-"`3661 <https://wg21.link/LWG3661>`__","``constinit atomic<shared_ptr<T>> a(nullptr);`` should work","February 2022","",""
+"`3661 <https://wg21.link/LWG3661>`__","``constinit atomic<shared_ptr<T>> a(nullptr);`` should work","February 2022","|Complete|","18.0"
 "","","","","",""
 "`3564 <https://wg21.link/LWG3564>`__","``transform_view::iterator<true>::value_type`` and ``iterator_category`` should use ``const F&``","July 2022","","","|ranges|"
 "`3617 <https://wg21.link/LWG3617>`__","``function``/``packaged_task`` deduction guides and deducing ``this``","July 2022","",""
diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index fe0f13f6e8cb2c2..88a08b01d4982ea 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -3,7 +3,7 @@
 "`3884 <https://wg21.link/LWG3884>`__","``flat_foo`` is missing allocator-extended copy/move constructors","Varna June 2023","","","|flat_containers|"
 "`3885 <https://wg21.link/LWG3885>`__","``op`` should be in [zombie.names]","Varna June 2023","|Nothing To Do|","",""
 "`3887 <https://wg21.link/LWG3887>`__","Version macro for ``allocate_at_least``","Varna June 2023","","",""
-"`3893 <https://wg21.link/LWG3893>`__","LWG 3661 broke ``atomic<shared_ptr<T>> a; a = nullptr;``","Varna June 2023","","",""
+"`3893 <https://wg21.link/LWG3893>`__","LWG 3661 broke ``atomic<shared_ptr<T>> a; a = nullptr;``","Varna June 2023","|Complete|","18.0",""
 "`3894 <https://wg21.link/LWG3894>`__","``generator::promise_type::yield_value(ranges::elements_of<Rng, Alloc>)`` should not be ``noexcept``","Varna June 2023","","",""
 "`3903 <https://wg21.link/LWG3903>`__","span destructor is redundantly noexcept","Varna June 2023","|Complete|","7.0",""
 "`3904 <https://wg21.link/LWG3904>`__","``lazy_split_view::outer-iterator``'s const-converting constructor isn't setting ``trailing_empty_``","Varna June 2023","","","|ranges|"
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 0fe3ab44d2466e9..0fd51788c757148 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -526,6 +526,7 @@ set(files
   __memory/allocator_destructor.h
   __memory/allocator_traits.h
   __memory/assume_aligned.h
+  __memory/atomic_shared_ptr.h
   __memory/auto_ptr.h
   __memory/builtin_new_allocator.h
   __memory/compressed_pair.h
diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index 3dfb6937d0325e7..da12b36568fb3be 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -249,6 +249,16 @@ struct atomic<_Tp> : __atomic_base<_Tp> {
   _LIBCPP_HIDE_FROM_ABI _Tp operator-=(_Tp __op) noexcept { return fetch_sub(__op) - __op; }
 };
 
+template <class _Tp>
+class shared_ptr;
+template <class _Tp>
+class weak_ptr;
+
+template <class _Tp>
+struct atomic<shared_ptr<_Tp>>;
+template <class _Tp>
+struct atomic<weak_ptr<_Tp>>;
+
 #endif // _LIBCPP_STD_VER >= 20
 
 // atomic_is_lock_free
diff --git a/libcxx/include/__memory/atomic_shared_ptr.h b/libcxx/include/__memory/atomic_shared_ptr.h
new file mode 100644
index 000000000000000..f3fa2260ac91049
--- /dev/null
+++ b/libcxx/include/__memory/atomic_shared_ptr.h
@@ -0,0 +1,233 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___MEMORY_ATOMIC_SHARED_PTR_H
+#define _LIBCPP___MEMORY_ATOMIC_SHARED_PTR_H
+
+#include <__memory/addressof.h>
+#include <__memory/shared_ptr.h>
+#include <cstddef>
+#if !defined(_LIBCPP_HAS_NO_ATOMIC_HEADER)
+#  include <__atomic/memory_order.h>
+#endif
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if !defined(_LIBCPP_HAS_NO_THREADS)
+
+class _LIBCPP_EXPORTED_FROM_ABI __sp_mut {
+  void* __lx_;
+
+public:
+  void lock() _NOEXCEPT;
+  void unlock() _NOEXCEPT;
+
+private:
+  _LIBCPP_CONSTEXPR __sp_mut(void*) _NOEXCEPT;
+  __sp_mut(const __sp_mut&);
+  __sp_mut& operator=(const __sp_mut&);
+
+  friend _LIBCPP_EXPORTED_FROM_ABI __sp_mut& __get_sp_mut(const void*);
+};
+
+_LIBCPP_EXPORTED_FROM_ABI __sp_mut& __get_sp_mut(const void*);
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _Tp __sp_atomic_load(const _Tp* __p) {
+  __sp_mut& __m = std::__get_sp_mut(__p);
+  __m.lock();
+  _Tp __q = *__p;
+  __m.unlock();
+  return __q;
+}
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI void __sp_atomic_store(_Tp* __p, _Tp& __r) {
+  __sp_mut& __m = std::__get_sp_mut(__p);
+  __m.lock();
+  __p->swap(__r);
+  __m.unlock();
+}
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI bool __sp_atomic_compare_exchange_strong(_Tp* __p, _Tp* __v, _Tp& __w) {
+  _Tp __temp;
+  __sp_mut& __m = std::__get_sp_mut(__p);
+  __m.lock();
+  if (__p->__owner_equivalent(*__v)) {
+    std::swap(__temp, *__p);
+    *__p = __w;
+    __m.unlock();
+    return true;
+  }
+  std::swap(__temp, *__v);
+  *__v = *__p;
+  __m.unlock();
+  return false;
+}
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _Tp __sp_atomic_exchange(_Tp* __p, _Tp& __r) {
+  __sp_mut& __m = std::__get_sp_mut(__p);
+  __m.lock();
+  __p->swap(__r);
+  __m.unlock();
+  return __r;
+}
+
+#  if _LIBCPP_STD_VER >= 20
+template <class _Tp>
+struct atomic;
+
+template <class _Tp>
+struct __sp_atomic_base {
+  using value_type = _Tp;
+
+  static constexpr bool is_always_lock_free = false;
+  _LIBCPP_HIDE_FROM_ABI bool is_lock_free() const noexcept { return false; }
+
+  _LIBCPP_HIDE_FROM_ABI constexpr __sp_atomic_base() noexcept = default;
+  _LIBCPP_HIDE_FROM_ABI __sp_atomic_base(_Tp&& __d) noexcept : __p(std::move(__d)) {}
+  _LIBCPP_HIDE_FROM_ABI __sp_atomic_base(const __sp_atomic_base&) = delete;
+  _LIBCPP_HIDE_FROM_ABI void operator=(const __sp_atomic_base&)   = delete;
+
+  _LIBCPP_HIDE_FROM_ABI _Tp load(memory_order = memory_order_seq_cst) noexcept {
+    return std::__sp_atomic_load(std::addressof(__p));
+  }
+  _LIBCPP_HIDE_FROM_ABI operator _Tp() const noexcept { return load(); }
+  _LIBCPP_HIDE_FROM_ABI void store(_Tp __d, memory_order = memory_order_seq_cst) noexcept {
+    std::__sp_atomic_store(std::addressof(__p), __d);
+  }
+  _LIBCPP_HIDE_FROM_ABI void operator=(_Tp __d) noexcept { std::__sp_atomic_store(std::addressof(__p), __d); }
+  _LIBCPP_HIDE_FROM_ABI void operator=(nullptr_t) noexcept { store(nullptr); }
+
+  _LIBCPP_HIDE_FROM_ABI _Tp exchange(_Tp __d, memory_order = memory_order_seq_cst) noexcept {
+    return std::__sp_atomic_exchange(std::addressof(__p), __d);
+  }
+  _LIBCPP_HIDE_FROM_ABI bool compare_exchange_weak(_Tp& __e, _Tp __d, memory_order, memory_order) noexcept {
+    return std::__sp_atomic_compare_exchange_strong(std::addressof(__p), std::addressof(__e), __d);
+  }
+  _LIBCPP_HIDE_FROM_ABI bool compare_exchange_strong(_Tp& __e, _Tp __d, memory_order, memory_order) noexcept {
+    return std::__sp_atomic_compare_exchange_strong(std::addressof(__p), std::addressof(__e), __d);
+  }
+  _LIBCPP_HIDE_FROM_ABI bool compare_exchange_weak(_Tp& __e, _Tp __d, memory_order = memory_order_seq_cst) noexcept {
+    return std::__sp_atomic_compare_exchange_strong(std::addressof(__p), std::addressof(__e), __d);
+  }
+  _LIBCPP_HIDE_FROM_ABI bool compare_exchange_strong(_Tp& __e, _Tp __d, memory_order = memory_order_seq_cst) noexcept {
+    return std::__sp_atomic_compare_exchange_strong(std::addressof(__p), std::addressof(__e), __d);
+  }
+
+  // P1644R0 not implemented
+  // void wait(_Tp old, memory_order order = memory_order::seq_cst) const noexcept;
+  // void notify_one() noexcept;
+  // void notify_all() noexcept;
+
+private:
+  _Tp __p;
+};
+
+template <class _Tp>
+struct atomic<shared_ptr<_Tp>> : __sp_atomic_base<shared_ptr<_Tp>> {
+  _LIBCPP_HIDE_FROM_ABI constexpr atomic() noexcept = default;
+  _LIBCPP_HIDE_FROM_ABI constexpr atomic(nullptr_t) noexcept : atomic() {}
+  _LIBCPP_HIDE_FROM_ABI atomic(shared_ptr<_Tp> desired) noexcept
+      : __sp_atomic_base<shared_ptr<_Tp>>(std::move(desired)) {}
+  _LIBCPP_HIDE_FROM_ABI atomic(const atomic&) = delete;
+
+  _LIBCPP_HIDE_FROM_ABI void operator=(const atomic&) = delete;
+  using __sp_atomic_base<shared_ptr<_Tp>>::operator=;
+};
+
+template <class _Tp>
+struct atomic<weak_ptr<_Tp>> : __sp_atomic_base<weak_ptr<_Tp>> {
+  _LIBCPP_HIDE_FROM_ABI constexpr atomic() noexcept = default;
+  _LIBCPP_HIDE_FROM_ABI constexpr atomic(nullptr_t) noexcept : atomic() {}
+  _LIBCPP_HIDE_FROM_ABI atomic(weak_ptr<_Tp> desired) noexcept : __sp_atomic_base<weak_ptr<_Tp>>(std::move(desired)) {}
+  _LIBCPP_HIDE_FROM_ABI atomic(const atomic&) = delete;
+
+  _LIBCPP_HIDE_FROM_ABI void operator=(const atomic&) = delete;
+  using __sp_atomic_base<weak_ptr<_Tp>>::operator=;
+};
+#  endif // _LIBCPP_STD_VER >= 20
+
+// [depr.util.smartptr.shared.atomic]
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI bool atomic_is_lock_free(const shared_ptr<_Tp>*) {
+  return false;
+}
+
+template <class _Tp>
+_LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+atomic_load(const shared_ptr<_Tp>* __p) {
+  return std::__sp_atomic_load(__p);
+}
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+atomic_load_explicit(const shared_ptr<_Tp>* __p, memory_order) {
+  return std::atomic_load(__p);
+}
+
+template <class _Tp>
+_LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI void atomic_store(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r) {
+  std::__sp_atomic_store(__p, __r);
+}
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI void
+atomic_store_explicit(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r, memory_order) {
+  std::atomic_store(__p, __r);
+}
+
+template <class _Tp>
+_LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+atomic_exchange(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r) {
+  return std::__sp_atomic_exchange(__p, __r);
+}
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+atomic_exchange_explicit(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r, memory_order) {
+  return std::atomic_exchange(__p, std::move(__r));
+}
+
+template <class _Tp>
+_LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI bool
+atomic_compare_exchange_strong(shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w) {
+  return std::__sp_atomic_compare_exchange_strong(__p, __v, __w);
+}
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI bool
+atomic_compare_exchange_weak(shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w) {
+  return std::atomic_compare_exchange_strong(__p, __v, std::move(__w));
+}
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI bool atomic_compare_exchange_strong_explicit(
+    shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w, memory_order, memory_order) {
+  return std::atomic_compare_exchange_strong(__p, __v, std::move(__w));
+}
+
+template <class _Tp>
+inline _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_HIDE_FROM_ABI bool atomic_compare_exchange_weak_explicit(
+    shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w, memory_order, memory_order) {
+  return std::atomic_compare_exchange_weak(__p, __v, std::move(__w));
+}
+
+#endif // !defined(_LIBCPP_HAS_NO_THREADS)
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // LLVM_ATOMIC_ATOMIC_SHARED_PTR_H
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 9a73d439306d9e0..c2a6953382dcd32 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -1556,110 +1556,6 @@ template <class _CharT, class _Traits, class _Yp>
 inline _LIBCPP_HIDE_FROM_ABI basic_ostream<_CharT, _Traits>&
 operator<<(basic_ostream<_CharT, _Traits>& __os, shared_ptr<_Yp> const& __p);
 
-#if !defined(_LIBCPP_HAS_NO_THREADS)
-
-class _LIBCPP_EXPORTED_FROM_ABI __sp_mut {
-  void* __lx_;
-
-public:
-  void lock() _NOEXCEPT;
-  void unlock() _NOEXCEPT;
-
-private:
-  _LIBCPP_CONSTEXPR __sp_mut(void*) _NOEXCEPT;
-  __sp_mut(const __sp_mut&);
-  __sp_mut& operator=(const __sp_mut&);
-
-  friend _LIBCPP_EXPORTED_FROM_ABI __sp_mut& __get_sp_mut(const void*);
-};
-
-_LIBCPP_EXPORTED_FROM_ABI __sp_mut& __get_sp_mut(const void*);
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI bool atomic_is_lock_free(const shared_ptr<_Tp>*) {
-  return false;
-}
-
-template <class _Tp>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> atomic_load(const shared_ptr<_Tp>* __p) {
-  __sp_mut& __m = std::__get_sp_mut(__p);
-  __m.lock();
-  shared_ptr<_Tp> __q = *__p;
-  __m.unlock();
-  return __q;
-}
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> atomic_load_explicit(const shared_ptr<_Tp>* __p, memory_order) {
-  return std::atomic_load(__p);
-}
-
-template <class _Tp>
-_LIBCPP_HIDE_FROM_ABI void atomic_store(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r) {
-  __sp_mut& __m = std::__get_sp_mut(__p);
-  __m.lock();
-  __p->swap(__r);
-  __m.unlock();
-}
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI void atomic_store_explicit(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r, memory_order) {
-  std::atomic_store(__p, __r);
-}
-
-template <class _Tp>
-_LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> atomic_exchange(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r) {
-  __sp_mut& __m = std::__get_sp_mut(__p);
-  __m.lock();
-  __p->swap(__r);
-  __m.unlock();
-  return __r;
-}
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
-atomic_exchange_explicit(shared_ptr<_Tp>* __p, shared_ptr<_Tp> __r, memory_order) {
-  return std::atomic_exchange(__p, __r);
-}
-
-template <class _Tp>
-_LIBCPP_HIDE_FROM_ABI bool
-atomic_compare_exchange_strong(shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w) {
-  shared_ptr<_Tp> __temp;
-  __sp_mut& __m = std::__get_sp_mut(__p);
-  __m.lock();
-  if (__p->__owner_equivalent(*__v)) {
-    std::swap(__temp, *__p);
-    *__p = __w;
-    __m.unlock();
-    return true;
-  }
-  std::swap(__temp, *__v);
-  *__v = *__p;
-  __m.unlock();
-  return false;
-}
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI bool
-atomic_compare_exchange_weak(shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w) {
-  return std::atomic_compare_exchange_strong(__p, __v, __w);
-}
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI bool atomic_compare_exchange_strong_explicit(
-    shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w, memory_order, memory_order) {
-  return std::atomic_compare_exchange_strong(__p, __v, __w);
-}
-
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI bool atomic_compare_exchange_weak_explicit(
-    shared_ptr<_Tp>* __p, shared_ptr<_Tp>* __v, shared_ptr<_Tp> __w, memory_order, memory_order) {
-  return std::atomic_compare_exchange_weak(__p, __v, __w);
-}
-
-#endif // !defined(_LIBCPP_HAS_NO_THREADS)
-
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___MEMORY_SHARED_PTR_H
diff --git a/libcxx/include/memory b/libcxx/include/memory
index ee245d5fd2dcb2e..6c0ec59119f6d20 100644
--- a/libcxx/include/memory
+++ b/libcxx/include/memory
@@ -831,29 +831,29 @@ public:
 };
 
 template<class T>
-    bool atomic_is_lock_free(const shared_ptr<T>* p);
+    bool atomic_is_lock_free(const shared_ptr<T>* p);                                      // Deprecated in C++20
 template<class T>
-    shared_ptr<T> atomic_load(const shared_ptr<T>* p);
+    shared_ptr<T> atomic_load(const shared_ptr<T>* p);                                     // Deprecated in C++20
 template<class T>
-    shared_ptr<T> atomic_load_explicit(const shared_ptr<T>* p, memory_order mo);
+    shared_ptr<T> atomic_load_explicit(const shared_ptr<T>* p, memory_order mo);           // Deprecated in C++20
 template<class T>
-    void atomic_store(shared_ptr<T>* p, shared_ptr<T> r);
+    void atomic_store(shared_ptr<T>* p, shared_ptr<T> r);                                  // Deprecated in C++20
 template<class T>
-    void atomic_store_explicit(shared_ptr<T>* p, shared_ptr<T> r, memory_order mo);
+    void atomic_store_explicit(shared_ptr<T>* p, shared_ptr<T> r, memory_order mo);        // Deprecated in C++20
 template<class T>
-    shared_ptr<T> atomic_exchange(shared_ptr<T>* p, shared_ptr<T> r);
+    shared_ptr<T> atomic_exchange(shared_ptr<T>* p, shared_ptr<T> r);                      // Deprecated in C++20
 template<class T>
     shared_ptr<T>
-    atomic_exchange_explicit(shared_ptr<T>* p, shared_ptr<T> r, memory_order mo);
+    atomic_exchange_explicit(shared_ptr<T>* p, shared_ptr<T> r, memory_order mo);          // Deprecated in C++20
 template<class T>
     bool
-    atomic_compare_exchange_weak(shared_ptr<T>* p, shared_ptr<T>* v, shared_ptr<T> w);
+    atomic_compare_exchange_weak(shared_ptr<T>* p, shared_ptr<T>* v, shared_ptr<T> w);     // Deprecated in C++20
 template<class T>
     bool
-    atomic_compare_exchange_strong( shared_ptr<T>* p, shared_ptr<T>* v, shared_ptr<T> w);
+    atomic_compare_exchange_strong( shared_ptr<T>* p, shared_ptr<T>* v, shared_ptr<T> w);  ...
[truncated]

@mordante
Copy link
Member

Thanks for your contribution!

Implements P0718R2.

This is quite a "dumb" implementation: It just reuses the now-deprecated functions in [depr.util.smartptr.shared.atomic] on a held std::{shared/weak}_ptr<T> member.

Can you provide information why using deprecated features is the way to go? It seems likely these functions will be removed in C++26. Per P2863R3 Review Annex D for C++26 (Alisdair Meredith)
"
6.24 Deprecated shared_ptr atomic access [depr.util.smartptr.shared.atomic]
The legacy C-style atomic API for manipulating shared pointers provided in C++11 is subtle, frequently misunderstood, and easily misused to cause data races. A type-safe replacement facility that also provides support for atomic<weak_ptr> was added to C++20, so we recommend removing the legacy API at the earliest opportunity.

See paper [P2869] for a more detailed discussion and proposed wording.
6.24.1 SG1 (concurrency) Review, 2023 June, Varna

Polled after discussion of the header compatibility concern:

Poll: Remove deprecated shared_ptr atomic access APIs from C++26, with any of the library options listed in P2689?

SF F N A SA
2 4 1 1 0

Consensus to move this paper to LEWG to resolve the header design issue, and then continue on to LWG.
"

Copy link
Member

@huixie90 huixie90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say we should take the opportunity to try implementing it in a lock-free way

Comment on lines +130 to +133
// P1644R0 not implemented
// void wait(_Tp old, memory_order order = memory_order::seq_cst) const noexcept;
// void notify_one() noexcept;
// void notify_all() noexcept;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these implementable given that you are just storing a shared_ptr ?

@ldionne
Copy link
Member

ldionne commented Jan 26, 2024

I would be OK with using this implementation strategy as a stepping stone for a lockfree implementation, which would require making this experimental before we commit to an ABI. I agree with @huixie90 that we need to implement this in a lockfree way, and my understanding is that this requires more research cause there is no efficient and publicly available implementation that works on all platforms.

@ldionne
Copy link
Member

ldionne commented Jan 26, 2024

@timuraudio Do you have thoughts about how we could implement this lockfree?

template <class _Tp>
struct atomic<weak_ptr<_Tp>> : __sp_atomic_base<weak_ptr<_Tp>> {
_LIBCPP_HIDE_FROM_ABI constexpr atomic() noexcept = default;
_LIBCPP_HIDE_FROM_ABI constexpr atomic(nullptr_t) noexcept : atomic() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason was said in LWG3611:

There is no need to also change atomic<weak_ptr<T>> because weak_ptr doesn't have a constructor taking nullptr.

Did you submit an LWG issue for corresponding change of atomic<weak_ptr<T>>?

_LIBCPP_HIDE_FROM_ABI atomic(weak_ptr<_Tp> desired) noexcept : __sp_atomic_base<weak_ptr<_Tp>>(std::move(desired)) {}
_LIBCPP_HIDE_FROM_ABI atomic(const atomic&) = delete;

_LIBCPP_HIDE_FROM_ABI void operator=(const atomic&) = delete;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we apply the resolution of LWG3611 to atomic<weak_ptr<T>>, perhaps we should also apply that of LWG3893.

@frederick-vs-ja

This comment was marked as resolved.

@MitalAshok MitalAshok marked this pull request as draft April 26, 2024 13:27
@Zingam
Copy link
Contributor

Zingam commented May 22, 2024

@MitalAshok Is there any progress on this?

@MitalAshok
Copy link
Contributor Author

@Zingam It appears that a completely different implementation needs to be used for this to be able to efficiently implement wait/notify_all. I'm not planning on continuing with this approach, no

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants