[libc++][memory] P1132R8: out_ptr - a scalable output pointer abstraction
#73618
Conversation
This file contains 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: Hristo Hristov (H-G-Hristov) ChangesDifferential Revision: https://reviews.llvm.org/D150525 Patch is 44.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73618.diff 26 Files Affected:
diff --git a/libcxx/docs/FeatureTestMacroTable.rst b/libcxx/docs/FeatureTestMacroTable.rst
index 3b2dff3108b0f60..f33ca7f0cc58fd3 100644
--- a/libcxx/docs/FeatureTestMacroTable.rst
+++ b/libcxx/docs/FeatureTestMacroTable.rst
@@ -342,7 +342,7 @@ Status
--------------------------------------------------- -----------------
``__cpp_lib_optional`` ``202110L``
--------------------------------------------------- -----------------
- ``__cpp_lib_out_ptr`` *unimplemented*
+ ``__cpp_lib_out_ptr`` ``202106L``
--------------------------------------------------- -----------------
``__cpp_lib_print`` *unimplemented*
--------------------------------------------------- -----------------
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index b24ecc5525a1497..e5ad365f9ffb877 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -192,7 +192,7 @@
"`3515 <https://wg21.link/LWG3515>`__","§[stacktrace.basic.nonmem]: ``operator<<`` should be less templatized", "November 2022","","",""
"`3545 <https://wg21.link/LWG3545>`__","``std::pointer_traits`` should be SFINAE-friendly", "November 2022","|Complete|","18.0",""
"`3569 <https://wg21.link/LWG3569>`__","``join_view`` fails to support ranges of ranges with non-default_initializable iterators", "November 2022","","","|ranges|"
-"`3594 <https://wg21.link/LWG3594>`__","``inout_ptr`` — inconsistent ``release()`` in destructor", "November 2022","","",""
+"`3594 <https://wg21.link/LWG3594>`__","``inout_ptr`` — inconsistent ``release()`` in destructor", "November 2022","|Complete|","17.0",""
"`3597 <https://wg21.link/LWG3597>`__","Unsigned integer types don't model advanceable", "November 2022","","","|ranges|"
"`3600 <https://wg21.link/LWG3600>`__","Making ``istream_iterator`` copy constructor trivial is an ABI break", "November 2022","","",""
"`3629 <https://wg21.link/LWG3629>`__","``make_error_code`` and ``make_error_condition`` are customization points","November 2022","|Complete|","16.0",""
@@ -282,7 +282,7 @@
"`3645 <https://wg21.link/LWG3645>`__","``resize_and_overwrite`` is overspecified to call its callback with lvalues","February 2023","|Complete|","14.0",""
"`3655 <https://wg21.link/LWG3655>`__","The ``INVOKE`` operation and union types","February 2023","|Complete|","18.0",""
"`3723 <https://wg21.link/LWG3723>`__","``priority_queue::push_range`` needs to ``append_range``","February 2023","","","|ranges|"
-"`3734 <https://wg21.link/LWG3734>`__","Inconsistency in ``inout_ptr`` and ``out_ptr`` for empty case","February 2023","","",""
+"`3734 <https://wg21.link/LWG3734>`__","Inconsistency in ``inout_ptr`` and ``out_ptr`` for empty case","February 2023","|Complete|","17.0",""
"`3772 <https://wg21.link/LWG3772>`__","``repeat_view``'s ``piecewise`` constructor is missing Postconditions","February 2023","","","|ranges|"
"`3786 <https://wg21.link/LWG3786>`__","Flat maps' deduction guide needs to default ``Allocator`` to be useful","February 2023","","",""
"`3803 <https://wg21.link/LWG3803>`__","``flat_foo`` constructors taking ``KeyContainer`` lack ``KeyCompare`` parameter","February 2023","","",""
diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index 5cc9e488297b9f7..198b0ab9d390e33 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -13,7 +13,7 @@
"","","","","","",""
"`P0401R6 <https://wg21.link/P0401R6>`__","LWG","Providing size feedback in the Allocator interface","June 2021","|Complete|","15.0"
"`P0448R4 <https://wg21.link/P0448R4>`__","LWG","A strstream replacement using span<charT> as buffer","June 2021","",""
-"`P1132R8 <https://wg21.link/P1132R8>`__","LWG","out_ptr - a scalable output pointer abstraction","June 2021","",""
+"`P1132R8 <https://wg21.link/P1132R8>`__","LWG","out_ptr - a scalable output pointer abstraction","June 2021","|Complete|","17.0"
"`P1328R1 <https://wg21.link/P1328R1>`__","LWG","Making std::type_info::operator== constexpr","June 2021","|Complete|","17.0"
"`P1425R4 <https://wg21.link/P1425R4>`__","LWG","Iterators pair constructors for stack and queue","June 2021","|Complete|","14.0","|ranges|"
"`P1518R2 <https://wg21.link/P1518R2>`__","LWG","Stop overconstraining allocators in container deduction guides","June 2021","|Complete|","13.0"
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index b0bc3718576f66b..f2ba3d12a2b3854 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -528,6 +528,8 @@ set(files
__memory/concepts.h
__memory/construct_at.h
__memory/destruct_n.h
+ __memory/inout_ptr.h
+ __memory/out_ptr.h
__memory/pointer_traits.h
__memory/ranges_construct_at.h
__memory/ranges_uninitialized_algorithms.h
diff --git a/libcxx/include/__memory/allocator_traits.h b/libcxx/include/__memory/allocator_traits.h
index eea5ee973d37fd7..ed13cf8a3c278c1 100644
--- a/libcxx/include/__memory/allocator_traits.h
+++ b/libcxx/include/__memory/allocator_traits.h
@@ -38,7 +38,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp> struct NAME<_Tp, __void_t<typename _Tp:: PROPERTY > > : true_type { }
// __pointer
-_LIBCPP_ALLOCATOR_TRAITS_HAS_XXX(__has_pointer, pointer);
template <class _Tp, class _Alloc,
class _RawAlloc = __libcpp_remove_reference_t<_Alloc>,
bool = __has_pointer<_RawAlloc>::value>
diff --git a/libcxx/include/__memory/inout_ptr.h b/libcxx/include/__memory/inout_ptr.h
new file mode 100644
index 000000000000000..c2044a4642cf4ee
--- /dev/null
+++ b/libcxx/include/__memory/inout_ptr.h
@@ -0,0 +1,100 @@
+// -*- 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___INOUT_PTR_H
+#define _LIBCPP___INOUT_PTR_H
+
+#include <__config>
+#include <__memory/addressof.h>
+#include <__memory/pointer_traits.h>
+#include <__memory/shared_ptr.h>
+#include <__memory/unique_ptr.h>
+#include <__type_traits/is_same.h>
+#include <__type_traits/is_specialization.h>
+#include <__type_traits/is_void.h>
+#include <tuple>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER >= 23
+
+template <class _Smart, class _Pointer, class... _Args>
+class _LIBCPP_TEMPLATE_VIS inout_ptr_t {
+ static_assert(!__is_specialization_v<_Smart, shared_ptr>, "std::shared_ptr<> is not supported");
+
+public:
+ _LIBCPP_HIDE_FROM_ABI explicit inout_ptr_t(_Smart& __s, _Args... __args)
+ : __s_(__s), __a_(std::forward<_Args>(__args)...), __p_([&__s] {
+ if constexpr (is_pointer_v<_Smart>) {
+ return __s;
+ } else {
+ return __s.get();
+ }
+ }()) {
+ if constexpr (requires { __s.release(); }) {
+ __s.release();
+ } else {
+ __s = _Smart();
+ }
+ }
+
+ _LIBCPP_HIDE_FROM_ABI inout_ptr_t(const inout_ptr_t&) = delete;
+
+ _LIBCPP_HIDE_FROM_ABI ~inout_ptr_t() {
+ if (!__p_) {
+ return;
+ }
+
+ using _SP = __pointer_of_or_t<_Smart, _Pointer>;
+ if constexpr (is_pointer_v<_Smart>) {
+ std::apply([&](auto&&... __args) { __s_ = _Smart(static_cast<_SP>(__p_), std::forward<_Args>(__args)...); },
+ std::move(__a_));
+ } else if constexpr (__resettable_smart_pointer_with_args<_Smart, _Pointer, _Args...>) {
+ std::apply([&](auto&&... __args) { __s_.reset(static_cast<_SP>(__p_), std::forward<_Args>(__args)...); },
+ std::move(__a_));
+ } else if constexpr (is_constructible_v<_Smart, _SP, _Args...>) {
+ std::apply([&](auto&&... __args) { __s_ = _Smart(static_cast<_SP>(__p_), std::forward<_Args>(__args)...); },
+ std::move(__a_));
+ } else {
+ static_assert(is_pointer_v<_Smart> || __resettable_smart_pointer_with_args<_Smart, _Pointer, _Args...> ||
+ is_constructible_v<_Smart, _SP, _Args...>);
+ }
+ }
+
+ _LIBCPP_HIDE_FROM_ABI operator _Pointer*() const noexcept { return std::addressof(const_cast<_Pointer&>(__p_)); }
+
+ _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
+ requires(!is_same_v<_Pointer, void*>)
+ {
+ static_assert(is_pointer_v<_Pointer>);
+
+ return reinterpret_cast<void**>(static_cast<_Pointer*>(*this));
+ }
+
+private:
+ _Smart& __s_;
+ tuple<_Args...> __a_;
+ _Pointer __p_;
+};
+
+template <class _Pointer = void, class _Smart, class... _Args>
+_LIBCPP_HIDE_FROM_ABI auto inout_ptr(_Smart& __s, _Args&&... __args) {
+ using _Ptr = conditional_t<is_void_v<_Pointer>, __pointer_of_t<_Smart>, _Pointer>;
+ return std::inout_ptr_t<_Smart, _Ptr, _Args&&...>{__s, std::forward<_Args>(__args)...};
+}
+
+#endif // _LIBCPP_STD_VER >= 23
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___INOUT_PTR_H
diff --git a/libcxx/include/__memory/out_ptr.h b/libcxx/include/__memory/out_ptr.h
new file mode 100644
index 000000000000000..09c77cbafe69e01
--- /dev/null
+++ b/libcxx/include/__memory/out_ptr.h
@@ -0,0 +1,97 @@
+// -*- 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___OUT_PTR_H
+#define _LIBCPP___OUT_PTR_H
+
+#include <__config>
+#include <__memory/addressof.h>
+#include <__memory/pointer_traits.h>
+#include <__memory/shared_ptr.h>
+#include <__memory/unique_ptr.h>
+#include <__type_traits/is_specialization.h>
+#include <__type_traits/is_void.h>
+#include <tuple>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER >= 23
+
+template <class _Tp>
+concept __resettable_adapted_ptr = requires(_Tp __ptr) { __ptr().reset(); };
+
+template <class _Smart, class _Pointer, class... _Args>
+class _LIBCPP_TEMPLATE_VIS out_ptr_t {
+ static_assert(!__is_specialization_v<_Smart, shared_ptr> || sizeof...(_Args) > 0,
+ "Specialization of std::shared_ptr<> requires a deleter.");
+
+public:
+ _LIBCPP_HIDE_FROM_ABI explicit out_ptr_t(_Smart& __s, _Args... __args)
+ : __s_(__s), __a_(std::forward<_Args>(__args)...), __p_() {
+ using _Ptr = decltype(__s);
+ if constexpr (__resettable_smart_pointer<_Ptr>) {
+ __s_.reset();
+ } else if constexpr (is_constructible_v<_Smart>) {
+ __s_ = _Smart();
+ } else {
+ static_assert(__resettable_smart_pointer<_Ptr> || is_constructible_v<_Smart>);
+ }
+ }
+
+ _LIBCPP_HIDE_FROM_ABI out_ptr_t(const out_ptr_t&) = delete;
+
+ _LIBCPP_HIDE_FROM_ABI ~out_ptr_t() {
+ if (!__p_) {
+ return;
+ }
+
+ using _SP = __pointer_of_or_t<_Smart, _Pointer>;
+ if constexpr (__resettable_smart_pointer_with_args<_Smart, _Pointer, _Args...>) {
+ std::apply([&](auto&&... __args) { __s_.reset(static_cast<_SP>(__p_), std::forward<_Args>(__args)...); },
+ std::move(__a_));
+ } else if constexpr (is_constructible_v<_Smart, _SP, _Args...>) {
+ std::apply([&](auto&&... __args) { __s_ = _Smart(static_cast<_SP>(__p_), std::forward<_Args>(__args)...); },
+ std::move(__a_));
+ } else {
+ static_assert(__resettable_smart_pointer_with_args<_Smart, _Pointer, _Args...> ||
+ is_constructible_v<_Smart, _SP, _Args...>);
+ }
+ }
+
+ _LIBCPP_HIDE_FROM_ABI operator _Pointer*() const noexcept { return std::addressof(const_cast<_Pointer&>(__p_)); }
+
+ _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
+ requires(!is_same_v<_Pointer, void*>)
+ {
+ static_assert(is_pointer_v<_Pointer>);
+
+ return reinterpret_cast<void**>(static_cast<_Pointer*>(*this));
+ }
+
+private:
+ _Smart& __s_;
+ tuple<_Args...> __a_;
+ _Pointer __p_ = _Pointer();
+};
+
+template <class _Pointer = void, class _Smart, class... _Args>
+_LIBCPP_HIDE_FROM_ABI auto out_ptr(_Smart& __s, _Args&&... __args) {
+ using _Ptr = conditional_t<is_void_v<_Pointer>, __pointer_of_t<_Smart>, _Pointer>;
+ return std::out_ptr_t<_Smart, _Ptr, _Args&&...>{__s, std::forward<_Args>(__args)...};
+}
+
+#endif // _LIBCPP_STD_VER >= 23
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___OUT_PTR_H
diff --git a/libcxx/include/__memory/pointer_traits.h b/libcxx/include/__memory/pointer_traits.h
index 7617948ed76bd66..2be0deb50cb8ed1 100644
--- a/libcxx/include/__memory/pointer_traits.h
+++ b/libcxx/include/__memory/pointer_traits.h
@@ -15,11 +15,14 @@
#include <__type_traits/conditional.h>
#include <__type_traits/conjunction.h>
#include <__type_traits/decay.h>
+#include <__type_traits/enable_if.h>
#include <__type_traits/is_class.h>
#include <__type_traits/is_function.h>
#include <__type_traits/is_void.h>
+#include <__type_traits/negation.h>
#include <__type_traits/void_t.h>
#include <__utility/declval.h>
+#include <__utility/forward.h>
#include <cstddef>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -28,11 +31,16 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-template <class _Tp, class = void>
-struct __has_element_type : false_type {};
+// clang-format off
+#define _LIBCPP_CLASS_TRAITS_HAS_XXX(NAME, PROPERTY) \
+ template <class _Tp, class = void> \
+ struct NAME : false_type {}; \
+ template <class _Tp> \
+ struct NAME<_Tp, __void_t<typename _Tp::PROPERTY> > : true_type {}
+// clang-format on
-template <class _Tp>
-struct __has_element_type<_Tp, __void_t<typename _Tp::element_type> > : true_type {};
+_LIBCPP_CLASS_TRAITS_HAS_XXX(__has_pointer, pointer);
+_LIBCPP_CLASS_TRAITS_HAS_XXX(__has_element_type, element_type);
template <class _Ptr, bool = __has_element_type<_Ptr>::value>
struct __pointer_traits_element_type {};
@@ -242,6 +250,57 @@ auto to_address(const _Pointer& __p) noexcept -> decltype(std::__to_address(__p)
}
#endif
+#if _LIBCPP_STD_VER >= 23
+
+template <class _Tp>
+struct __pointer_of {};
+
+template <class _Tp>
+ requires(__has_pointer<_Tp>::value)
+struct __pointer_of<_Tp> {
+ using type = typename _Tp::pointer;
+};
+
+template <class _Tp>
+ requires(!__has_pointer<_Tp>::value && __has_element_type<_Tp>::value)
+struct __pointer_of<_Tp> {
+ using type = typename _Tp::element_type*;
+};
+
+template <class _Tp>
+ requires(!__has_pointer<_Tp>::value && !__has_element_type<_Tp>::value &&
+ __has_element_type<pointer_traits<_Tp>>::value)
+struct __pointer_of<_Tp> {
+ using type = typename pointer_traits<_Tp>::element_type*;
+};
+
+template <typename _Tp>
+using __pointer_of_t = typename __pointer_of<_Tp>::type;
+
+template <class _Tp, class _Up>
+struct __pointer_of_or {
+ using type _LIBCPP_NODEBUG = _Up;
+};
+
+template <class _Tp, class _Up>
+ requires requires { typename __pointer_of_t<_Tp>; }
+struct __pointer_of_or<_Tp, _Up> {
+ using type _LIBCPP_NODEBUG = __pointer_of_t<_Tp>;
+};
+
+template <typename _Tp, typename _Up>
+using __pointer_of_or_t = typename __pointer_of_or<_Tp, _Up>::type;
+
+template <class _Smart>
+concept __resettable_smart_pointer = requires(_Smart __s) { __s.reset(); };
+
+template <class _Smart, class _Pointer = void, class... _Args>
+concept __resettable_smart_pointer_with_args = requires(_Smart __s, _Pointer __p, _Args... __args) {
+ __s.reset(static_cast<__pointer_of_or_t<_Smart, _Pointer>>(__p), std::forward<_Args>(__args)...);
+};
+
+#endif
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___MEMORY_POINTER_TRAITS_H
diff --git a/libcxx/include/memory b/libcxx/include/memory
index 24ba82f43ddd309..d3ab5f73bf00aae 100644
--- a/libcxx/include/memory
+++ b/libcxx/include/memory
@@ -910,6 +910,22 @@ void* align(size_t alignment, size_t size, void*& ptr, size_t& space);
template<size_t N, class T>
[[nodiscard]] constexpr T* assume_aligned(T* ptr); // since C++20
+// [out.ptr.t], class template out_ptr_t
+template<class Smart, class Pointer, class... Args>
+ class out_ptr_t; // since c++23
+
+// [out.ptr], function template out_ptr
+template<class Pointer = void, class Smart, class... Args>
+ auto out_ptr(Smart& s, Args&&... args); // since c++23
+
+// [inout.ptr.t], class template inout_ptr_t
+template<class Smart, class Pointer, class... Args>
+ class inout_ptr_t; // since c++23
+
+// [inout.ptr], function template inout_ptr
+template<class Pointer = void, class Smart, class... Args>
+ auto inout_ptr(Smart& s, Args&&... args); // since c++23
+
} // std
*/
@@ -928,6 +944,8 @@ template<size_t N, class T>
#include <__memory/compressed_pair.h>
#include <__memory/concepts.h>
#include <__memory/construct_at.h>
+#include <__memory/inout_ptr.h>
+#include <__memory/out_ptr.h>
#include <__memory/pointer_traits.h>
#include <__memory/ranges_construct_at.h>
#include <__memory/ranges_uninitialized_algorithms.h>
diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index 90381f4f7f720d4..bb9143b3afa4d27 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -1518,6 +1518,8 @@ module std_private_memory_concepts [system] {
}
module std_private_memory_construct_at [system] { header "__memory/construct_at.h" }
module std_private_memory_destruct_n [system] { header "__memory/destruct_n.h" }
+module std_private_memory_inout_ptr [system] { header "__memory/inout_ptr.h" }
+module std_private_memory_out_ptr [system] { header "__memory/out_ptr.h" }
module std_private_memory_pointer_traits [system] { header "__memory/pointer_traits.h" }
module std_private_memory_ranges_construct_at [system] { header "__memory/ranges_construct_at.h" }
module std_private_memory_ranges_uninitialized_algorithms [system] {
diff --git a/libcxx/include/version b/libcxx/include/version
index ef01af753298222..96a16c8c6381a66 100644
--- a/libcxx/include/version
+++ b/libcxx/include/version
@@ -455,7 +455,7 @@ __cpp_lib_within_lifetime 202306L <type_traits>
// # define __cpp_lib_move_only_function 202110L
# undef __cpp_lib_optional
# define __cpp_lib_optional 202110L
-// # define __cpp_lib_out_ptr 202106L
+# define __cpp_lib_out_ptr 202106L
// # define __cpp_lib_print 202207L
// # define __cpp_lib_ranges_as_const 202207L
# define __cpp_lib_ranges_as_rvalue 202207L
diff --git a/libcxx/modules/std/memory.inc b/libcxx/modules/std/memory.inc
index fba2461732c1b9d..a02fa2b865f57fa 100644
--- a/libcxx/modules/std/memory.inc
+++ b/libcxx/modules/std/memory.inc
@@ -177,17 +177,19 @@ export namespace std {
// [util.smartptr.atomic], atomic smart pointers
// using std::atomic;
+#if _LIBCPP_STD_VER >= 23
// [out.ptr.t], class template out_ptr_t
- // using std::out_ptr_t;
+ using std::out_ptr_t;
// [out.ptr], function template out_ptr
- // using std::out_ptr;
+ using std::out_ptr;
// [inout.ptr.t], class template inout_ptr_t
- // using std::inout_ptr_t;
+ using std::inout_ptr_t;
// [inout.ptr], function template inout_ptr
- // using std::inout_ptr;
+ using std::inout_ptr;
+#endif // _LIBCPP_STD_VER >= 23
#ifndef _LIBCPP_HAS_NO_THREADS
// [depr.util.smartptr.shared.atomic]
diff --git a/libcxx/test/std/language.support/support.limits/support.limits.general/memory.version.c...
[truncated]
|
ldionne
requested changes
Nov 28, 2023
libcxx/test/std/utilities/smartptr/adapt/out.ptr/out.ptr.t.compile.pass.cpp
Outdated
Show resolved
Hide resolved
H-G-Hristov
force-pushed
the
hgh/libcxx/P1132R8-out_ptr-a-scalable-output-pointer-abstraction
branch
from
654ef57
to
cb71ea2
Compare
…utput-pointer-abstraction
ldionne
requested changes
Dec 14, 2023
libcxx/test/std/utilities/smartptr/adapt/out.ptr/out.ptr.general.cpp
Outdated
Show resolved
Hide resolved
Comment on lines
+35
to
+36
| static_assert(!__is_specialization_v<_Smart, shared_ptr> || sizeof...(_Args) > 0, | ||
| "Specialization of std::shared_ptr<> requires a deleter."); |
There was a problem hiding this comment.
I don't think we can have this static_assert, nor the one in inout_ptr_t. That would make us non-conforming, since something like
std::shared_ptr<int> int_ptr;
some_function(std::out_ptr<int*>(int_ptr));
is valid and would end up calling int_ptr.reset(internal-ptr), and then eventually delete internal-ptr in ~shared_ptr, which seems OK? If you agree, please add a test that this works as it should (right now it would fail to compile). The same applies to inout_ptr_t.
There was a problem hiding this comment.
@ldionne After rereading the specs I found these requirements, did we miss them?
My interpretation is that the standard mandates:
template <class _Smart, class _Pointer, class... _Args>
class _LIBCPP_TEMPLATE_VIS out_ptr_t {
static_assert(!__is_specialization_v<_Smart, shared_ptr> || sizeof...(_Args) > 0,
"Specialization of std::shared_ptr<> requires a deleter.");
template <class _Smart, class _Pointer, class... _Args>
class _LIBCPP_TEMPLATE_VIS inout_ptr_t {
static_assert(!__is_specialization_v<_Smart, shared_ptr>, "std::shared_ptr<> is not supported");The other implementations GCC, MSVC also static_assert this condition:
https://godbolt.org/z/sdM5MK98Y
Also implemented similarly in libstdc++ with more clear message than mine:
https://github.com/gcc-mirror/gcc/blob/95b70545331764c85079a1d0e1e19b605bda1456/libstdc%2B%2B-v3/include/bits/out_ptr.h#L57
https://github.com/gcc-mirror/gcc/blob/95b70545331764c85079a1d0e1e19b605bda1456/libstdc%2B%2B-v3/include/bits/out_ptr.h#L296
|
|
||
| _LIBCPP_HIDE_FROM_ABI out_ptr_t(const out_ptr_t&) = delete; | ||
|
|
||
| _LIBCPP_HIDE_FROM_ABI ~out_ptr_t() { |
There was a problem hiding this comment.
I think we need to do something about https://eel.is/c++draft/smartptr#out.ptr.t-7. Otherwise, when we use a shared_ptr here, if we fail to allocate the control block we'll be in trouble, since this is a destructor. So, instead what we could do is allocate a control block in the constructor, then use shared_ptr<T>::__create_with_control_block in the destructor. The simplest way to do this might be to create a specialization for shared_ptr<T>. Do you think that is overkill?
| template <class _Pointer = void, class _Smart, class... _Args> | ||
| _LIBCPP_HIDE_FROM_ABI auto inout_ptr(_Smart& __s, _Args&&... __args) { | ||
| using _Ptr = conditional_t<is_void_v<_Pointer>, __pointer_of_t<_Smart>, _Pointer>; | ||
| return std::inout_ptr_t<_Smart, _Ptr, _Args&&...>(__s, std::forward<_Args>(__args)...); |
There was a problem hiding this comment.
If you do something like
struct Foo {
Foo() = default;
Foo(Foo const&) = delete; // not copyable
Foo(Foo&&) = delete; // not movable either!
};
Foo arg;
std::inout_ptr(smart, arg);
It should work. Your current implementation will work, but it would break if you didn't store reference in your tuple, or if you used std::inout_ptr_t<_Smart, _Ptr, std::decay_t<_Args>...>. Basically, we can add a test like this to ensure that you're storing references in your tuple internally.
Applies to both inout_ptr and out_ptr.
There was a problem hiding this comment.
Maybe I don't understand but the following fails to compile: https://godbolt.org/z/cseMEfdj5
opt/compiler-explorer/gcc-trunk-20240117/include/c++/14.0.1/bits/out_ptr.h:212:42: error: use of deleted function 'constexpr Foo& Foo::operator=(const Foo&)'
212 | _M_smart._M_t._M_deleter() = std::forward<_Del2>(_M_del);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#include <memory>
#include <tuple>
struct Foo {
Foo() = default;
Foo(Foo const&) = delete; // not copyable
Foo(Foo&&) = delete; // not movable either!
void operator()(int*) {};
};
int main() {
std::unique_ptr<int, Foo> smart;
Foo arg;
// std::ignore = std::out_ptr(smart, arg);
std::ignore = std::inout_ptr(smart, arg);
}
libcxx/test/std/utilities/smartptr/adapt/inout.ptr/inout.ptr.general.pass.cpp
Outdated
Show resolved
Hide resolved
…utput-pointer-abstraction
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…utput-pointer-abstraction
…utput-pointer-abstraction
…-pointer-abstraction
H-G-Hristov
changed the title
out_ptr - a scalable output pointer abstraction
…-pointer-abstraction
…-pointer-abstraction
…-pointer-abstraction
…-pointer-abstraction
…-pointer-abstraction
…-pointer-abstraction
…-pointer-abstraction
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.
Differential Revision: https://reviews.llvm.org/D150525
Implements: P1132R8 - 20.3.4 Smart pointer adaptors
To implement: