Implement P3503R3 Make Type-Erased Allocator Use In promise And packaged_task Consistent (#5630)

Co-authored-by: Stephan T. Lavavej <[email protected]>

Author: frederick-vs-ja

stl/inc/functional

@@ -719,7 +719,7 @@ constexpr bool _Testable_callable_v =
 template <class _Ty>
 bool _Test_callable(const _Ty& _Arg) noexcept { // determine whether std::function must store _Arg
     if constexpr (_Testable_callable_v<_Ty>) {
-        return !!_Arg;
+        return static_cast<bool>(_Arg);
     } else {
         return true;
     }
@@ -755,7 +755,6 @@ template <class _Impl> // determine whether _Impl must be dynamically allocated
 constexpr bool _Is_large = sizeof(_Impl) > _Space_size || alignof(_Impl) > alignof(max_align_t)
                         || !_Impl::_Nothrow_move::value;
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
 template <class _Callable, class _Alloc, class _Rx, class... _Types>
 class _Func_impl final : public _Func_base<_Rx, _Types...> {
     // derived class for specific implementation types that use allocators
@@ -834,7 +833,6 @@ private:
 
     _Compressed_pair<_Alloc, _Callable> _Mypair;
 };
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
 template <class _Ty, class... _Types>
 _Ty* _Global_new(_Types&&... _Args) { // acts as "new" while disallowing user overload selection
@@ -989,7 +987,6 @@ protected:
         }
     }
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
     template <class _Fx, class _Alloc>
     void _Reset_alloc(_Fx&& _Val, const _Alloc& _Ax) { // store copy of _Val with allocator
         if (!_STD _Test_callable(_Val)) { // null member pointer/function pointer/std::function
@@ -1012,7 +1009,6 @@ protected:
             _Set(_Ptr);
         }
     }
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     void _Tidy() noexcept {
         if (!_Empty()) { // destroy callable object and maybe delete it
@@ -1141,6 +1137,7 @@ public:
             "The target function object type must be copy constructible (N4140 [func.wrap.func.con]/7).");
         this->_Reset_alloc(_STD forward<_Fx>(_Func), _Ax);
     }
+#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     template <class _SecretTag, class _Fx, class _Alloc,
         enable_if_t<is_same_v<_SecretTag, _Secret_copyability_ignoring_tag>, int> = 0,
@@ -1149,7 +1146,6 @@ public:
         // used exclusively for packaged_task
         this->_Reset_alloc(_STD forward<_Fx>(_Func), _Ax);
     }
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     function& operator=(const function& _Right) {
         function(_Right).swap(*this);
@@ -1160,11 +1156,16 @@ public:
         this->_Reset_move(_STD move(_Right));
     }
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
-    template <class _Alloc>
-    function(allocator_arg_t, const _Alloc& _Al, function&& _Right) {
+    template <class _SecretTag, class _Alloc,
+        enable_if_t<is_same_v<_SecretTag, _Secret_copyability_ignoring_tag>, int> = 0>
+    explicit function(_SecretTag, allocator_arg_t, const _Alloc& _Al, function&& _Right) {
         this->_Reset_alloc(_STD move(_Right), _Al);
     }
+
+#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
+    template <class _Alloc>
+    function(allocator_arg_t, const _Alloc& _Al, function&& _Right)
+        : function(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD move(_Right)) {}
 #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     function& operator=(function&& _Right) noexcept /* strengthened */ {

stl/inc/future

@@ -187,6 +187,25 @@ struct _State_deleter : _Deleter_base<_Ty> { // manage allocator and deletion st
     _Alloc _My_alloc;
 };
 
+template <class _Ty>
+struct __declspec(novtable) _Clonable_deleter_base : _Deleter_base<_Ty> {
+    // TRANSITION, ABI, should be fused into _Deleter_base
+    virtual _Associated_state<_Ty>* _Move_clone(_Associated_state<_Ty>&&) = 0;
+};
+
+template <class _Ty, class _Derived, class _Alloc>
+struct _State_deleter_v2 : _Clonable_deleter_base<_Ty> { // manage allocator and deletion state objects
+    _State_deleter_v2(const _Alloc& _Al) : _My_alloc(_Al) {}
+
+    _State_deleter_v2(const _State_deleter_v2&)            = delete;
+    _State_deleter_v2& operator=(const _State_deleter_v2&) = delete;
+
+    void _Delete(_Associated_state<_Ty>* _State) noexcept override;
+    _Associated_state<_Ty>* _Move_clone(_Associated_state<_Ty>&& _Src) override;
+
+    _Alloc _My_alloc;
+};
+
 template <class _Ty>
 union _Result_holder {
     _Result_holder() noexcept {}
@@ -389,6 +408,10 @@ public:
         }
     }
 
