[libc++] Extract destroy algorithms into separate headers #126449

ldionne · 2025-02-10T00:55:43Z

This patch separates the destroy algorithms from the primitive construct_at and destroy_at operations, which are conceptually not algorithms. This makes it easier to start using these destroy algorithms from upcoming relocation facilities.

As part of this, it also implements std::destroy_at for arrays without relying on the std::destroy algorithm, which is conceptually a higher-level facility.

llvmbot · 2025-02-10T00:56:13Z

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This patch separates the destroy algorithms from the primitive construct_at and destroy_at operations, which are conceptually not algorithms. This makes it easier to start using these destroy algorithms from upcoming relocation facilities.

Full diff: https://github.com/llvm/llvm-project/pull/126449.diff

10 Files Affected:

(modified) libcxx/include/CMakeLists.txt (+2)
(modified) libcxx/include/__format/buffer.h (+3-4)
(modified) libcxx/include/__memory/construct_at.h (+3-34)
(added) libcxx/include/__memory/destroy.h (+70)
(modified) libcxx/include/__memory/ranges_construct_at.h (-35)
(added) libcxx/include/__memory/ranges_destroy.h (+77)
(modified) libcxx/include/__memory/shared_ptr.h (+1)
(modified) libcxx/include/__memory/uninitialized_algorithms.h (+1-8)
(modified) libcxx/include/__pstl/backends/libdispatch.h (+1)
(modified) libcxx/include/module.modulemap (+2)

diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index ce805b4eb7b8b4f..255a0474c0f6bf3 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -557,12 +557,14 @@ set(files
   __memory/compressed_pair.h
   __memory/concepts.h
   __memory/construct_at.h
+  __memory/destroy.h
   __memory/destruct_n.h
   __memory/inout_ptr.h
   __memory/noexcept_move_assign_container.h
   __memory/out_ptr.h
   __memory/pointer_traits.h
   __memory/ranges_construct_at.h
+  __memory/ranges_destroy.h
   __memory/ranges_uninitialized_algorithms.h
   __memory/raw_storage_iterator.h
   __memory/shared_count.h
diff --git a/libcxx/include/__format/buffer.h b/libcxx/include/__format/buffer.h
index 0c054bbc3a1d859..c88b7f322201095 100644
--- a/libcxx/include/__format/buffer.h
+++ b/libcxx/include/__format/buffer.h
@@ -15,7 +15,6 @@
 #include <__algorithm/max.h>
 #include <__algorithm/min.h>
 #include <__algorithm/ranges_copy.h>
-#include <__algorithm/ranges_copy_n.h>
 #include <__algorithm/transform.h>
 #include <__algorithm/unwrap_iter.h>
 #include <__concepts/same_as.h>
@@ -33,7 +32,7 @@
 #include <__memory/allocator.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/construct_at.h>
-#include <__memory/ranges_construct_at.h>
+#include <__memory/destroy.h>
 #include <__memory/uninitialized_algorithms.h>
 #include <__type_traits/add_pointer.h>
 #include <__type_traits/conditional.h>
@@ -621,7 +620,7 @@ class _LIBCPP_TEMPLATE_VIS __retarget_buffer {
   }
 
   _LIBCPP_HIDE_FROM_ABI ~__retarget_buffer() {
-    ranges::destroy_n(__ptr_, __size_);
+    std::destroy_n(__ptr_, __size_);
     allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, __capacity_);
   }
 
@@ -686,7 +685,7 @@ class _LIBCPP_TEMPLATE_VIS __retarget_buffer {
     // guard is optimized away so there is no runtime overhead.
     std::uninitialized_move_n(__ptr_, __size_, __result.ptr);
     __guard.__complete();
-    ranges::destroy_n(__ptr_, __size_);
+    std::destroy_n(__ptr_, __size_);
     allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, __capacity_);
 
     __ptr_      = __result.ptr;
