Skip to content

Commit a0839b1

Browse files
ldionneldionne
committed
[libc++] Fix tuple assignment from types derived from a tuple-like
The implementation of tuple's constructors and assignment operators currently diverges from the way the Standard specifies them, which leads to subtle cases where the behavior is not as specified. In particular, a class derived from a tuple-like type (e.g. pair) can't be assigned to a tuple with corresponding members, when it should. This commit re-implements the assignment operators (BUT NOT THE CONSTRUCTORS) in a way much closer to the specification to get rid of this bug. Most of the tests have been stolen from Eric's patch https://reviews.llvm.org/D27606. As a fly-by improvement, tests for noexcept correctness have been added to all overloads of operator=. We should tackle the same issue for the tuple constructors in a future patch - I'm just trying to make progress on fixing this long-standing bug. PR17550 rdar://15837420 Differential Revision: https://reviews.llvm.org/D50106
1 parent 946a099 commit a0839b1

12 files changed

+725
-90
lines changed

‎libcxx/include/tuple

+130-73
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ public:
5555
explicit(see-below) tuple(allocator_arg_t, const Alloc& a, pair<U1, U2>&&);
5656
5757
tuple& operator=(const tuple&);
58-
tuple&
59-
operator=(tuple&&) noexcept(AND(is_nothrow_move_assignable<T>::value ...));
58+
tuple& operator=(tuple&&) noexcept(is_nothrow_move_assignable_v<T> && ...);
6059
template <class... U>
6160
tuple& operator=(const tuple<U...>&);
6261
template <class... U>
@@ -66,6 +65,11 @@ public:
6665
template <class U1, class U2>
6766
tuple& operator=(pair<U1, U2>&&); // iff sizeof...(T) == 2
6867
68+
template<class U, size_t N>
69+
tuple& operator=(array<U, N> const&) // iff sizeof...(T) == N, EXTENSION
70+
template<class U, size_t N>
71+
tuple& operator=(array<U, N>&&) // iff sizeof...(T) == N, EXTENSION
72+
6973
void swap(tuple&) noexcept(AND(swap(declval<T&>(), declval<T&>())...));
7074
};
7175
@@ -257,15 +261,6 @@ public:
257261
__tuple_leaf(const __tuple_leaf& __t) = default;
258262
__tuple_leaf(__tuple_leaf&& __t) = default;
259263

260-
template <class _Tp>
261-
_LIBCPP_INLINE_VISIBILITY
262-
__tuple_leaf&
263-
operator=(_Tp&& __t) _NOEXCEPT_((is_nothrow_assignable<_Hp&, _Tp>::value))
264-
{
265-
__value_ = _VSTD::forward<_Tp>(__t);
266-
return *this;
267-
}
268-
269264
_LIBCPP_INLINE_VISIBILITY
270265
int swap(__tuple_leaf& __t) _NOEXCEPT_(__is_nothrow_swappable<__tuple_leaf>::value)
271266
{
@@ -331,15 +326,6 @@ public:
331326
__tuple_leaf(__tuple_leaf const &) = default;
332327
__tuple_leaf(__tuple_leaf &&) = default;
333328

334-
template <class _Tp>
335-
_LIBCPP_INLINE_VISIBILITY
336-
__tuple_leaf&
337-
operator=(_Tp&& __t) _NOEXCEPT_((is_nothrow_assignable<_Hp&, _Tp>::value))
338-
{
339-
_Hp::operator=(_VSTD::forward<_Tp>(__t));
340-
return *this;
341-
}
342-
343329
_LIBCPP_INLINE_VISIBILITY
344330
int
345331
swap(__tuple_leaf& __t) _NOEXCEPT_(__is_nothrow_swappable<__tuple_leaf>::value)
@@ -429,49 +415,30 @@ struct _LIBCPP_DECLSPEC_EMPTY_BASES __tuple_impl<__tuple_indices<_Indx...>, _Tp.
429415
typename __make_tuple_types<_Tuple>::type>::type>(_VSTD::get<_Indx>(__t)))...
430416
{}
431417

