Skip to content

Commit 6938270

Browse files
committed
[libcxx] Fix enable_if condition of std::reverse_iterator::operator=
The template std::is_assignable<T, U> checks that T is assignable from U. Hence, the order of operands in the instantiation of std::is_assignable in the std::reverse_iterator::operator= condition should be reversed. This issue remained unnoticed because std::reverse_iterator has an implicit conversion constructor. This patch adds a test to check that the assignment operator is used directly, without any implicit conversions. The patch also adds a similar test for std::move_iterator. Reviewed By: Quuxplusone, ldionne, #libc Differential Revision: https://reviews.llvm.org/D113417
1 parent 254aa65 commit 6938270

File tree

3 files changed

+67
-2
lines changed

3 files changed

+67
-2
lines changed

libcxx/include/__iterator/reverse_iterator.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ _LIBCPP_SUPPRESS_DEPRECATED_POP
8686
template <class _Up, class = __enable_if_t<
8787
!is_same<_Up, _Iter>::value &&
8888
is_convertible<_Up const&, _Iter>::value &&
89-
is_assignable<_Up const&, _Iter>::value
89+
is_assignable<_Iter, _Up const&>::value
9090
> >
9191
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
9292
reverse_iterator& operator=(const reverse_iterator<_Up>& __u) {
@@ -111,7 +111,7 @@ _LIBCPP_SUPPRESS_DEPRECATED_POP
111111
template <class _Up, class = __enable_if_t<
112112
!is_same<_Up, _Iter>::value &&
113113
is_convertible<_Up const&, _Iter>::value &&
114-
is_assignable<_Up const&, _Iter>::value
114+
is_assignable<_Iter, _Up const&>::value
115115
> >
116116
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
117117
reverse_iterator& operator=(const reverse_iterator<_Up>& __u) {

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op=/move_iterator.pass.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,39 @@ test(U u)
3737
struct Base {};
3838
struct Derived : Base {};
3939

40+
struct ToIter {
41+
typedef std::forward_iterator_tag iterator_category;
42+
typedef char *pointer;
43+
typedef char &reference;
44+
typedef char value_type;
45+
typedef value_type difference_type;
46+
47+
explicit TEST_CONSTEXPR_CXX17 ToIter() : m_value(0) {}
48+
TEST_CONSTEXPR_CXX17 ToIter(const ToIter &src) : m_value(src.m_value) {}
49+
// Intentionally not defined, must not be called.
50+
ToIter(char *src);
51+
TEST_CONSTEXPR_CXX17 ToIter &operator=(char *src) {
52+
m_value = src;
53+
return *this;
54+
}
55+
TEST_CONSTEXPR_CXX17 ToIter &operator=(const ToIter &src) {
56+
m_value = src.m_value;
57+
return *this;
58+
}
59+
char *m_value;
60+
};
61+
62+
TEST_CONSTEXPR_CXX17 bool test_conv_assign()
63+
{
64+
char c = '\0';
65+
char *fi = &c;
66+
const std::move_iterator<char *> move_fi(fi);
67+
std::move_iterator<ToIter> move_ti;
68+
move_ti = move_fi;
69+
assert(move_ti.base().m_value == fi);
70+
return true;
71+
}
72+
4073
int main(int, char**)
4174
{
4275
Derived d;
@@ -46,6 +79,7 @@ int main(int, char**)
4679
test<bidirectional_iterator<Base*> >(bidirectional_iterator<Derived*>(&d));
4780
test<random_access_iterator<const Base*> >(random_access_iterator<Derived*>(&d));
4881
test<Base*>(&d);
82+
test_conv_assign();
4983
#if TEST_STD_VER > 14
5084
{
5185
using BaseIter = std::move_iterator<const Base *>;
@@ -54,6 +88,7 @@ int main(int, char**)
5488
constexpr DerivedIter it1 = std::make_move_iterator(p);
5589
constexpr BaseIter it2 = (BaseIter{nullptr} = it1);
5690
static_assert(it2.base() == p, "");
91+
static_assert(test_conv_assign(), "");
5792
}
5893
#endif
5994

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/assign.pass.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,41 @@ TEST_CONSTEXPR_CXX17 void test(U u) {
3131
struct Base { };
3232
struct Derived : Base { };
3333

34+
struct ToIter {
35+
typedef std::bidirectional_iterator_tag iterator_category;
36+
typedef char *pointer;
37+
typedef char &reference;
38+
typedef char value_type;
39+
typedef value_type difference_type;
40+
41+
explicit TEST_CONSTEXPR_CXX17 ToIter() : m_value(0) {}
42+
TEST_CONSTEXPR_CXX17 ToIter(const ToIter &src) : m_value(src.m_value) {}
43+
// Intentionally not defined, must not be called.
44+
ToIter(char *src);
45+
TEST_CONSTEXPR_CXX17 ToIter &operator=(char *src) {
46+
m_value = src;
47+
return *this;
48+
}
49+
TEST_CONSTEXPR_CXX17 ToIter &operator=(const ToIter &src) {
50+
m_value = src.m_value;
51+
return *this;
52+
}
53+
char *m_value;
54+
};
55+
3456
TEST_CONSTEXPR_CXX17 bool tests() {
3557
Derived d;
3658
test<bidirectional_iterator<Base*> >(bidirectional_iterator<Derived*>(&d));
3759
test<random_access_iterator<const Base*> >(random_access_iterator<Derived*>(&d));
3860
test<Base*>(&d);
61+
62+
char c = '\0';
63+
char *fi = &c;
64+
const std::reverse_iterator<char *> rev_fi(fi);
65+
std::reverse_iterator<ToIter> rev_ti;
66+
rev_ti = rev_fi;
67+
assert(rev_ti.base().m_value == fi);
68+
3969
return true;
4070
}
4171

0 commit comments

Comments
 (0)