+    _Mydel* _Get_deleter() const noexcept { // get deleter for cloning
+        return _Deleter;
+    }
+
 protected:
     void _Maybe_run_deferred_function(unique_lock<mutex>& _Lock) { // run a deferred function if not already done
         if (!_Running) { // run the function
@@ -495,19 +518,13 @@ public:
     template <class _Fty2, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _Function_type>, int> = 0>
     explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {}
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
-    template <class _Alloc>
-    _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
-        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {}
-
     template <class _Alloc>
     _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
-        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {}
+        : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD move(_Fnarg)) {}
 
     template <class _Fty2, class _Alloc, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _Function_type>, int> = 0>
     _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
         : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {}
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     void _Call_deferred(_ArgTypes... _Args) { // set deferred call
         _TRY_BEGIN
@@ -543,10 +560,7 @@ public:
         _CATCH_END
     }
 
-    const auto& _Get_fn() const& {
-        return _Fn;
-    }
-    auto&& _Get_fn() && noexcept {
+    _Function_type&& _Get_fn() && noexcept {
         return _STD move(_Fn);
     }
 
@@ -569,11 +583,14 @@ _Associated_state<_Ty>* _Make_associated_state(const _Alloc& _Al) {
     return _STD _Unfancy(_Res.release()); // ownership transferred to caller
 }
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
 template <class _Pack_state, class _Fty2, class _Alloc>
 _Pack_state* _Make_packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al) {
     // construct a _Packaged_state object with an allocator from an rvalue function object
-    using _Delty   = _State_deleter<typename _Pack_state::_Mybase::_State_type, _Pack_state, _Alloc>;
+#ifdef _CPPRTTI // TRANSITION, ABI, should not rely on RTTI
+    using _Delty = _State_deleter_v2<typename _Pack_state::_Mybase::_State_type, _Pack_state, _Alloc>;
+#else // ^^^ defined(_CPPRTTI) / !defined(_CPPRTTI) vvv
+    using _Delty = _State_deleter<typename _Pack_state::_Mybase::_State_type, _Pack_state, _Alloc>;
+#endif // ^^^ !defined(_CPPRTTI) ^^^
     using _Aldelty = _Rebind_alloc_t<_Alloc, _Delty>;
     using _Alstate = _Rebind_alloc_t<_Alloc, _Pack_state>;
 
@@ -584,7 +601,26 @@ _Pack_state* _Make_packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al) {
     (void) _Del.release(); // ownership of _Del.get() now transferred to _Res
     return _STD _Unfancy(_Res.release()); // ownership transferred to caller
 }
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
+
+template <class _Ty, class _Derived, class _Alloc>
+void _State_deleter_v2<_Ty, _Derived, _Alloc>::_Delete(_Associated_state<_Ty>* _State) noexcept {
+    // delete _State and this using stored allocator
+    using _State_allocator = _Rebind_alloc_t<_Alloc, _Derived>;
+    _State_allocator _St_alloc(_My_alloc);
+
+    using _Deleter_allocator = _Rebind_alloc_t<_Alloc, _State_deleter_v2>;
+    _Deleter_allocator _Del_alloc(_My_alloc);
+
+    _Derived* _Ptr = static_cast<_Derived*>(_State);
+
+    _STD _Delete_plain_internal(_St_alloc, _Ptr);
+    _STD _Delete_plain_internal(_Del_alloc, this);
+}
+
+template <class _Ty, class _Derived, class _Alloc>
+_Associated_state<_Ty>* _State_deleter_v2<_Ty, _Derived, _Alloc>::_Move_clone(_Associated_state<_Ty>&& _Src) {
+    return _STD _Make_packaged_state<_Derived>(static_cast<_Derived&&>(_Src)._Get_fn(), _My_alloc);
+}
 
 template <class _Rx>
 class _Deferred_async_state : public _Packaged_state<_Rx()> {
@@ -1185,9 +1221,6 @@ private:
     _Promise<int> _MyPromise;
 };
 
-template <class _Ty, class _Alloc>
-struct uses_allocator<promise<_Ty>, _Alloc> : true_type {};
-
 _EXPORT_STD template <class _Ty>
 void swap(promise<_Ty>& _Left, promise<_Ty>& _Right) noexcept {
     _Left.swap(_Right);
@@ -1211,21 +1244,19 @@ public:
     template <class _Fty2, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, packaged_task>, int> = 0>
     explicit packaged_task(_Fty2&& _Fnarg) : _MyPromise(new _MyStateType(_STD forward<_Fty2>(_Fnarg))) {
         static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value,
-            "The function object must be callable with _ArgTypes... and return _Ret (N4988 [futures.task.members]/3).");
+            "The function object must be callable with _ArgTypes... and return _Ret (N5014 [futures.task.members]/4).");
     }
 
     packaged_task(packaged_task&&) noexcept = default;
 
     packaged_task& operator=(packaged_task&&) noexcept = default;
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
     template <class _Fty2, class _Alloc, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, packaged_task>, int> = 0>