432-
template <class _Tuple>
433-
_LIBCPP_INLINE_VISIBILITY
434-
typename enable_if
435-
<
436-
__tuple_assignable<_Tuple, tuple<_Tp...> >::value,
437-
__tuple_impl&
438-
>::type
439-
operator=(_Tuple&& __t) _NOEXCEPT_((__all<is_nothrow_assignable<_Tp&, typename tuple_element<_Indx,
440-
typename __make_tuple_types<_Tuple>::type>::type>::value...>::value))
441-
{
442-
__swallow(__tuple_leaf<_Indx, _Tp>::operator=(_VSTD::forward<typename tuple_element<_Indx,
443-
typename __make_tuple_types<_Tuple>::type>::type>(_VSTD::get<_Indx>(__t)))...);
444-
return *this;
445-
}
446-
447418
__tuple_impl(const __tuple_impl&) = default;
448419
__tuple_impl(__tuple_impl&&) = default;
449420

450-
_LIBCPP_INLINE_VISIBILITY
451-
__tuple_impl&
452-
operator=(const __tuple_impl& __t) _NOEXCEPT_((__all<is_nothrow_copy_assignable<_Tp>::value...>::value))
453-
{
454-
__swallow(__tuple_leaf<_Indx, _Tp>::operator=(static_cast<const __tuple_leaf<_Indx, _Tp>&>(__t).get())...);
455-
return *this;
456-
}
457-
458-
_LIBCPP_INLINE_VISIBILITY
459-
__tuple_impl&
460-
operator=(__tuple_impl&& __t) _NOEXCEPT_((__all<is_nothrow_move_assignable<_Tp>::value...>::value))
461-
{
462-
__swallow(__tuple_leaf<_Indx, _Tp>::operator=(_VSTD::forward<_Tp>(static_cast<__tuple_leaf<_Indx, _Tp>&>(__t).get()))...);
463-
return *this;
464-
}
465-
466421
_LIBCPP_INLINE_VISIBILITY
467422
void swap(__tuple_impl& __t)
468423
_NOEXCEPT_(__all<__is_nothrow_swappable<_Tp>::value...>::value)
469424
{
470-
__swallow(__tuple_leaf<_Indx, _Tp>::swap(static_cast<__tuple_leaf<_Indx, _Tp>&>(__t))...);
425+
_VSTD::__swallow(__tuple_leaf<_Indx, _Tp>::swap(static_cast<__tuple_leaf<_Indx, _Tp>&>(__t))...);
471426
}
472427
};
473428

429+
template<class _Dest, class _Source, size_t ..._Np>
430+
_LIBCPP_INLINE_VISIBILITY
431+
void __memberwise_copy_assign(_Dest& __dest, _Source const& __source, __tuple_indices<_Np...>) {
432+
_VSTD::__swallow(((_VSTD::get<_Np>(__dest) = _VSTD::get<_Np>(__source)), void(), 0)...);
433+
}
474434

435+
template<class _Dest, class _Source, class ..._Up, size_t ..._Np>
436+
_LIBCPP_INLINE_VISIBILITY
437+
void __memberwise_forward_assign(_Dest& __dest, _Source&& __source, __tuple_types<_Up...>, __tuple_indices<_Np...>) {
438+
_VSTD::__swallow(((
439+
_VSTD::get<_Np>(__dest) = _VSTD::forward<_Up>(_VSTD::get<_Np>(_VSTD::forward<_Source>(__source)))
440+
), void(), 0)...);
441+
}
475442

476443
template <class ..._Tp>
477444
class _LIBCPP_TEMPLATE_VIS tuple
@@ -916,39 +883,129 @@ public:
916883
tuple(allocator_arg_t, const _Alloc& __a, _Tuple&& __t)
917884
: __base_(allocator_arg_t(), __a, _VSTD::forward<_Tuple>(__t)) {}
918885

