[libcxx-commits] [libcxx] [libc++] LWG2899: Constrain move special functions of `tuple` and `unique_ptr` (PR #167211)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Nov 9 01:40:51 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: A. Jiang (frederick-vs-ja)
<details>
<summary>Changes</summary>
libc++'s `tuple`'s move constructor is well-constrained when initially implemented. So this patch only adds test cases.
For `unique_ptr`, its move constructor and move assignment operator were previously unconstrained and thus this patch changes them.
- In C++20 and later, I think it's better to use obviously correct `requires`-clauses.
- In C++17 and earlier, there doesn't seem "obviously correct" approach for constraining. I'm attempting to use `_nat` trick to avoid turning the functions into templates (which are not move special functions).
Some tests case are adjusted because false positive of move-assignability of `unique_ptr` is reduced. Comments are updated to reflect that move-constructibility is not actually required for `unique_ptr`'s deleter.
This patch also explicitly deletes copy functions of `unique_ptr` in all modes. Previously, they are implicitly deleted since C++11 mode, although the standard wording always explicitly deletes them. Clang seems to be somehow buggy on propagating deleted-ness of special member functions from unnamed structs, while the explicit deletion can act as a workaround.
The title of this patch is adjusted to reflect the final resolution of LWG2899.
Fixes #<!-- -->100255.
---
Full diff: https://github.com/llvm/llvm-project/pull/167211.diff
5 Files Affected:
- (modified) libcxx/docs/Status/Cxx20Issues.csv (+1-1)
- (modified) libcxx/include/__memory/unique_ptr.h (+35-11)
- (modified) libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move.pass.cpp (+9-2)
- (modified) libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/move.pass.cpp (+18-6)
- (modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp (+25)
``````````diff
diff --git a/libcxx/docs/Status/Cxx20Issues.csv b/libcxx/docs/Status/Cxx20Issues.csv
index 3d01ff5bbbdfb..f2c6676f5123b 100644
--- a/libcxx/docs/Status/Cxx20Issues.csv
+++ b/libcxx/docs/Status/Cxx20Issues.csv
@@ -143,7 +143,7 @@
"`LWG3180 <https://wg21.link/LWG3180>`__","Inconsistently named return type for ``ranges::minmax_element``\ ","2019-02 (Kona)","|Complete|","15","`#103844 <https://github.com/llvm/llvm-project/issues/103844>`__",""
"`LWG3182 <https://wg21.link/LWG3182>`__","Specification of ``Same``\ could be clearer","2019-02 (Kona)","|Complete|","15","`#103845 <https://github.com/llvm/llvm-project/issues/103845>`__",""
"","","","","","",""
-"`LWG2899 <https://wg21.link/LWG2899>`__","``is_(nothrow_)move_constructible``\ and ``tuple``\ , ``optional``\ and ``unique_ptr``\ ","2019-07 (Cologne)","","","`#100255 <https://github.com/llvm/llvm-project/issues/100255>`__",""
+"`LWG2899 <https://wg21.link/LWG2899>`__","``is_(nothrow_)move_constructible``\ and ``tuple``\ , ``optional``\ and ``unique_ptr``\ ","2019-07 (Cologne)","|Complete|","22","`#100255 <https://github.com/llvm/llvm-project/issues/100255>`__",""
"`LWG3055 <https://wg21.link/LWG3055>`__","``path::operator+=(*single-character*)``\ misspecified","2019-07 (Cologne)","|Complete|","7","`#103846 <https://github.com/llvm/llvm-project/issues/103846>`__",""
"`LWG3158 <https://wg21.link/LWG3158>`__","``tuple(allocator_arg_t, const Alloc&)``\ should be conditionally explicit","2019-07 (Cologne)","|Complete|","10","`#103847 <https://github.com/llvm/llvm-project/issues/103847>`__",""
"`LWG3169 <https://wg21.link/LWG3169>`__","``ranges``\ permutation generators discard useful information","2019-07 (Cologne)","|Complete|","15","`#103848 <https://github.com/llvm/llvm-project/issues/103848>`__",""
diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index eff24546cdc01..f2014440fbc05 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -45,6 +45,7 @@
#include <__type_traits/is_trivially_relocatable.h>
#include <__type_traits/is_unbounded_array.h>
#include <__type_traits/is_void.h>
+#include <__type_traits/nat.h>
#include <__type_traits/remove_extent.h>
#include <__type_traits/type_identity.h>
#include <__utility/declval.h>
@@ -208,9 +209,15 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
template <bool _Dummy = true, class = _EnableIfDeleterConstructible<_BadRValRefType<_Dummy> > >
_LIBCPP_HIDE_FROM_ABI unique_ptr(pointer __p, _BadRValRefType<_Dummy> __d) = delete;
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
- : __ptr_(__u.release()),
- __deleter_(std::forward<deleter_type>(__u.get_deleter())) {}
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
+#if _LIBCPP_STD_VER >= 20
+ unique_ptr(unique_ptr&& __u) noexcept
+ requires is_move_constructible_v<_Dp>
+#else
+ unique_ptr(_If<is_move_constructible<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
+ : __ptr_(__u.release()), __deleter_(std::forward<deleter_type>(__u.get_deleter())) {
+ }
template <class _Up,
class _Ep,
@@ -226,7 +233,14 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
_LIBCPP_HIDE_FROM_ABI unique_ptr(auto_ptr<_Up>&& __p) _NOEXCEPT : __ptr_(__p.release()), __deleter_() {}
#endif
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __u) _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr&
+#if _LIBCPP_STD_VER >= 20
+ operator=(unique_ptr&& __u) noexcept
+ requires is_move_assignable_v<_Dp>
+#else
+ operator=(_If<is_move_assignable<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
+ {
reset(__u.release());
__deleter_ = std::forward<deleter_type>(__u.get_deleter());
return *this;
@@ -251,10 +265,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr {
}
#endif
-#ifdef _LIBCPP_CXX03_LANG
unique_ptr(unique_ptr const&) = delete;
unique_ptr& operator=(unique_ptr const&) = delete;
-#endif
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
@@ -532,12 +544,26 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr<_Tp[], _Dp> {
class = _EnableIfPointerConvertible<_Pp> >
_LIBCPP_HIDE_FROM_ABI unique_ptr(_Pp __ptr, _BadRValRefType<_Dummy> __deleter) = delete;
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23
+#if _LIBCPP_STD_VER >= 20
+ unique_ptr(unique_ptr&& __u) noexcept
+ requires is_move_constructible_v<_Dp>
+#else
+ unique_ptr(_If<is_move_constructible<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
: __ptr_(__u.release()),
__deleter_(std::forward<deleter_type>(__u.get_deleter())),
- __checker_(std::move(__u.__checker_)) {}
+ __checker_(std::move(__u.__checker_)) {
+ }
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr& operator=(unique_ptr&& __u) _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr&
+#if _LIBCPP_STD_VER >= 20
+ operator=(unique_ptr&& __u) noexcept
+ requires is_move_assignable_v<_Dp>
+#else
+ operator=(_If<is_move_assignable<_Dp>::value, unique_ptr&&, __nat> __u) _NOEXCEPT
+#endif
+ {
reset(__u.release());
__deleter_ = std::forward<deleter_type>(__u.get_deleter());
__checker_ = std::move(__u.__checker_);
@@ -564,10 +590,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI unique_ptr<_Tp[], _Dp> {
return *this;
}
-#ifdef _LIBCPP_CXX03_LANG
unique_ptr(unique_ptr const&) = delete;
unique_ptr& operator=(unique_ptr const&) = delete;
-#endif
public:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move.pass.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move.pass.cpp
index 1b1f848e4d587..a6e9872a5b4d2 100644
--- a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move.pass.cpp
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.asgn/move.pass.cpp
@@ -112,19 +112,26 @@ TEST_CONSTEXPR_CXX23 void test_sfinae() {
static_assert(!std::is_assignable<U, const U&&>::value, "");
static_assert(std::is_nothrow_assignable<U, U&&>::value, "");
}
+ {
+ typedef std::unique_ptr<VT, NCDeleter<VT> > U;
+ static_assert(!std::is_assignable<U, U&>::value, "");
+ static_assert(!std::is_assignable<U, const U&>::value, "");
+ static_assert(!std::is_assignable<U, const U&&>::value, "");
+ static_assert(!std::is_assignable<U, U&&>::value, "");
+ }
{
typedef std::unique_ptr<VT, NCDeleter<VT>&> U;
static_assert(!std::is_assignable<U, U&>::value, "");
static_assert(!std::is_assignable<U, const U&>::value, "");
static_assert(!std::is_assignable<U, const U&&>::value, "");
- static_assert(std::is_nothrow_assignable<U, U&&>::value, "");
+ static_assert(!std::is_assignable<U, U&&>::value, "");
}
{
typedef std::unique_ptr<VT, const NCDeleter<VT>&> U;
static_assert(!std::is_assignable<U, U&>::value, "");
static_assert(!std::is_assignable<U, const U&>::value, "");
static_assert(!std::is_assignable<U, const U&&>::value, "");
- static_assert(std::is_nothrow_assignable<U, U&&>::value, "");
+ static_assert(!std::is_assignable<U, U&&>::value, "");
}
}
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/move.pass.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/move.pass.cpp
index 318f4b18a0d1e..097ca4e604711 100644
--- a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/move.pass.cpp
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/move.pass.cpp
@@ -12,6 +12,8 @@
// Test unique_ptr move ctor
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
#include <memory>
#include <utility>
#include <cassert>
@@ -24,10 +26,8 @@
//
// Concerns
// 1 The moved from pointer is empty and the new pointer stores the old value.
-// 2 The only requirement on the deleter is that it is MoveConstructible
-// or a reference.
-// 3 The constructor works for explicitly moved values (i.e. std::move(x))
-// 4 The constructor works for true temporaries (e.g. a return value)
+// 2 The constructor works for explicitly moved values (i.e. std::move(x))
+// 3 The constructor works for true temporaries (e.g. a return value)
//
// Plan
// 1 Explicitly construct unique_ptr<T, D> for various deleter types 'D'.
@@ -73,10 +73,22 @@ void sink3(std::unique_ptr<VT, NCDeleter<VT>&> p) {
template <class ValueT>
TEST_CONSTEXPR_CXX23 void test_sfinae() {
- typedef std::unique_ptr<ValueT> U;
- { // Ensure unique_ptr is non-copyable
+ // Ensure that
+ // - unique_ptr is non-copyable, and
+ // - unique_ptr's move-constructibility is correctly propagated from its deleter's.
+ {
+ typedef std::unique_ptr<ValueT> U;
static_assert((!std::is_constructible<U, U const&>::value), "");
static_assert((!std::is_constructible<U, U&>::value), "");
+ static_assert(std::is_move_constructible<U>::value, "");
+ static_assert(!std::is_constructible<U, const U>::value, "");
+ }
+ {
+ typedef std::unique_ptr<ValueT, NCDeleter<ValueT> > U;
+ static_assert(!std::is_constructible<U, U const&>::value, "");
+ static_assert(!std::is_constructible<U, U&>::value, "");
+ static_assert(!std::is_move_constructible<U>::value, "");
+ static_assert(!std::is_constructible<U, const U>::value, "");
}
}
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp
index d6c192d2e0543..c564d42e51c8e 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/move.pass.cpp
@@ -49,6 +49,19 @@ struct move_only_large final {
int value;
};
+// a non-movable type
+struct nonmovable {
+ nonmovable(const nonmovable&) = default;
+ nonmovable(nonmovable&&) = delete;
+};
+
+// a non-movable type with a usable copy constructor
+// verifying that tuple's move constructor is not confused to select that copy constructor
+struct nonmovable_with_copy_ctor {
+ nonmovable_with_copy_ctor(const nonmovable_with_copy_ctor&) = default;
+ nonmovable_with_copy_ctor(nonmovable_with_copy_ctor&&) = delete;
+};
+
template <class Elem>
void test_sfinae() {
using Tup = std::tuple<Elem>;
@@ -123,6 +136,18 @@ int main(int, char**)
test_sfinae<move_only_ebo>();
test_sfinae<move_only_large>();
}
+ // non-movable types
+ {
+ using Alloc = std::allocator<int>;
+ using Tag = std::allocator_arg_t;
+
+ static_assert(!std::is_move_constructible<nonmovable>::value, "");
+ static_assert(!std::is_constructible<nonmovable, Tag, Alloc, nonmovable>::value, "");
+
+ static_assert(!std::is_move_constructible<nonmovable_with_copy_ctor>::value, "");
+ static_assert(
+ !std::is_constructible<nonmovable_with_copy_ctor, Tag, Alloc, nonmovable_with_copy_ctor>::value, "");
+ }
return 0;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/167211
More information about the libcxx-commits
mailing list