[libc++] Revert __uninitialized_buffer changes
authorLouis Dionne <ldionne.2@gmail.com>
Thu, 29 Jun 2023 22:09:58 +0000 (18:09 -0400)
committerLouis Dionne <ldionne.2@gmail.com>
Fri, 30 Jun 2023 13:17:24 +0000 (09:17 -0400)
This patch reverts the following commits:

    015cd317eaed28a923d14a33c9d6739012a688be (add missing HIDE_FROM_ABI)
    420a204d52205f1277a8d5df3dbafac6082e02e2 (add _LIBCPP_NO_CFI)
    31eeba3f7c0e2ef4a21c07da9326a4ae1a8de7e2 (add __uninitialized_buffer)

It also reverts a small part of b935ab8e747cf52ff12471879460206a9f433eea
which is required to make the stable_partition.pass.cpp test pass on GCC.

Some issues were pointed out in https://reviews.llvm.org/D152208 and
in https://reviews.llvm.org/D154017, so I am reverting this patch
until we have time to weigh the various solutions and get consensus
on the design of the API.

Differential Revision: https://reviews.llvm.org/D154161

libcxx/include/CMakeLists.txt
libcxx/include/__algorithm/inplace_merge.h
libcxx/include/__algorithm/stable_partition.h
libcxx/include/__algorithm/stable_sort.h
libcxx/include/__memory/construct_at.h
libcxx/include/__memory/temporary_buffer.h
libcxx/include/__memory/uninitialized_buffer.h [deleted file]
libcxx/include/module.modulemap.in
libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp

index 35c0275..9ab8fcf 100644 (file)
@@ -504,7 +504,6 @@ set(files
   __memory/temp_value.h
   __memory/temporary_buffer.h
   __memory/uninitialized_algorithms.h
-  __memory/uninitialized_buffer.h
   __memory/unique_ptr.h
   __memory/uses_allocator.h
   __memory/uses_allocator_construction.h
index 8da7a34..44a9425 100644 (file)
@@ -24,7 +24,7 @@
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
 #include <__memory/destruct_n.h>
-#include <__memory/uninitialized_buffer.h>
+#include <__memory/temporary_buffer.h>
 #include <__memory/unique_ptr.h>
 #include <__utility/pair.h>
 #include <new>
@@ -225,16 +225,13 @@ __inplace_merge(_BidirectionalIterator __first, _BidirectionalIterator __middle,
     difference_type __len1 = _IterOps<_AlgPolicy>::distance(__first, __middle);
     difference_type __len2 = _IterOps<_AlgPolicy>::distance(__middle, __last);
     difference_type __buf_size = _VSTD::min(__len1, __len2);
-    auto __buf = std::__make_uninitialized_buffer<value_type[]>(nothrow, __buf_size);
+// TODO: Remove the use of std::get_temporary_buffer
+_LIBCPP_SUPPRESS_DEPRECATED_PUSH
+    pair<value_type*, ptrdiff_t> __buf = _VSTD::get_temporary_buffer<value_type>(__buf_size);
+_LIBCPP_SUPPRESS_DEPRECATED_POP
+    unique_ptr<value_type, __return_temporary_buffer> __h(__buf.first);
     return std::__inplace_merge<_AlgPolicy>(
-        std::move(__first),
-        std::move(__middle),
-        std::move(__last),
-        __comp,
-        __len1,
-        __len2,
-        __buf.get(),
-        __buf ? __buf_size : 0);
+        std::move(__first), std::move(__middle), std::move(__last), __comp, __len1, __len2, __buf.first, __buf.second);
 }
 
 template <class _BidirectionalIterator, class _Compare>
index 3333981..38fa9ce 100644 (file)
@@ -16,7 +16,7 @@
 #include <__iterator/distance.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/destruct_n.h>
-#include <__memory/uninitialized_buffer.h>
+#include <__memory/temporary_buffer.h>
 #include <__memory/unique_ptr.h>
 #include <__utility/move.h>
 #include <__utility/pair.h>