919-
using _CanCopyAssign = __all<is_copy_assignable<_Tp>::value...>;
920-
using _CanMoveAssign = __all<is_move_assignable<_Tp>::value...>;
886+
// [tuple.assign]
887+
_LIBCPP_INLINE_VISIBILITY
888+
tuple& operator=(_If<_And<is_copy_assignable<_Tp>...>::value, tuple, __nat> const& __tuple)
889+
_NOEXCEPT_((_And<is_nothrow_copy_assignable<_Tp>...>::value))
890+
{
891+
_VSTD::__memberwise_copy_assign(*this, __tuple,
892+
typename __make_tuple_indices<sizeof...(_Tp)>::type());
893+
return *this;
894+
}
895+
896+
_LIBCPP_INLINE_VISIBILITY
897+
tuple& operator=(_If<_And<is_move_assignable<_Tp>...>::value, tuple, __nat>&& __tuple)
898+
_NOEXCEPT_((_And<is_nothrow_move_assignable<_Tp>...>::value))
899+
{
900+
_VSTD::__memberwise_forward_assign(*this, _VSTD::move(__tuple),
901+
__tuple_types<_Tp...>(),
902+
typename __make_tuple_indices<sizeof...(_Tp)>::type());
903+
return *this;
904+
}
921905

906+
template<class... _Up, _EnableIf<
907+
_And<
908+
_BoolConstant<sizeof...(_Tp) == sizeof...(_Up)>,
909+
is_assignable<_Tp&, _Up const&>...
910+
>::value
911+
,int> = 0>
922912
_LIBCPP_INLINE_VISIBILITY
923-
tuple& operator=(typename conditional<_CanCopyAssign::value, tuple, __nat>::type const& __t)
924-
_NOEXCEPT_((__all<is_nothrow_copy_assignable<_Tp>::value...>::value))
913+
tuple& operator=(tuple<_Up...> const& __tuple)
914+
_NOEXCEPT_((_And<is_nothrow_assignable<_Tp&, _Up const&>...>::value))
925915
{
926-
__base_.operator=(__t.__base_);
916+
_VSTD::__memberwise_copy_assign(*this, __tuple,
917+
typename __make_tuple_indices<sizeof...(_Tp)>::type());
927918
return *this;
928919
}
929920

921+
template<class... _Up, _EnableIf<
922+
_And<
923+
_BoolConstant<sizeof...(_Tp) == sizeof...(_Up)>,
924+
is_assignable<_Tp&, _Up>...
925+
>::value
926+
,int> = 0>
930927
_LIBCPP_INLINE_VISIBILITY
931-
tuple& operator=(typename conditional<_CanMoveAssign::value, tuple, __nat>::type&& __t)
932-
_NOEXCEPT_((__all<is_nothrow_move_assignable<_Tp>::value...>::value))
928+
tuple& operator=(tuple<_Up...>&& __tuple)
929+
_NOEXCEPT_((_And<is_nothrow_assignable<_Tp&, _Up>...>::value))
933930
{
934-
__base_.operator=(static_cast<_BaseT&&>(__t.__base_));
931+
_VSTD::__memberwise_forward_assign(*this, _VSTD::move(__tuple),
932+
__tuple_types<_Up...>(),
933+
typename __make_tuple_indices<sizeof...(_Tp)>::type());
935934
return *this;
936935
}
937936

