[libc++] Reland LWG2921 and LWG2976 #107960

frederick-vs-ja · 2024-09-10T03:44:41Z

They were originally implemented in d42db7e but reverted later in a2f3c63.

This PR implement both LWG issues again, guarding the removed functions with
_LIBCPP_STD_VER <= 14, because they should be treated as patches for P0302R1 which was adopted for C++17.

Fixes #103598. Fixes #103755.

They were originally implemented in d42db7e but reverted later in a2f3c63. This PR implement both LWG issues again, guarding the removed functions with `_LIBCPP_STD_VER <= 14`, because they should be treated as patches for P0302R1 which was adopted for C++17.

llvmbot · 2024-09-10T03:45:09Z

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

They were originally implemented in d42db7e but reverted later in a2f3c63.

This PR implement both LWG issues again, guarding the removed functions with
_LIBCPP_STD_VER <= 14, because they should be treated as patches for P0302R1 which was adopted for C++17.

Fixes #103598. Fixes #103755.

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

4 Files Affected:

(modified) libcxx/docs/Status/Cxx17Issues.csv (+1-1)
(modified) libcxx/docs/Status/Cxx20Issues.csv (+1-1)
(modified) libcxx/include/future (+12-2)
(modified) libcxx/test/std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp (+14-5)

diff --git a/libcxx/docs/Status/Cxx17Issues.csv b/libcxx/docs/Status/Cxx17Issues.csv
index 7119382eb5cfb4..af3dee9ca50c98 100644
--- a/libcxx/docs/Status/Cxx17Issues.csv
+++ b/libcxx/docs/Status/Cxx17Issues.csv
@@ -306,7 +306,7 @@
 "`LWG2905 <https://wg21.link/LWG2905>`__","is_constructible_v<unique_ptr<P, D>, P, D const &> should be false when D is not copy constructible","2017-02 (Kona)","|Complete|","",""
 "`LWG2908 <https://wg21.link/LWG2908>`__","The less-than operator for shared pointers could do more","2017-02 (Kona)","|Complete|","",""
 "`LWG2911 <https://wg21.link/LWG2911>`__","An is_aggregate type trait is needed","2017-02 (Kona)","|Complete|","",""
-"`LWG2921 <https://wg21.link/LWG2921>`__","packaged_task and type-erased allocators","2017-02 (Kona)","|Complete|","",""
+"`LWG2921 <https://wg21.link/LWG2921>`__","packaged_task and type-erased allocators","2017-02 (Kona)","|Complete|","20.0","Originally implemented in LLVM 6.0 but reverted later. Old documentation incorrectly said it was implemented."
 "`LWG2934 <https://wg21.link/LWG2934>`__","optional<const T> doesn't compare with T","2017-02 (Kona)","|Complete|","",""
 "","","","","",""
 "`LWG2901 <https://wg21.link/LWG2901>`__","Variants cannot properly support allocators","2017-07 (Toronto)","|Complete|","",""
diff --git a/libcxx/docs/Status/Cxx20Issues.csv b/libcxx/docs/Status/Cxx20Issues.csv
index e5d2498473ecde..8c4224616d73e6 100644
--- a/libcxx/docs/Status/Cxx20Issues.csv
+++ b/libcxx/docs/Status/Cxx20Issues.csv
@@ -27,7 +27,7 @@
 "`LWG2964 <https://wg21.link/LWG2964>`__","Apparently redundant requirement for dynamic_pointer_cast","2017-11 (Albuquerque)","","",""
 "`LWG2965 <https://wg21.link/LWG2965>`__","Non-existing path::native_string() in filesystem_error::what() specification","2017-11 (Albuquerque)","|Nothing To Do|","",""
 "`LWG2972 <https://wg21.link/LWG2972>`__","What is ``is_trivially_destructible_v<int>``\ ?","2017-11 (Albuquerque)","|Complete|","",""
-"`LWG2976 <https://wg21.link/LWG2976>`__","Dangling uses_allocator specialization for packaged_task","2017-11 (Albuquerque)","|Complete|","",""
+"`LWG2976 <https://wg21.link/LWG2976>`__","Dangling uses_allocator specialization for packaged_task","2017-11 (Albuquerque)","|Complete|","20.0","Originally implemented in LLVM 6.0 but reverted later. Old documentation incorrectly said it was implemented."
 "`LWG2977 <https://wg21.link/LWG2977>`__","unordered_meow::merge() has incorrect Throws: clause","2017-11 (Albuquerque)","|Nothing To Do|","",""
 "`LWG2978 <https://wg21.link/LWG2978>`__","Hash support for pmr::string and friends","2017-11 (Albuquerque)","|Complete|","16.0",""
 "`LWG2979 <https://wg21.link/LWG2979>`__","aligned_union should require complete object types","2017-11 (Albuquerque)","|Complete|","",""