diff --git a/libcxx/include/__memory/construct_at.h b/libcxx/include/__memory/construct_at.h
index 1f129d17970b19c..5f389a479cceff9 100644
--- a/libcxx/include/__memory/construct_at.h
+++ b/libcxx/include/__memory/construct_at.h
@@ -57,9 +57,6 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp* __construct_at(_Tp* __l
 // The internal functions are available regardless of the language version (with the exception of the `__destroy_at`
 // taking an array).
 
-template <class _ForwardIterator>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator __destroy(_ForwardIterator, _ForwardIterator);
-
 template <class _Tp, __enable_if_t<!is_array<_Tp>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy_at(_Tp* __loc) {
   _LIBCPP_ASSERT_NON_NULL(__loc != nullptr, "null pointer given to destroy_at");
@@ -70,28 +67,12 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy_at(_Tp* __loc
 template <class _Tp, __enable_if_t<is_array<_Tp>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy_at(_Tp* __loc) {
   _LIBCPP_ASSERT_NON_NULL(__loc != nullptr, "null pointer given to destroy_at");
-  std::__destroy(std::begin(*__loc), std::end(*__loc));
+  auto const __end = std::end(*__loc);
+  for (auto __it = std::begin(*__loc); __it != __end; ++__it)
+    std::__destroy_at(*__it);
 }
 #endif
 
-template <class _ForwardIterator>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator
-__destroy(_ForwardIterator __first, _ForwardIterator __last) {
-  for (; __first != __last; ++__first)
-    std::__destroy_at(std::addressof(*__first));
-  return __first;
-}
-
-template <class _BidirectionalIterator>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _BidirectionalIterator
-__reverse_destroy(_BidirectionalIterator __first, _BidirectionalIterator __last) {
-  while (__last != __first) {
-    --__last;
-    std::__destroy_at(std::addressof(*__last));
-  }
-  return __last;
-}
-
 #if _LIBCPP_STD_VER >= 17
 
 template <class _Tp, enable_if_t<!is_array_v<_Tp>, int> = 0>
@@ -106,18 +87,6 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void destroy_at(_Tp* __loc)
 }
 #  endif
 
-template <class _ForwardIterator>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void destroy(_ForwardIterator __first, _ForwardIterator __last) {
-  (void)std::__destroy(std::move(__first), std::move(__last));
-}
-
-template <class _ForwardIterator, class _Size>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator destroy_n(_ForwardIterator __first, _Size __n) {
-  for (; __n > 0; (void)++__first, --__n)
-    std::__destroy_at(std::addressof(*__first));
-  return __first;
-}
-
 #endif // _LIBCPP_STD_VER >= 17
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__memory/destroy.h b/libcxx/include/__memory/destroy.h
new file mode 100644
index 000000000000000..48d895827553b6b
--- /dev/null
+++ b/libcxx/include/__memory/destroy.h
@@ -0,0 +1,70 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_DESTROY_H
+#define _LIBCPP___MEMORY_DESTROY_H
+
+#include <__config>
+#include <__memory/addressof.h>
+#include <__memory/allocator_traits.h>
+#include <__memory/construct_at.h>
+#include <__memory/pointer_traits.h>
+#include <__utility/move.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class _ForwardIterator>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator
+__destroy(_ForwardIterator __first, _ForwardIterator __last) {
+  for (; __first != __last; ++__first)
+    std::__destroy_at(std::addressof(*__first));
+  return __first;
+}
+
+template <class _BidirectionalIterator>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _BidirectionalIterator
+__reverse_destroy(_BidirectionalIterator __first, _BidirectionalIterator __last) {
+  while (__last != __first) {
+    --__last;
+    std::__destroy_at(std::addressof(*__last));
+  }
+  return __last;
+}
+
+// Destroy all elements in [__first, __last) from left to right using allocator destruction.
+template <class _Alloc, class _Iter, class _Sent>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
+__allocator_destroy(_Alloc& __alloc, _Iter __first, _Sent __last) {
+  for (; __first != __last; ++__first)
+    allocator_traits<_Alloc>::destroy(__alloc, std::__to_address(__first));
+}
+
+template <class _ForwardIterator>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void destroy(_ForwardIterator __first, _ForwardIterator __last) {
+  (void)std::__destroy(std::move(__first), std::move(__last));
+}
+
+template <class _ForwardIterator, class _Size>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator destroy_n(_ForwardIterator __first, _Size __n) {
+  for (; __n > 0; (void)++__first, --__n)
+    std::__destroy_at(std::addressof(*__first));
+  return __first;
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // _LIBCPP___MEMORY_DESTROY_H
diff --git a/libcxx/include/__memory/ranges_construct_at.h b/libcxx/include/__memory/ranges_construct_at.h
index 35ed79651000386..b8a94678c10c850 100644
--- a/libcxx/include/__memory/ranges_construct_at.h
+++ b/libcxx/include/__memory/ranges_construct_at.h
@@ -61,41 +61,6 @@ inline namespace __cpo {
 inline constexpr auto destroy_at = __destroy_at{};
 } // namespace __cpo
 
-// destroy
-
-struct __destroy {
-  template <__nothrow_input_iterator _InputIterator, __nothrow_sentinel_for<_InputIterator> _Sentinel>
-    requires destructible<iter_value_t<_InputIterator>>
-  _LIBCPP_HIDE_FROM_ABI constexpr _InputIterator operator()(_InputIterator __first, _Sentinel __last) const noexcept {
-    return std::__destroy(std::move(__first), std::move(__last));
-  }
-
-  template <__nothrow_input_range _InputRange>
-    requires destructible<range_value_t<_InputRange>>
-  _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_InputRange> operator()(_InputRange&& __range) const noexcept {
-    return (*this)(ranges::begin(__range), ranges::end(__range));
-  }
-};
-
-inline namespace __cpo {
-inline constexpr auto destroy = __destroy{};
-} // namespace __cpo
-
-// destroy_n
-
-struct __destroy_n {
-  template <__nothrow_input_iterator _InputIterator>
-    requires destructible<iter_value_t<_InputIterator>>
-  _LIBCPP_HIDE_FROM_ABI constexpr _InputIterator
-  operator()(_InputIterator __first, iter_difference_t<_InputIterator> __n) const noexcept {
-    return std::destroy_n(std::move(__first), __n);
-  }
-};
-
-inline namespace __cpo {
-inline constexpr auto destroy_n = __destroy_n{};
-} // namespace __cpo
-
 } // namespace ranges
 
 #endif // _LIBCPP_STD_VER >= 20
diff --git a/libcxx/include/__memory/ranges_destroy.h b/libcxx/include/__memory/ranges_destroy.h
new file mode 100644
index 000000000000000..83421e025a891e5
--- /dev/null
+++ b/libcxx/include/__memory/ranges_destroy.h
@@ -0,0 +1,77 @@
+// -*- 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_RANGES_DESTROY_H
+#define _LIBCPP___MEMORY_RANGES_DESTROY_H
+
+#include <__concepts/destructible.h>
+#include <__config>
+#include <__iterator/iterator_traits.h>
+#include <__memory/concepts.h>
+#include <__memory/destroy.h>
+#include <__ranges/access.h>
+#include <__ranges/concepts.h>
+#include <__utility/move.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER >= 20
+namespace ranges {
+
+// destroy
+
+struct __destroy {
+  template <__nothrow_input_iterator _InputIterator, __nothrow_sentinel_for<_InputIterator> _Sentinel>
+    requires destructible<iter_value_t<_InputIterator>>
+  _LIBCPP_HIDE_FROM_ABI constexpr _InputIterator operator()(_InputIterator __first, _Sentinel __last) const noexcept {
+    return std::__destroy(std::move(__first), std::move(__last));
+  }
+
+  template <__nothrow_input_range _InputRange>
+    requires destructible<range_value_t<_InputRange>>
+  _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_InputRange> operator()(_InputRange&& __range) const noexcept {
+    return (*this)(ranges::begin(__range), ranges::end(__range));
+  }
+};
+
+inline namespace __cpo {
+inline constexpr auto destroy = __destroy{};
+} // namespace __cpo
+
+// destroy_n
+
+struct __destroy_n {
+  template <__nothrow_input_iterator _InputIterator>
+    requires destructible<iter_value_t<_InputIterator>>
+  _LIBCPP_HIDE_FROM_ABI constexpr _InputIterator
+  operator()(_InputIterator __first, iter_difference_t<_InputIterator> __n) const noexcept {
+    return std::destroy_n(std::move(__first), __n);
+  }
+};
+
+inline namespace __cpo {
+inline constexpr auto destroy_n = __destroy_n{};
+} // namespace __cpo
+
+} // namespace ranges
+
+#endif // _LIBCPP_STD_VER >= 20
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // _LIBCPP___MEMORY_RANGES_DESTROY_H
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 06b1fc488cf5155..d513fa692c1f973 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -29,6 +29,7 @@
 #include <__memory/auto_ptr.h>
 #include <__memory/compressed_pair.h>
 #include <__memory/construct_at.h>
+#include <__memory/destroy.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/shared_count.h>
 #include <__memory/uninitialized_algorithms.h>
diff --git a/libcxx/include/__memory/uninitialized_algorithms.h b/libcxx/include/__memory/uninitialized_algorithms.h
index a02a88399a7a727..735ca020bca39a8 100644
--- a/libcxx/include/__memory/uninitialized_algorithms.h
+++ b/libcxx/include/__memory/uninitialized_algorithms.h
@@ -19,6 +19,7 @@
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
 #include <__memory/addressof.h>
+#include <__memory/destroy.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/construct_at.h>
 #include <__memory/pointer_traits.h>
@@ -511,14 +512,6 @@ __uninitialized_allocator_value_construct_n_multidimensional(_Alloc& __alloc, _B
 
 #endif // _LIBCPP_STD_VER >= 17
 
-// Destroy all elements in [__first, __last) from left to right using allocator destruction.
-template <class _Alloc, class _Iter, class _Sent>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-__allocator_destroy(_Alloc& __alloc, _Iter __first, _Sent __last) {
-  for (; __first != __last; ++__first)
-    allocator_traits<_Alloc>::destroy(__alloc, std::__to_address(__first));
-}
-
 template <class _Alloc, class _Iter>
 class _AllocatorDestroyRangeReverse {
 public:
diff --git a/libcxx/include/__pstl/backends/libdispatch.h b/libcxx/include/__pstl/backends/libdispatch.h
index 4c63c4c8442074d..a640a40352f5c5d 100644
--- a/libcxx/include/__pstl/backends/libdispatch.h
+++ b/libcxx/include/__pstl/backends/libdispatch.h
@@ -22,6 +22,7 @@
 #include <__iterator/move_iterator.h>
 #include <__memory/allocator.h>
 #include <__memory/construct_at.h>
+#include <__memory/destroy.h>
 #include <__memory/unique_ptr.h>
 #include <__new/exceptions.h>
 #include <__numeric/reduce.h>
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index fd39c946b992a43..b0720703bd0def6 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1567,6 +1567,7 @@ module std [system] {
     module compressed_pair                    { header "__memory/compressed_pair.h" }
     module concepts                           { header "__memory/concepts.h" }
     module construct_at                       { header "__memory/construct_at.h" }
+    module destroy                            { header "__memory/destroy.h" }
     module destruct_n                         { header "__memory/destruct_n.h" }
     module fwd                                { header "__fwd/memory.h" }
     module inout_ptr                          { header "__memory/inout_ptr.h" }
@@ -1574,6 +1575,7 @@ module std [system] {
     module out_ptr                            { header "__memory/out_ptr.h" }
     module pointer_traits                     { header "__memory/pointer_traits.h" }
     module ranges_construct_at                { header "__memory/ranges_construct_at.h" }
+    module ranges_destroy                     { header "__memory/ranges_destroy.h" }
     module ranges_uninitialized_algorithms {
       header "__memory/ranges_uninitialized_algorithms.h"
       export std.algorithm.in_out_result

github-actions · 2025-02-10T00:59:07Z

✅ With the latest revision this PR passed the C/C++ code formatter.

This patch separates the destroy algorithms from the primitive construct_at and destroy_at operations, which are conceptually not algorithms. This makes it easier to start using these destroy algorithms from upcoming relocation facilities.

philnik777 · 2025-02-10T14:03:45Z

libcxx/include/__memory/construct_at.h

+  auto const __end = std::end(*__loc);
+  for (auto __it = std::begin(*__loc); __it != __end; ++__it)
+    std::__destroy_at(__it);


Just to note: the previous implementation was based on the standard wording. I think this is simple enough that the change in implementation is fine.

This patch separates the destroy algorithms from the primitive construct_at and destroy_at operations, which are conceptually not algorithms. This makes it easier to start using these destroy algorithms from upcoming relocation facilities. As part of this, it also implements `std::destroy_at` for arrays without relying on the `std::destroy` algorithm, which is conceptually a higher-level facility.

ldionne requested a review from a team as a code owner February 10, 2025 00:55

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 10, 2025

ldionne force-pushed the review/destroy-algorithms branch from 7f38485 to f0d0a6f Compare February 10, 2025 01:10

ldionne added 2 commits February 9, 2025 21:33

Sort includes

0b08e02

Missing includes

6937076

philnik777 approved these changes Feb 10, 2025

View reviewed changes

ldionne merged commit f332455 into llvm:main Feb 10, 2025
76 checks passed

ldionne deleted the review/destroy-algorithms branch February 10, 2025 20:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Extract destroy algorithms into separate headers #126449

[libc++] Extract destroy algorithms into separate headers #126449

Uh oh!

ldionne commented Feb 10, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 10, 2025

Uh oh!

github-actions bot commented Feb 10, 2025 •

edited

Loading

Uh oh!

philnik777 Feb 10, 2025

Uh oh!

Uh oh!

Uh oh!

[libc++] Extract destroy algorithms into separate headers #126449

[libc++] Extract destroy algorithms into separate headers #126449

Uh oh!

Conversation

ldionne commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 10, 2025

Uh oh!

github-actions bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philnik777 Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ldionne commented Feb 10, 2025 •

edited

Loading

github-actions bot commented Feb 10, 2025 •

edited

Loading