938-
template <class _Tuple,
939-
class = typename enable_if
940-
<
941-
__tuple_assignable<_Tuple, tuple>::value
942-
>::type
943-
>
944-
_LIBCPP_INLINE_VISIBILITY
945-
tuple&
946-
operator=(_Tuple&& __t) _NOEXCEPT_((is_nothrow_assignable<_BaseT&, _Tuple>::value))
947-
{
948-
__base_.operator=(_VSTD::forward<_Tuple>(__t));
949-
return *this;
950-
}
937+
template<class _Up1, class _Up2, class _Dep = true_type, _EnableIf<
938+
_And<_Dep,
939+
_BoolConstant<sizeof...(_Tp) == 2>,
940+
is_assignable<_FirstType<_Tp..., _Dep>&, _Up1 const&>,
941+
is_assignable<_SecondType<_Tp..., _Dep>&, _Up2 const&>
942+
>::value
943+
,int> = 0>
944+
_LIBCPP_INLINE_VISIBILITY
945+
tuple& operator=(pair<_Up1, _Up2> const& __pair)
946+
_NOEXCEPT_((_And<
947+
is_nothrow_assignable<_FirstType<_Tp...>&, _Up1 const&>,
948+
is_nothrow_assignable<_SecondType<_Tp...>&, _Up2 const&>
949+
>::value))
950+
{
951+
_VSTD::get<0>(*this) = __pair.first;
952+
_VSTD::get<1>(*this) = __pair.second;
953+
return *this;
954+
}
955+
956+
template<class _Up1, class _Up2, class _Dep = true_type, _EnableIf<
957+
_And<_Dep,
958+
_BoolConstant<sizeof...(_Tp) == 2>,
959+
is_assignable<_FirstType<_Tp..., _Dep>&, _Up1>,
960+
is_assignable<_SecondType<_Tp..., _Dep>&, _Up2>
961+
>::value
962+
,int> = 0>
963+
_LIBCPP_INLINE_VISIBILITY
964+
tuple& operator=(pair<_Up1, _Up2>&& __pair)
965+
_NOEXCEPT_((_And<
966+
is_nothrow_assignable<_FirstType<_Tp...>&, _Up1>,
967+
is_nothrow_assignable<_SecondType<_Tp...>&, _Up2>
968+
>::value))
969+
{
970+
_VSTD::get<0>(*this) = _VSTD::move(__pair.first);
971+
_VSTD::get<1>(*this) = _VSTD::move(__pair.second);
972+
return *this;
973+
}
974+
975+
// EXTENSION
976+
template<class _Up, size_t _Np, class = _EnableIf<
977+
_And<
978+
_BoolConstant<_Np == sizeof...(_Tp)>,
979+
is_assignable<_Tp&, _Up const&>...
980+
>::value
981+
> >
982+
_LIBCPP_INLINE_VISIBILITY
983+
tuple& operator=(array<_Up, _Np> const& __array)
984+
_NOEXCEPT_((_And<is_nothrow_assignable<_Tp&, _Up const&>...>::value))
985+
{
986+
_VSTD::__memberwise_copy_assign(*this, __array,
987+
typename __make_tuple_indices<sizeof...(_Tp)>::type());
988+
return *this;
989+
}
990+
991+
// EXTENSION
992+
template<class _Up, size_t _Np, class = void, class = _EnableIf<
993+
_And<
994+
_BoolConstant<_Np == sizeof...(_Tp)>,
995+
is_assignable<_Tp&, _Up>...
996+
>::value
997+
> >
998+
_LIBCPP_INLINE_VISIBILITY
999+
tuple& operator=(array<_Up, _Np>&& __array)
1000+
_NOEXCEPT_((_And<is_nothrow_assignable<_Tp&, _Up>...>::value))
1001+
{
1002+
_VSTD::__memberwise_forward_assign(*this, _VSTD::move(__array),
1003+
__tuple_types<_If<true, _Up, _Tp>...>(),
1004+
typename __make_tuple_indices<sizeof...(_Tp)>::type());
1005+
return *this;
1006+
}
9511007

1008+
// [tuple.swap]
9521009
_LIBCPP_INLINE_VISIBILITY
9531010
void swap(tuple& __t) _NOEXCEPT_(__all<__is_nothrow_swappable<_Tp>::value...>::value)
9541011
{__base_.swap(__t.__base_);}

‎libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.assign/array.extension.pass.cpp