-    packaged_task(allocator_arg_t, const _Alloc& _Al, _Fty2&& _Fnarg)
+    explicit packaged_task(allocator_arg_t, const _Alloc& _Al, _Fty2&& _Fnarg)
         : _MyPromise(_STD _Make_packaged_state<_MyStateType>(_STD forward<_Fty2>(_Fnarg), _Al)) {
         static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value,
-            "The function object must be callable with _ArgTypes... and return _Ret (N4140 [futures.task.members]/2).");
+            "The function object must be callable with _ArgTypes... and return _Ret (N5014 [futures.task.members]/4).");
     }
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     ~packaged_task() noexcept {
         _MyPromise._Get_state()._Abandon();
@@ -1270,7 +1301,20 @@ public:
     void reset() { // reset to newly constructed state
         _MyStateManagerType& _State_mgr = _MyPromise._Get_state_for_set();
         _MyStateType& _MyState          = *static_cast<_MyStateType*>(_State_mgr._Ptr());
-        _MyPromiseType _New_promise(new _MyStateType(_STD move(_MyState)._Get_fn()));
+
+        const auto _New_state_ptr = [&_MyState]() -> _Associated_state<_P_arg_type_t<_Ret>>* {
+#ifdef _CPPRTTI // TRANSITION, ABI, should not rely on RTTI
+            using _Clonable_deleter_t = _Clonable_deleter_base<_P_arg_type_t<_Ret>>;
+            if (const auto _Clonable_deleter = dynamic_cast<_Clonable_deleter_t*>(_MyState._Get_deleter())) {
+                return _Clonable_deleter->_Move_clone(_STD move(_MyState));
+            } else {
+                return new _MyStateType(_STD move(_MyState)._Get_fn());
+            }
+#else // ^^^ defined(_CPPRTTI) / !defined(_CPPRTTI) vvv
+            return new _MyStateType(_STD move(_MyState)._Get_fn());
+#endif // ^^^ !defined(_CPPRTTI) ^^^
+        }();
+        _MyPromiseType _New_promise{_New_state_ptr};
         _MyPromise._Get_state()._Abandon();
         _MyPromise._Swap(_New_promise);
     }
@@ -1294,11 +1338,6 @@ template <class _Fx>
 packaged_task(_Fx) -> packaged_task<typename _Deduce_signature<_Fx>::type>;
 #endif // _HAS_CXX17
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
-template <class _Ty, class _Alloc>
-struct uses_allocator<packaged_task<_Ty>, _Alloc> : true_type {};
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
-
 _EXPORT_STD template <class _Ty>
 void swap(packaged_task<_Ty>& _Left, packaged_task<_Ty>& _Right) noexcept {
     _Left.swap(_Right);

stl/inc/yvals_core.h

@@ -83,6 +83,7 @@
 // P3223R2 Making istream::ignore() Less Surprising
 // P3323R1 Forbid atomic<cv T>, Specify atomic_ref<cv T>
 //     (for atomic<cv T>)
+// P3503R3 Make Type-Erased Allocator Use In promise And packaged_task Consistent
 
 // _HAS_CXX17 directly controls:
 // P0005R4 not_fn()
@@ -153,8 +154,6 @@
 // P0298R3 std::byte
 // P0302R1 Removing Allocator Support In std::function
 // LWG-2385 function::assign allocator argument doesn't make sense
-// LWG-2921 packaged_task and type-erased allocators
-// LWG-2976 Dangling uses_allocator specialization for packaged_task
 // Enforcement of matching allocator value_types
 
 // _HAS_CXX17 and _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS control:
@@ -1032,8 +1031,6 @@ _EMIT_STL_ERROR(STL1004, "C++98 unexpected() is incompatible with C++23 unexpect
 
 // P0302R1 Removing Allocator Support In std::function
 // LWG-2385 function::assign allocator argument doesn't make sense
-// LWG-2921 packaged_task and type-erased allocators
-// LWG-2976 Dangling uses_allocator specialization for packaged_task
 #ifndef _HAS_FUNCTION_ALLOCATOR_SUPPORT
 #define _HAS_FUNCTION_ALLOCATOR_SUPPORT (!_HAS_CXX17)
 #endif // !defined(_HAS_FUNCTION_ALLOCATOR_SUPPORT)

tests/libcxx/expected_results.txt

@@ -145,6 +145,10 @@ std/language.support/support.limits/support.limits.general/cstring.version.compi
 # libc++ has not implemented P3323R1: "Forbid atomic<cv T>, Specify atomic_ref<cv T>"
 std/atomics/atomics.ref/member_types.compile.pass.cpp FAIL
 
+# libc++ has not implemented P3503R3 "Make Type-Erased Allocator Use In promise And packaged_task Consistent"
+std/thread/futures/futures.promise/uses_allocator.pass.cpp FAIL
+std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp FAIL
+
 # Various bogosity (LLVM-D141004)
 std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/ctor_does_not_allocate.pass.cpp FAIL
 std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/sync_with_default_resource.pass.cpp FAIL

tests/std/test.lst

@@ -721,6 +721,7 @@ tests\P2693R1_text_formatting_header_thread
 tests\P2693R1_text_formatting_stacktrace
 tests\P2693R1_text_formatting_thread_id
 tests\P3107R5_enabled_specializations
+tests\P3503R3_packaged_task_promise_with_allocator
 tests\VSO_0000000_allocator_propagation
 tests\VSO_0000000_any_calling_conventions
 tests\VSO_0000000_c_math_functions

tests/std/tests/Dev10_561430_list_and_tree_leaks/test.cpp

@@ -157,7 +157,6 @@ int main() {
         f.get();
     }
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
     {
         packaged_task<int()> pt(allocator_arg, Mallocator<int>(), [] { return 1234; });
 
@@ -200,7 +199,6 @@ int main() {
 
         f.get();
     }
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 #endif // _M_CEE_PURE
 
     assert(g_mallocs == 0);

tests/std/tests/Dev11_0920385_list_sort_allocator/test.cpp

@@ -254,7 +254,6 @@ int main() {
         f.get();
     }
 
-#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
     {
         packaged_task<int()> pt(allocator_arg, alloc, [] { return 1234; });
         future<int> f = pt.get_future();
@@ -276,7 +275,6 @@ int main() {
         pt();
         f.get();
     }
-#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 #endif // _M_CEE_PURE
 
     test_DevDiv_1119194();

tests/std/tests/GH_000140_adl_proof_construction/test.compile.pass.cpp

@@ -130,13 +130,11 @@ void test_packaged_task() {
     packaged_task<void(int)>{validating_identity{}};
     packaged_task<void(int)>{validating_large_identity{}};
 
-#if !_HAS_CXX17
     packaged_task<void(validator)>{allocator_arg, adl_proof_allocator<unsigned char>{}, simple_identity{}};
     packaged_task<void(validator)>{allocator_arg, adl_proof_allocator<unsigned char>{}, simple_large_identity{}};
 
     packaged_task<void(int)>{allocator_arg, adl_proof_allocator<unsigned char>{}, validating_identity{}};
     packaged_task<void(int)>{allocator_arg, adl_proof_allocator<unsigned char>{}, validating_large_identity{}};
-#endif // !_HAS_CXX17
 }
 
 void test_promise() {

tests/std/tests/P3503R3_packaged_task_promise_with_allocator/env.lst

@@ -0,0 +1,4 @@
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+RUNALL_INCLUDE ..\impure_matrix.lst