diff --git a/libcxx/include/future b/libcxx/include/future
index 01c0b10172cd3b..b52bcc7e25452f 100644
--- a/libcxx/include/future
+++ b/libcxx/include/future
@@ -329,7 +329,7 @@ public:
     template <class F>
         explicit packaged_task(F&& f);
     template <class F, class Allocator>
-        packaged_task(allocator_arg_t, const Allocator& a, F&& f);
+        packaged_task(allocator_arg_t, const Allocator& a, F&& f);              // removed in C++17
     ~packaged_task();
 
     // no copy
@@ -356,7 +356,7 @@ public:
 template <class R>
   void swap(packaged_task<R(ArgTypes...)&, packaged_task<R(ArgTypes...)>&) noexcept;
 
-template <class R, class Alloc> struct uses_allocator<packaged_task<R>, Alloc>;
+template <class R, class Alloc> struct uses_allocator<packaged_task<R>, Alloc>; // removed in C++17
 
 }  // std
 
@@ -1460,8 +1460,10 @@ public:
   _LIBCPP_HIDE_FROM_ABI __packaged_task_function() _NOEXCEPT : __f_(nullptr) {}
   template <class _Fp>
   _LIBCPP_HIDE_FROM_ABI __packaged_task_function(_Fp&& __f);
+#  if _LIBCPP_STD_VER <= 14
   template <class _Fp, class _Alloc>
   _LIBCPP_HIDE_FROM_ABI __packaged_task_function(allocator_arg_t, const _Alloc& __a, _Fp&& __f);
+#  endif
 
   _LIBCPP_HIDE_FROM_ABI __packaged_task_function(__packaged_task_function&&) _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI __packaged_task_function& operator=(__packaged_task_function&&) _NOEXCEPT;
@@ -1507,6 +1509,7 @@ __packaged_task_function<_Rp(_ArgTypes...)>::__packaged_task_function(_Fp&& __f)
   }
 }
 
+#  if _LIBCPP_STD_VER <= 14
 template <class _Rp, class... _ArgTypes>
 template <class _Fp, class _Alloc>
 __packaged_task_function<_Rp(_ArgTypes...)>::__packaged_task_function(allocator_arg_t, const _Alloc& __a0, _Fp&& __f)
@@ -1525,6 +1528,7 @@ __packaged_task_function<_Rp(_ArgTypes...)>::__packaged_task_function(allocator_
     __f_ = std::addressof(*__hold.release());
   }
 }
+#  endif
 
 template <class _Rp, class... _ArgTypes>
 __packaged_task_function<_Rp(_ArgTypes...)>&
@@ -1606,9 +1610,11 @@ public:
   template <class _Fp, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, packaged_task>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI explicit packaged_task(_Fp&& __f) : __f_(std::forward<_Fp>(__f)) {}
 
+#  if _LIBCPP_STD_VER <= 14
   template <class _Fp, class _Allocator, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, packaged_task>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI packaged_task(allocator_arg_t, const _Allocator& __a, _Fp&& __f)
       : __f_(allocator_arg_t(), __a, std::forward<_Fp>(__f)), __p_(allocator_arg_t(), __a) {}
+#  endif
   // ~packaged_task() = default;
 
   // no copy
@@ -1696,9 +1702,11 @@ public:
   _LIBCPP_HIDE_FROM_ABI packaged_task() _NOEXCEPT : __p_(nullptr) {}
   template <class _Fp, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, packaged_task>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI explicit packaged_task(_Fp&& __f) : __f_(std::forward<_Fp>(__f)) {}
+#  if _LIBCPP_STD_VER <= 14
   template <class _Fp, class _Allocator, __enable_if_t<!is_same<__remove_cvref_t<_Fp>, packaged_task>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI packaged_task(allocator_arg_t, const _Allocator& __a, _Fp&& __f)
       : __f_(allocator_arg_t(), __a, std::forward<_Fp>(__f)), __p_(allocator_arg_t(), __a) {}
+#  endif
   // ~packaged_task() = default;
 
   // no copy