@@ -29,7 +29,7 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _AlgPolicy, class _Predicate, class _ForwardIterator, class _Distance, class _Pair>
-_LIBCPP_HIDDEN _ForwardIterator
+_LIBCPP_HIDE_FROM_ABI _ForwardIterator
 __stable_partition_impl(_ForwardIterator __first, _ForwardIterator __last, _Predicate __pred,
                    _Distance __len, _Pair __p, forward_iterator_tag __fit)
 {
@@ -138,18 +138,18 @@ __stable_partition_impl(_ForwardIterator __first, _ForwardIterator __last, _Pred
     // We now have a reduced range [__first, __last)
     // *__first is known to be false
     difference_type __len = _IterOps<_AlgPolicy>::distance(__first, __last);
-
-    __uninitialized_buffer_t<value_type[]> __buf;
+    pair<value_type*, ptrdiff_t> __p(0, 0);
+    unique_ptr<value_type, __return_temporary_buffer> __h;
     if (__len >= __alloc_limit)
-        __buf = std::__make_uninitialized_buffer<value_type[]>(nothrow, __len);
-
+    {
+// TODO: Remove the use of std::get_temporary_buffer
+_LIBCPP_SUPPRESS_DEPRECATED_PUSH
+        __p = _VSTD::get_temporary_buffer<value_type>(__len);
+_LIBCPP_SUPPRESS_DEPRECATED_POP
+        __h.reset(__p.first);
+    }
     return std::__stable_partition_impl<_AlgPolicy, _Predicate&>(
-        std::move(__first),
-        std::move(__last),
-        __pred,
-        __len,
-        std::make_pair(__buf.get(), __buf ? __len : 0),
-        forward_iterator_tag());
+        std::move(__first), std::move(__last), __pred, __len, __p, forward_iterator_tag());
 }
 
 template <class _AlgPolicy, class _Predicate, class _BidirectionalIterator, class _Distance, class _Pair>
@@ -292,17 +292,18 @@ __stable_partition_impl(_BidirectionalIterator __first, _BidirectionalIterator _
     // *__last is known to be true
     // __len >= 2
     difference_type __len = _IterOps<_AlgPolicy>::distance(__first, __last) + 1;
-    __uninitialized_buffer_t<value_type[]> __buf;
+    pair<value_type*, ptrdiff_t> __p(0, 0);
+    unique_ptr<value_type, __return_temporary_buffer> __h;
     if (__len >= __alloc_limit)
-        __buf = std::__make_uninitialized_buffer<value_type[]>(nothrow, __len);
-
+    {
+// TODO: Remove the use of std::get_temporary_buffer
+_LIBCPP_SUPPRESS_DEPRECATED_PUSH
+        __p = _VSTD::get_temporary_buffer<value_type>(__len);
+_LIBCPP_SUPPRESS_DEPRECATED_POP
+        __h.reset(__p.first);
+    }
     return std::__stable_partition_impl<_AlgPolicy, _Predicate&>(
-        std::move(__first),
-        std::move(__last),
-        __pred,
-        __len,
-        std::make_pair(__buf.get(), __buf ? __len : 0),
-        bidirectional_iterator_tag());
+        std::move(__first), std::move(__last), __pred, __len, __p, bidirectional_iterator_tag());
 }
 
 template <class _AlgPolicy, class _Predicate, class _ForwardIterator, class _IterCategory>
index 0a15b4d..dc24218 100644 (file)
@@ -18,7 +18,7 @@
 #include <__debug_utils/strict_weak_ordering_check.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/destruct_n.h>
-#include <__memory/uninitialized_buffer.h>
+#include <__memory/temporary_buffer.h>
 #include <__memory/unique_ptr.h>
 #include <__type_traits/is_trivially_copy_assignable.h>
 #include <__utility/move.h>
