[libc++] Remove get_temporary_buffer/return_temporary_buffer
#100914
Merged
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++
Member
|
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesWorks towards P0619R4/#99985.
Escape hatches:
Drive-by changes:
Full diff: https://github.com/llvm/llvm-project/pull/100914.diff 21 Files Affected:
diff --git a/libcxx/docs/Status/Cxx17.rst b/libcxx/docs/Status/Cxx17.rst
index ad4f8576f03db..ac07d299d7e74 100644
--- a/libcxx/docs/Status/Cxx17.rst
+++ b/libcxx/docs/Status/Cxx17.rst
@@ -40,6 +40,7 @@ Paper Status
.. note::
+ .. [#note-N4259] N4259: ``std::uncaught_exception`` is deprecated since version 20.0.
.. [#note-P0067] P0067: ``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``.
.. [#note-P0226] P0226: Progress is tracked `here <https://https://libcxx.llvm.org/Status/SpecialMath.html>`_.
.. [#note-P0607] P0607: The parts of P0607 that are not done are the ``<regex>`` bits.
diff --git a/libcxx/docs/Status/Cxx17Papers.csv b/libcxx/docs/Status/Cxx17Papers.csv
index 6c657d51f5c7e..f9a87b2cbfd22 100644
--- a/libcxx/docs/Status/Cxx17Papers.csv
+++ b/libcxx/docs/Status/Cxx17Papers.csv
@@ -4,7 +4,7 @@
"`N4169 <https://wg21.link/n4169>`__","LWG","A proposal to add invoke function template","Urbana","|Complete|","3.7"
"`N4190 <https://wg21.link/n4190>`__","LWG","Removing auto_ptr, random_shuffle(), And Old <functional> Stuff.","Urbana","|Complete|","15.0"
"`N4258 <https://wg21.link/n4258>`__","LWG","Cleaning-up noexcept in the Library.","Urbana","|In Progress|","3.7"
-"`N4259 <https://wg21.link/n4259>`__","CWG","Wording for std::uncaught_exceptions","Urbana","|Complete|","3.7"
+"`N4259 <https://wg21.link/n4259>`__","CWG","Wording for std::uncaught_exceptions","Urbana","|Complete| [#note-N4259]_","3.7"
"`N4277 <https://wg21.link/n4277>`__","LWG","TriviallyCopyable ``reference_wrapper``\ .","Urbana","|Complete|","3.2"
"`N4279 <https://wg21.link/n4279>`__","LWG","Improved insertion interface for unique-key maps.","Urbana","|Complete|","3.7"
"`N4280 <https://wg21.link/n4280>`__","LWG","Non-member size() and more","Urbana","|Complete|","3.6"
diff --git a/libcxx/docs/Status/Cxx20.rst b/libcxx/docs/Status/Cxx20.rst
index b76e30fbb3712..a13496bd0a8fd 100644
--- a/libcxx/docs/Status/Cxx20.rst
+++ b/libcxx/docs/Status/Cxx20.rst
@@ -44,7 +44,7 @@ Paper Status
.. [#note-P0645] P0645: The paper is implemented but still marked as an incomplete feature
(the feature-test macro is not set).
.. [#note-P0966] P0966: It was previously erroneously marked as complete in version 8.0. See `bug 45368 <https://llvm.org/PR45368>`__.
- .. [#note-P0619] P0619: Only sections D.8, D.9, D.10 and D.13 are implemented. Sections D.4, D.7, D.11, and D.12 remain undone.
+ .. [#note-P0619] P0619: Section D.4 remains undone.
.. [#note-P0883.1] P0883: shared_ptr and floating-point changes weren't applied as they themselves aren't implemented yet.
.. [#note-P0883.2] P0883: ``ATOMIC_FLAG_INIT`` was marked deprecated in version 14.0, but was undeprecated with the implementation of LWG3659 in version 15.0.
.. [#note-P0660] P0660: The paper is implemented but the features are experimental and can be enabled via ``-fexperimental-library``.
diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst
index df08875c13bea..8937639bd2bf2 100644
--- a/libcxx/docs/UsingLibcxx.rst
+++ b/libcxx/docs/UsingLibcxx.rst
@@ -215,6 +215,9 @@ C++17 Specific Configuration Macros
C++20 Specific Configuration Macros
-----------------------------------
+**_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION**:
+ This macro is used to re-enable `uncaught_exception`.
+
**_LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE**:
This macro is used to re-enable the function
``std::shared_ptr<...>::unique()``.
@@ -231,6 +234,9 @@ C++20 Specific Configuration Macros
**_LIBCPP_ENABLE_CXX20_REMOVED_RAW_STORAGE_ITERATOR**:
This macro is used to re-enable `raw_storage_iterator`.
+**_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER**:
+ This macro is used to re-enable `get_temporary_buffer` and `return_temporary_buffer`.
+
**_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS**:
This macro is used to re-enable `is_literal_type`, `is_literal_type_v`,
`result_of` and `result_of_t`.
diff --git a/libcxx/include/__algorithm/inplace_merge.h b/libcxx/include/__algorithm/inplace_merge.h
index a6bcc66a2fa47..4c6ab527c8188 100644
--- a/libcxx/include/__algorithm/inplace_merge.h
+++ b/libcxx/include/__algorithm/inplace_merge.h
@@ -211,10 +211,7 @@ _LIBCPP_HIDE_FROM_ABI void __inplace_merge(
difference_type __len1 = _IterOps<_AlgPolicy>::distance(__first, __middle);
difference_type __len2 = _IterOps<_AlgPolicy>::distance(__middle, __last);
difference_type __buf_size = std::min(__len1, __len2);
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- pair<value_type*, ptrdiff_t> __buf = std::get_temporary_buffer<value_type>(__buf_size);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ pair<value_type*, ptrdiff_t> __buf = std::__get_temporary_buffer<value_type>(__buf_size);
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.first, __buf.second);
diff --git a/libcxx/include/__algorithm/stable_partition.h b/libcxx/include/__algorithm/stable_partition.h
index 8bb1eaf2d2249..4f09b87ae0f9a 100644
--- a/libcxx/include/__algorithm/stable_partition.h
+++ b/libcxx/include/__algorithm/stable_partition.h
@@ -135,10 +135,7 @@ __stable_partition_impl(_ForwardIterator __first, _ForwardIterator __last, _Pred
pair<value_type*, ptrdiff_t> __p(0, 0);
unique_ptr<value_type, __return_temporary_buffer> __h;
if (__len >= __alloc_limit) {
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- __p = std::get_temporary_buffer<value_type>(__len);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ __p = std::__get_temporary_buffer<value_type>(__len);
__h.reset(__p.first);
}
return std::__stable_partition_impl<_AlgPolicy, _Predicate&>(
@@ -275,10 +272,7 @@ _LIBCPP_HIDE_FROM_ABI _BidirectionalIterator __stable_partition_impl(
pair<value_type*, ptrdiff_t> __p(0, 0);
unique_ptr<value_type, __return_temporary_buffer> __h;
if (__len >= __alloc_limit) {
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- __p = std::get_temporary_buffer<value_type>(__len);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ __p = std::__get_temporary_buffer<value_type>(__len);
__h.reset(__p.first);
}
return std::__stable_partition_impl<_AlgPolicy, _Predicate&>(
diff --git a/libcxx/include/__algorithm/stable_sort.h b/libcxx/include/__algorithm/stable_sort.h
index 726e7e16b3564..69fd85cc798bd 100644
--- a/libcxx/include/__algorithm/stable_sort.h
+++ b/libcxx/include/__algorithm/stable_sort.h
@@ -244,10 +244,7 @@ __stable_sort_impl(_RandomAccessIterator __first, _RandomAccessIterator __last,
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
+ __buf = std::__get_temporary_buffer<value_type>(__len);
__h.reset(__buf.first);
}
diff --git a/libcxx/include/__exception/operations.h b/libcxx/include/__exception/operations.h
index 0a9c7a7c7f0d8..4a0a697c00e6e 100644
--- a/libcxx/include/__exception/operations.h
+++ b/libcxx/include/__exception/operations.h
@@ -29,7 +29,9 @@ using terminate_handler = void (*)();
_LIBCPP_EXPORTED_FROM_ABI terminate_handler set_terminate(terminate_handler) _NOEXCEPT;
_LIBCPP_EXPORTED_FROM_ABI terminate_handler get_terminate() _NOEXCEPT;
-_LIBCPP_EXPORTED_FROM_ABI bool uncaught_exception() _NOEXCEPT;
+#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION)
+_LIBCPP_EXPORTED_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX17 bool uncaught_exception() _NOEXCEPT;
+#endif // _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION)
_LIBCPP_EXPORTED_FROM_ABI int uncaught_exceptions() _NOEXCEPT;
class _LIBCPP_EXPORTED_FROM_ABI exception_ptr;
diff --git a/libcxx/include/__memory/temporary_buffer.h b/libcxx/include/__memory/temporary_buffer.h
index 88799ca95c1f3..cb44006954fa7 100644
--- a/libcxx/include/__memory/temporary_buffer.h
+++ b/libcxx/include/__memory/temporary_buffer.h
@@ -22,8 +22,8 @@
_LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp>
-_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI _LIBCPP_DEPRECATED_IN_CXX17 pair<_Tp*, ptrdiff_t>
-get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
+_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI pair<_Tp*, ptrdiff_t>
+__get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
pair<_Tp*, ptrdiff_t> __r(0, 0);
const ptrdiff_t __m =
(~ptrdiff_t(0) ^ ptrdiff_t(ptrdiff_t(1) << (sizeof(ptrdiff_t) * __CHAR_BIT__ - 1))) / sizeof(_Tp);
@@ -56,20 +56,28 @@ get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
return __r;
}
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX17 void return_temporary_buffer(_Tp* __p) _NOEXCEPT {
- std::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
-}
-
struct __return_temporary_buffer {
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
template <class _Tp>
_LIBCPP_HIDE_FROM_ABI void operator()(_Tp* __p) const {
- std::return_temporary_buffer(__p);
+ std::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
}
- _LIBCPP_SUPPRESS_DEPRECATED_POP
};
+#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER)
+
+template <class _Tp>
+_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI _LIBCPP_DEPRECATED_IN_CXX17 pair<_Tp*, ptrdiff_t>
+get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
+ return std::__get_temporary_buffer(n);
+}
+
+template <class _Tp>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX17 void return_temporary_buffer(_Tp* __p) _NOEXCEPT {
+ std::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
+}
+
+#endif // _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER)
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___MEMORY_TEMPORARY_BUFFER_H
diff --git a/libcxx/include/__ostream/basic_ostream.h b/libcxx/include/__ostream/basic_ostream.h
index 178359d681567..e0698ccb4842a 100644
--- a/libcxx/include/__ostream/basic_ostream.h
+++ b/libcxx/include/__ostream/basic_ostream.h
@@ -152,7 +152,7 @@ basic_ostream<_CharT, _Traits>::sentry::sentry(basic_ostream<_CharT, _Traits>& _
template <class _CharT, class _Traits>
basic_ostream<_CharT, _Traits>::sentry::~sentry() {
- if (__os_.rdbuf() && __os_.good() && (__os_.flags() & ios_base::unitbuf) && !uncaught_exception()) {
+ if (__os_.rdbuf() && __os_.good() && (__os_.flags() & ios_base::unitbuf) && uncaught_exceptions() == 0) {
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
diff --git a/libcxx/include/exception b/libcxx/include/exception
index 5eff8e3f8a4bf..64463e02cb16a 100644
--- a/libcxx/include/exception
+++ b/libcxx/include/exception
@@ -47,7 +47,7 @@ terminate_handler set_terminate(terminate_handler f ) noexcept;
terminate_handler get_terminate() noexcept;
[[noreturn]] void terminate() noexcept;
-bool uncaught_exception() noexcept;
+bool uncaught_exception() noexcept; // deprecated in C++17, removed in C++20
int uncaught_exceptions() noexcept; // C++17
typedef unspecified exception_ptr;
diff --git a/libcxx/include/memory b/libcxx/include/memory
index b940a32c3ebe6..db3386cca4800 100644
--- a/libcxx/include/memory
+++ b/libcxx/include/memory
@@ -182,8 +182,8 @@ public:
raw_storage_iterator operator++(int);
};
-template <class T> pair<T*,ptrdiff_t> get_temporary_buffer(ptrdiff_t n) noexcept;
-template <class T> void return_temporary_buffer(T* p) noexcept;
+template <class T> pair<T*,ptrdiff_t> get_temporary_buffer(ptrdiff_t n) noexcept; // deprecated in C++17, removed in C++20
+template <class T> void return_temporary_buffer(T* p) noexcept; // deprecated in C++17, removed in C++20
template <class T> T* addressof(T& r) noexcept;
template <class T> T* addressof(const T&& r) noexcept = delete;
diff --git a/libcxx/include/syncstream b/libcxx/include/syncstream
index e6f35b6f428ed..9da2abb252bfd 100644
--- a/libcxx/include/syncstream
+++ b/libcxx/include/syncstream
@@ -358,7 +358,7 @@ private:
// TODO Use a more generic buffer.
// That buffer should be light with almost no additional headers. Then
// it can be use here, the __retarget_buffer, and place that use
- // the now deprecated get_temporary_buffer
+ // the now removed get_temporary_buffer
basic_string<_CharT, _Traits, _Allocator> __str_;
bool __emit_on_sync_{false};
diff --git a/libcxx/modules/std/exception.inc b/libcxx/modules/std/exception.inc
index 8e802c7065e0f..0c110e6e743fb 100644
--- a/libcxx/modules/std/exception.inc
+++ b/libcxx/modules/std/exception.inc
@@ -21,6 +21,5 @@ export namespace std {
using std::terminate;
using std::terminate_handler;
using std::throw_with_nested;
- using std::uncaught_exception;
using std::uncaught_exceptions;
} // namespace std
diff --git a/libcxx/src/exception.cpp b/libcxx/src/exception.cpp
index ddb186bf8048f..ac6324cd9fe35 100644
--- a/libcxx/src/exception.cpp
+++ b/libcxx/src/exception.cpp
@@ -6,6 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#define _LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION
+#define _LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
#include <exception>
#include <new>
#include <typeinfo>
diff --git a/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp
index 646569e3d573a..6410c84e926aa 100644
--- a/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp
@@ -10,6 +10,7 @@
// check that <memory> functions are marked [[nodiscard]]
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
// clang-format off
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
index 85d12d08c9675..5fc66339d5f80 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
@@ -282,9 +282,6 @@ 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;
@@ -300,7 +297,6 @@ 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)
}
diff --git a/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp b/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp
index e368ce177b317..6fa4cb22070c8 100644
--- a/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp
+++ b/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp
@@ -7,6 +7,10 @@
//===----------------------------------------------------------------------===//
// UNSUPPORTED: no-exceptions
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
// test uncaught_exception
#include <exception>
diff --git a/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp b/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp
index 8499478d1fb38..4c66370fac922 100644
--- a/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp
+++ b/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp
@@ -8,6 +8,7 @@
// UNSUPPORTED: c++03
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
// <memory>
diff --git a/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp b/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp
index 6c0fbf2568fff..5f7fc4571906c 100644
--- a/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp
+++ b/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp
@@ -8,6 +8,7 @@
// <memory>
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
// template <class T>
diff --git a/libcxx/utils/libcxx/test/modules.py b/libcxx/utils/libcxx/test/modules.py
index b7758dc9a41ee..ed0e0d684327b 100644
--- a/libcxx/utils/libcxx/test/modules.py
+++ b/libcxx/utils/libcxx/test/modules.py
@@ -52,13 +52,6 @@
"std::operator==",
]
-# TODO MODULES remove zombie names
-# https://libcxx.llvm.org/Status/Cxx20.html#note-p0619
-SkipDeclarations["memory"] = [
- "std::return_temporary_buffer",
- "std::get_temporary_buffer",
-]
-
# include/__type_traits/is_swappable.h
SkipDeclarations["type_traits"] = [
"std::swap",
|
frederick-vs-ja
force-pushed
the
libcxx-cxx20-removal
branch
3 times, most recently
from
bf4e6d5 to
da6d71e
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Works towards P0619R4. - `std::uncaught_exception` was not previously deprecated. This patch deprecates it since C++17 as per N4259. `std::uncaught_exceptions` is used instead as libc++ unconditionally provides this function. - `std::get_temporary_buffer` is replaced with the internal version `__get_temporary_buffer`. - `std::return_temporary_buffer` is replaced with direct `__libcpp_deallocate_unsized` call. Escape hatches: - `_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION` restores `std::uncaught_exception`. - `_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER` restores `std::get_temporary_buffer` and `std::return_temporary_buffer`.
frederick-vs-ja
force-pushed
the
libcxx-cxx20-removal
branch
from
da6d71e to
f7ff07e
Compare
philnik777
reviewed
Jul 28, 2024
mordante
requested changes
Aug 3, 2024
Member
mordante
left a comment
mordante
left a comment
There was a problem hiding this comment.
I really like to see this patch split in 2 patches. One for each removal. That makes reviewing the patch a lot easier.
# Conflicts: # libcxx/docs/Status/Cxx17Papers.csv
frederick-vs-ja
changed the title
get_temporary_buffer/return_temporary_buffer
Member
|
Somewhat related: https://reviews.llvm.org/D152208 |
ldionne
approved these changes
Aug 27, 2024
Member
ldionne
left a comment
ldionne
left a comment
There was a problem hiding this comment.
This is a really well written patch. I like the approach of using a __scoped_temporary_buffer. I have a few nitpick comments but this LGTM with those fixed.
# Conflicts: # libcxx/docs/ReleaseNotes/20.rst # libcxx/docs/Status/Cxx20.rst
philnik777
reviewed
Aug 28, 2024
Comment on lines
75
to
89
| #if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) | ||
| if (__is_overaligned_for_new(_LIBCPP_ALIGNOF(_Tp))) { | ||
| align_val_t __al = align_val_t(_LIBCPP_ALIGNOF(_Tp)); | ||
| __ptr_ = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), __al, nothrow)); | ||
| } else { | ||
| __ptr_ = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), nothrow)); | ||
| } | ||
| #else | ||
| if (__is_overaligned_for_new(_LIBCPP_ALIGNOF(_Tp))) { | ||
| // Since aligned operator new is unavailable, constructs an empty buffer rather than one with invalid alignment. | ||
| return; | ||
| } | ||
|
|
||
| __ptr_ = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), nothrow)); | ||
| #endif |
Contributor
There was a problem hiding this comment.
Isn't this the same logic __libcpp_operator_new implements?
Member
There was a problem hiding this comment.
FWIW I'd be in favour of copy-pasting the implementation of get_temporary_buffer() (what was done here) and then refactoring it, to avoid any unintended fallout from this patch.
| return; | ||
| } | ||
|
|
||
| std::__libcpp_deallocate_unsized((void*)__ptr_, _LIBCPP_ALIGNOF(_Tp)); |
Contributor
There was a problem hiding this comment.
Why unsized when we have the count?
Member
There was a problem hiding this comment.
This was just copied from the old code. I think we can refactor this quite a bit, but I would tackle that as a follow-up patch.
# Conflicts: # libcxx/docs/Status/Cxx20.rst
- C++03 `> >` - Module exporting - `unique_ptr` construction
philnik777
reviewed
Sep 1, 2024
Contributor
philnik777
left a comment
philnik777
left a comment
There was a problem hiding this comment.
I like this a lot more than the previous version. Let's see what Louis thinks.
ldionne
approved these changes
Sep 3, 2024
Member
ldionne
left a comment
ldionne
left a comment
There was a problem hiding this comment.
LGTM. I agree this is simpler now.
philnik777
approved these changes
Sep 3, 2024
Contributor
philnik777
left a comment
philnik777
left a comment
There was a problem hiding this comment.
I think we can simplify a few things, but this is an improvement as-is (and it may be better to simplify things separately). LGTM.
nico
reviewed
Sep 23, 2024
| align_val_t __al = align_val_t(_LIBCPP_ALIGNOF(_Tp)); | ||
| __ptr = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), __al, nothrow)); | ||
| } else { | ||
| __ptr = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), nothrow)); |
Contributor
There was a problem hiding this comment.
I think this breaks CFI builds. It allocates memory with uninitialized objects and casts them to _Tp*. I think you're supposed to keep this void* until you placement-new into it, at which point it becomes an actual object.
The old get_temporary_buffer had this problem too, but it's marked _LIBCPP_NO_CFI while this new function isn't. (get_temporary_buffer on the other hand probably doesn't have to be marked _LIBCPP_NO_CFI anymore.)
A simple fix would be to mark this new function here _LIBCPP_NO_CFI.
(Maybe the whole design is a bit questionable though? But I mostly care about CFI no longer working.)
Contributor
Author
There was a problem hiding this comment.
I'm now opening a PR to add _LIBCPP_NO_CFI to this function. Is there a better way to address this problem?
Contributor
There was a problem hiding this comment.
Have a void* for the uninitialized tail and a _Tp* for the initialized head, I suppose? Not sure :)
Thanks for the quick PR!
frederick-vs-ja
added a commit
that referenced
this pull request
Sep 24, 2024
…#109682) Follows up #100914. Addresses the issue revealed in #100914 (comment).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Works towards P0619R4/#99985.
The use of
std::get_temporary_bufferandstd::return_temporary_bufferare replaced withunique_ptr-based RAII buffer holder.Escape hatches:
_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFERrestoresstd::get_temporary_bufferandstd::return_temporary_buffer.Drive-by changes:
<syncstream>, states thatget_temporary_bufferis now removed, because<syncstream>is added in C++20.