[libcxx] Add move constructor to compressed pair

[zoecarver]

Summary

This patch adds a move constructor to __compressed_pair so that move-only elements of nested compressed pairs (such as a move-only deleter in a std::shared_ptr) can be move-constructed.

[zoecarver]

• remove unnecessary template

[EricWF]

This test case already passes.

I don't see why this change is necessary.

[zoecarver]

I should have made it clear, my bad, this fixes the move constructor in C++03 mode.

[EricWF]

In D69344#1719825, @zoecarver wrote:

I should have made it clear, my bad, this fixes the move constructor in C++03 mode.

I don't know how much we care to fix that. But if we do, does = default work instead?

[zoecarver]

I would have also expected = default to work but, it doesn't seem to. I think because __compressed_pair inherits from both its elements instead of storing them as members.

Yes, we care to fix that for D62274.

[EricWF]

In D69344#1719855, @zoecarver wrote:

I would have also expected = default to work but, it doesn't seem to. I think because __compressed_pair inherits from both its elements instead of storing them as members.

You need to default it in both bases and in the compressed pair itself.

Yes, we care to fix that for D62274.

I don't think we care about move-only types in C++03.

Also, did you test this change? Because adding a move constructor to compressed pair suppresses the copy constructor and make the type always move only.

[zoecarver]

You need to default it in both bases and in the compressed pair itself.

Yes, you're right. That fixed it.

I don't think we care about move-only types in C++03.

Depends if we want to resolve 2802 in C++03. I think we do, if for no other reason than we don't want to have half a dozen more #ifndef blocks.

Also, did you test this change? Because adding a move constructor to compressed pair suppresses the copy constructor and make the type always move only.

It seemed to pass the memory tests. I also defaulted the copy constructor so, that shouldn't be an issue anymore.

[zoecarver]

• use = default in move and copy constructors

[ldionne]

This sounds reasonable to me, however = default is not going to work in C++03 (unless there's a Clang extension I'm not aware of). @zoecarver Do you still need this?

[zoecarver]

I think I originally created this as a precursor to D62274. But, I don't think it's needed anymore.

libcxx/include/memory

__compressed_pair_elem(piecewise_construct_t, tuple<_Args...> __args,
                          __tuple_indices<_Indexes...>)
       : __value_(_VSTD::forward<_Args>(_VSTD::get<_Indexes>(__args))...) {}
 #else
   _LIBCPP_INLINE_VISIBILITY __compressed_pair_elem() : __value_() {}
   _LIBCPP_INLINE_VISIBILITY
   __compressed_pair_elem(_ParamT __p) : __value_(std::forward<_ParamT>(__p)) {}
 #endif
 
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair_elem(__compressed_pair_elem &&) = default;
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair_elem(__compressed_pair_elem const&) = default;
+
   _LIBCPP_INLINE_VISIBILITY reference __get() _NOEXCEPT { return __value_; }
   _LIBCPP_INLINE_VISIBILITY
   const_reference __get() const _NOEXCEPT { return __value_; }
 
 private:
   _Tp __value_;
 };
 
__compressed_pair_elem(piecewise_construct_t, tuple<_Args...> __args,
       : __value_type(_VSTD::forward<_Args>(_VSTD::get<_Indexes>(__args))...) {}
 #else
   _LIBCPP_INLINE_VISIBILITY __compressed_pair_elem() : __value_type() {}
   _LIBCPP_INLINE_VISIBILITY
   __compressed_pair_elem(_ParamT __p)
       : __value_type(std::forward<_ParamT>(__p)) {}
 #endif
 
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair_elem(__compressed_pair_elem &&) = default;
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair_elem(__compressed_pair_elem const&) = default;
+
   _LIBCPP_INLINE_VISIBILITY reference __get() _NOEXCEPT { return *this; }
   _LIBCPP_INLINE_VISIBILITY
   const_reference __get() const _NOEXCEPT { return *this; }
 };
 
 // Tag used to construct the second element of the compressed pair.
 struct __second_tag {};
 
#else
   __compressed_pair(__second_tag, _T2 __t2)
       : _Base1(), _Base2(_VSTD::forward<_T2>(__t2)) {}
 
   _LIBCPP_INLINE_VISIBILITY
   __compressed_pair(_T1 __t1, _T2 __t2)
       : _Base1(_VSTD::forward<_T1>(__t1)), _Base2(_VSTD::forward<_T2>(__t2)) {}
 #endif
 
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair(__compressed_pair &&) = default;
+
+  _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
+  __compressed_pair(__compressed_pair const&) = default;
+
   _LIBCPP_INLINE_VISIBILITY
   typename _Base1::reference first() _NOEXCEPT {
     return static_cast<_Base1&>(*this).__get();
   }
 
   _LIBCPP_INLINE_VISIBILITY
   typename _Base1::const_reference first() const _NOEXCEPT {
     return static_cast<_Base1 const&>(*this).__get();

libcxx/test/libcxx/utilities/compressed_pair/move_const.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+#include <memory>
+
+#include "test_macros.h"
+
+struct NoCopy {
+  NoCopy(NoCopy const&) = delete;
+  NoCopy() = delete;
+  explicit NoCopy(int) {}
+
+  NoCopy(NoCopy &&) = default;
+};
+
+template<class T>
+struct Foo {
+  std::__compressed_pair<std::__compressed_pair<NoCopy, int>, int> x;
+  Foo(T t, int i) : x(std::__compressed_pair<NoCopy, int>(std::move(t), i), 0) {}
+};
+
+int main(int, char**)
+{
+  Foo<NoCopy> f(NoCopy(0), 1);
+
+  return 0;
+}
 No newline at end of file