@@ -249,12 +249,17 @@ void __stable_sort_impl(_RandomAccessIterator __first, _RandomAccessIterator __l
   using difference_type = typename iterator_traits<_RandomAccessIterator>::difference_type;
 
   difference_type __len = __last - __first;
-  __uninitialized_buffer_t<value_type[]> __buf;
-  if (__len > static_cast<difference_type>(__stable_sort_switch<value_type>::value))
-        __buf = std::__make_uninitialized_buffer<value_type[]>(nothrow, __len);
+  pair<value_type*, ptrdiff_t> __buf(0, 0);
+  unique_ptr<value_type, __return_temporary_buffer> __h;
+  if (__len > static_cast<difference_type>(__stable_sort_switch<value_type>::value)) {
+// TODO: Remove the use of std::get_temporary_buffer
+_LIBCPP_SUPPRESS_DEPRECATED_PUSH
+      __buf = std::get_temporary_buffer<value_type>(__len);
+_LIBCPP_SUPPRESS_DEPRECATED_POP
+      __h.reset(__buf.first);
+  }
 
-  std::__stable_sort<_AlgPolicy, __comp_ref_type<_Compare> >(
-      __first, __last, __comp, __len, __buf.get(), __buf ? __len : 0);
+  std::__stable_sort<_AlgPolicy, __comp_ref_type<_Compare> >(__first, __last, __comp, __len, __buf.first, __buf.second);
   std::__check_strict_weak_ordering_sorted(__first, __last, __comp);
 }
 
index 4bab3be..a41fcaa 100644 (file)
@@ -93,16 +93,6 @@ _BidirectionalIterator __reverse_destroy(_BidirectionalIterator __first, _Bidire
     return __last;
 }
 
-template <class _ForwardIterator, class _Size>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
-_ForwardIterator __destroy_n(_ForwardIterator __first, _Size __n) {
-    while (__n > 0) {
-        std::__destroy_at(std::addressof(*__first));
-        ++__first;
-        --__n;
-    }
-    return __first;
-}
 #if _LIBCPP_STD_VER >= 17
 
 template <class _Tp, enable_if_t<!is_array_v<_Tp>, int> = 0>
@@ -128,7 +118,9 @@ void destroy(_ForwardIterator __first, _ForwardIterator __last) {
 template <class _ForwardIterator, class _Size>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
 _ForwardIterator destroy_n(_ForwardIterator __first, _Size __n) {
-  return std::__destroy_n(__first, __n);
+    for (; __n > 0; (void)++__first, --__n)
+        std::__destroy_at(std::addressof(*__first));
+    return __first;
 }
 
 #endif // _LIBCPP_STD_VER >= 17
index a32dfa0..c917f04 100644 (file)
@@ -74,6 +74,14 @@ void return_temporary_buffer(_Tp* __p) _NOEXCEPT
   _VSTD::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
 }
 