@@ -1790,8 +1798,10 @@ swap(packaged_task<_Rp(_ArgTypes...)>& __x, packaged_task<_Rp(_ArgTypes...)>& __
   __x.swap(__y);
 }
 
+#  if _LIBCPP_STD_VER <= 14
 template <class _Callable, class _Alloc>
 struct _LIBCPP_TEMPLATE_VIS uses_allocator<packaged_task<_Callable>, _Alloc> : public true_type {};
+#  endif
 
 template <class _Rp, class _Fp>
 _LIBCPP_HIDE_FROM_ABI future<_Rp> __make_deferred_assoc_state(_Fp&& __f) {
diff --git a/libcxx/test/std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp b/libcxx/test/std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp
index 3ab59909cfafbe..a3bdd45975c96f 100644
--- a/libcxx/test/std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp
+++ b/libcxx/test/std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp
@@ -18,11 +18,13 @@
 
 #include <cassert>
 #include <future>
+#include <memory>
+#include <type_traits>
 
 #include "test_allocator.h"
 
 struct A {};
-using PT = std::packaged_task<A(int, char)>;
+using PT  = std::packaged_task<A(int, char)>;
 using VPT = volatile std::packaged_task<A(int, char)>;
 
 static_assert(!std::is_constructible<PT, std::allocator_arg_t, test_allocator<A>, VPT>::value, "");
@@ -35,7 +37,14 @@ static_assert(!std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>
 static_assert(!std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, volatile PA&>::value, "");
 static_assert(!std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, volatile PA&&>::value, "");
 
-static_assert( std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, const PI&>::value, "");
-static_assert( std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, const PI&&>::value, "");
-static_assert( std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, volatile PI&>::value, "");
-static_assert( std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, volatile PI&&>::value, "");
+#if TEST_STD_VER >= 17 // packaged_task allocator support was removed in C++17 (LWG 2921)
+static_assert(!std::is_constructible_v<PA, std::allocator_arg_t, std::allocator<A>, const PI&>);
+static_assert(!std::is_constructible_v<PA, std::allocator_arg_t, std::allocator<A>, const PI&&>);
+static_assert(!std::is_constructible_v<PA, std::allocator_arg_t, std::allocator<A>, volatile PI&>);
+static_assert(!std::is_constructible_v<PA, std::allocator_arg_t, std::allocator<A>, volatile PI&&>);
+#else
+static_assert(std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, const PI&>::value, "");
+static_assert(std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, const PI&&>::value, "");
+static_assert(std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, volatile PI&>::value, "");
+static_assert(std::is_constructible<PA, std::allocator_arg_t, std::allocator<A>, volatile PI&&>::value, "");
+#endif

Zingam · 2024-09-10T04:55:00Z

They were originally implemented in d42db7e but reverted later in a2f3c63.

This PR implement both LWG issues again, guarding the removed functions with _LIBCPP_STD_VER <= 14, because they should be treated as patches for P0302R1 which was adopted for C++17.

Fixes #103598. Fixes #103755.

#103598 (comment)

Should these issues be reopened first before landing?

ldionne

LGTM with minor comment. Thanks for spotting this!

libcxx/include/future

frederick-vs-ja requested a review from a team as a code owner September 10, 2024 03:44

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

ldionne approved these changes Sep 10, 2024

View reviewed changes

libcxx/include/future Outdated Show resolved Hide resolved

libcxx/include/future Outdated Show resolved Hide resolved

Unguard implementation details

5a7cec4

ldionne merged commit 3a0ef2a into llvm:main Sep 11, 2024
63 checks passed

frederick-vs-ja deleted the lwg-2921-2976 branch September 11, 2024 23:51

frederick-vs-ja mentioned this pull request Sep 29, 2024

[libc++] packaged_task(std::allocator_arg_t, const Allocator&, F&&) should be gone in C++17 and later #53912

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Reland LWG2921 and LWG2976 #107960

[libc++] Reland LWG2921 and LWG2976 #107960

Uh oh!

frederick-vs-ja commented Sep 10, 2024

Uh oh!

llvmbot commented Sep 10, 2024

Uh oh!

Zingam commented Sep 10, 2024

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[libc++] Reland LWG2921 and LWG2976 #107960

[libc++] Reland LWG2921 and LWG2976 #107960

Uh oh!

Conversation

frederick-vs-ja commented Sep 10, 2024

Uh oh!

llvmbot commented Sep 10, 2024

Uh oh!

Zingam commented Sep 10, 2024

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!