+104
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// <tuple>
10+
11+
// template <class... Types> class tuple;
12+
13+
// EXTENSION
14+
// template <class U, size_t N>
15+
// tuple& operator=(const array<U, N>& u);
16+
//
17+
// template <class U, size_t N>
18+
// tuple& operator=(array<U, N>&& u);
19+
20+
// UNSUPPORTED: c++03
21+
22+
#include <array>
23+
#include <cassert>
24+
#include <tuple>
25+
#include <type_traits>
26+
#include <utility>
27+
28+
29+
template <class T>
30+
struct NothrowAssignableFrom {
31+
NothrowAssignableFrom& operator=(T) noexcept { return *this; }
32+
};
33+
34+
template <class T>
35+
struct PotentiallyThrowingAssignableFrom {
36+
PotentiallyThrowingAssignableFrom& operator=(T) { return *this; }
37+
};
38+
39+
int main(int, char**) {
40+
// Tests for the array const& overload
41+
{
42+
std::array<long, 3> array = {1l, 2l, 3l};
43+
std::tuple<int, int, int> tuple;
44+
tuple = array;
45+
assert(std::get<0>(tuple) == 1);
46+
assert(std::get<1>(tuple) == 2);
47+
assert(std::get<2>(tuple) == 3);
48+
}
49+
{
50+
typedef std::tuple<NothrowAssignableFrom<int>> Tuple;
51+
typedef std::array<int, 1> Array;
52+
static_assert(std::is_nothrow_assignable<Tuple&, Array const&>::value, "");
53+
}
54+
{
55+
typedef std::tuple<PotentiallyThrowingAssignableFrom<int>> Tuple;
56+
typedef std::array<int, 1> Array;
57+
static_assert(std::is_assignable<Tuple&, Array const&>::value, "");
58+
static_assert(!std::is_nothrow_assignable<Tuple&, Array const&>::value, "");
59+
}
60+
61+
// Tests for the array&& overload
62+
{
63+
std::array<long, 3> array = {1l, 2l, 3l};
64+
std::tuple<int, int, int> tuple;
65+
tuple = std::move(array);
66+
assert(std::get<0>(tuple) == 1);
67+
assert(std::get<1>(tuple) == 2);
68+
assert(std::get<2>(tuple) == 3);
69+
}
70+
{
71+
typedef std::tuple<NothrowAssignableFrom<int>> Tuple;
72+
typedef std::array<int, 1> Array;
73+
static_assert(std::is_nothrow_assignable<Tuple&, Array&&>::value, "");
74+
}
75+
{
76+
typedef std::tuple<PotentiallyThrowingAssignableFrom<int>> Tuple;
77+
typedef std::array<int, 1> Array;
78+
static_assert(std::is_assignable<Tuple&, Array&&>::value, "");
79+
static_assert(!std::is_nothrow_assignable<Tuple&, Array&&>::value, "");
80+
}
81+
82+
// Test lvalue-refs and const rvalue-ref
83+
{
84+
typedef std::tuple<NothrowAssignableFrom<int>> Tuple;
85+
typedef std::array<int, 1> Array;
86+
static_assert(std::is_nothrow_assignable<Tuple&, Array&>::value, "");
87+
static_assert(std::is_nothrow_assignable<Tuple&, const Array&&>::value, "");
88+
}
89+
90+
{
91+
typedef std::tuple<NothrowAssignableFrom<int>> Tuple;
92+
static_assert(!std::is_assignable<Tuple&, std::array<long, 2>&>::value, "");
93+
static_assert(!std::is_assignable<Tuple&, std::array<long, 2>&&>::value, "");
94+
static_assert(!std::is_assignable<Tuple&, const std::array<long, 2>&>::value, "");
95+
static_assert(!std::is_assignable<Tuple&, const std::array<long, 2>&&>::value, "");
96+
97+
static_assert(!std::is_assignable<Tuple&, std::array<long, 4>&>::value, "");
98+
static_assert(!std::is_assignable<Tuple&, std::array<long, 4>&&>::value, "");
99+
static_assert(!std::is_assignable<Tuple&, const std::array<long, 4>&>::value, "");
100+
static_assert(!std::is_assignable<Tuple&, const std::array<long, 4>&&>::value, "");
101+
}
102+
103+
return 0;
104+
}

‎libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/tuple_array_template_depth.pass.cpp ‎libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.assign/tuple_array_template_depth.pass.cpp

File renamed without changes.

0 commit comments

Comments
 (0)