+struct __return_temporary_buffer
+{
+_LIBCPP_SUPPRESS_DEPRECATED_PUSH
+    template <class _Tp>
+    _LIBCPP_INLINE_VISIBILITY void operator()(_Tp* __p) const {_VSTD::return_temporary_buffer(__p);}
+_LIBCPP_SUPPRESS_DEPRECATED_POP
+};
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___MEMORY_TEMPORARY_BUFFER_H
diff --git a/libcxx/include/__memory/uninitialized_buffer.h b/libcxx/include/__memory/uninitialized_buffer.h
deleted file mode 100644 (file)
index 5b12633..0000000
+++ /dev/null
@@ -1,81 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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_UNINITIALIZED_BUFFER_H
-#define _LIBCPP___MEMORY_UNINITIALIZED_BUFFER_H
-
-#include <__config>
-#include <__memory/construct_at.h>
-#include <__memory/unique_ptr.h>
-#include <__type_traits/is_array.h>
-#include <__type_traits/is_default_constructible.h>
-#include <__type_traits/remove_extent.h>
-#include <__utility/move.h>
-#include <cstddef>
-#include <new>
-
-#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-#  pragma GCC system_header
-#endif
-
-// __make_uninitialized_buffer is a utility function to allocate some memory for scratch storage. The __destructor is
-// called before deleting the memory, making it possible to destroy any leftover elements. The __destructor is called
-// with the pointer to the first element and the total number of elements.
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-template <class _Destructor>
-class __uninitialized_buffer_deleter {
-  size_t __count_;
-  _Destructor __destructor_;
-
-public:
-  template <class _Dummy = int, __enable_if_t<is_default_constructible<_Destructor>::value, _Dummy> = 0>
-  _LIBCPP_HIDE_FROM_ABI __uninitialized_buffer_deleter() : __count_(0) {}
-
-  _LIBCPP_HIDE_FROM_ABI __uninitialized_buffer_deleter(size_t __count, _Destructor __destructor)
-      : __count_(__count), __destructor_(std::move(__destructor)) {}
-
-  template <class _Tp>
-  _LIBCPP_HIDE_FROM_ABI void operator()(_Tp* __ptr) {
-    __destructor_(__ptr, __count_);
-#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
-    ::operator delete(__ptr);
-#else
-    ::operator delete(__ptr, __count_ * sizeof(_Tp), align_val_t(_LIBCPP_ALIGNOF(_Tp)));
-#endif
-  }
-};
-
-struct __noop {
-  template <class... _Args>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void operator()(_Args&&...) const {}
-};
-
-template <class _Array, class _Destructor = __noop>
-using __uninitialized_buffer_t = unique_ptr<_Array, __uninitialized_buffer_deleter<_Destructor> >;
-
-template <class _Array, class _Destructor = __noop>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI __uninitialized_buffer_t<_Array, _Destructor>
-__make_uninitialized_buffer(nothrow_t, size_t __count, _Destructor __destructor = __noop()) {
-  static_assert(is_array<_Array>::value, "");
-  using _Tp = __remove_extent_t<_Array>;
-
-#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
-  _Tp* __ptr = static_cast<_Tp*>(::operator new(sizeof(_Tp) * __count, nothrow));
-#else
-  _Tp* __ptr = static_cast<_Tp*>(::operator new(sizeof(_Tp) * __count, align_val_t(_LIBCPP_ALIGNOF(_Tp)), nothrow));
-#endif
-
-  using _Deleter = __uninitialized_buffer_deleter<_Destructor>;
-  return unique_ptr<_Array, _Deleter>(__ptr, _Deleter(__count, std::move(__destructor)));
-}
-
-_LIBCPP_END_NAMESPACE_STD
-
-#endif // _LIBCPP___MEMORY_UNINITIALIZED_BUFFER_H
index a47e0ef..ced3a49 100644 (file)
@@ -1214,7 +1214,6 @@ module std [system] {
       module temp_value                      { private header "__memory/temp_value.h" }
       module temporary_buffer                { private header "__memory/temporary_buffer.h" }
       module uninitialized_algorithms        { private header "__memory/uninitialized_algorithms.h" }
-      module uninitialized_buffer            { private header "__memory/uninitialized_buffer.h" }
       module unique_ptr                      { private header "__memory/unique_ptr.h" }
       module uses_allocator                  { private header "__memory/uses_allocator.h" }
       module uses_allocator_construction     { private header "__memory/uses_allocator_construction.h" }
index 5fc6633..85d12d0 100644 (file)
@@ -282,6 +282,9 @@ test()
     assert(array[9] == P(0, 2));
   }
 #if TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
+  // TODO: Re-enable this test once we're no longer using get_temporary_buffer().
+  // For now it trips up GCC due to the use of always_inline.
+#if 0
   { // check that the algorithm still works when no memory is available
     std::vector<int> vec(150, 3);
     vec[5]                             = 6;
@@ -297,6 +300,7 @@ test()
     assert(std::is_partitioned(vec.begin(), vec.end(), [](int i) { return i < 5; }));
     getGlobalMemCounter()->reset();
   }
+#endif
 #endif // TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
 }