[libcxx-commits] [PATCH] D113417: [libcxx] Fix enable_if condition of std::reverse_iterator::operator=

[Mikhail Maltsev via Phabricator via libcxx-commits]

miyuki created this revision.
miyuki added reviewers: Quuxplusone, ldionne.
miyuki requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113417

Files:
  libcxx/include/__iterator/reverse_iterator.h
  libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/assign.pass.cpp


Index: libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/assign.pass.cpp
===================================================================
--- libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/assign.pass.cpp
+++ libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/assign.pass.cpp
@@ -39,8 +39,36 @@
     return true;
 }
 
+struct from_iter : bidirectional_iterator<char *> {
+    from_iter(char *p) : bidirectional_iterator(p) {}
+};
+
+typedef bidirectional_iterator<const char *> const_bidir_iter;
+
+struct to_iter : const_bidir_iter {
+    to_iter() {}
+    to_iter(const from_iter &fi) : const_bidir_iter(fi) {
+        // Ensure that the conversion constructor is not called
+        assert(false);
+    }
+    to_iter& operator=(const from_iter &fi) {
+        static_cast<const_bidir_iter &>(*this) = fi;
+        return *this;
+    }
+};
+
+void test_assign_convertible() {
+    char c;
+    from_iter fi(&c);
+    to_iter ti;
+    const std::reverse_iterator<from_iter> rev_fi(fi);
+    std::reverse_iterator<to_iter> rev_ti;
+    rev_ti = rev_fi;
+}
+
 int main(int, char**) {
     tests();
+    test_assign_convertible();
 #if TEST_STD_VER > 14
     static_assert(tests(), "");
 #endif
Index: libcxx/include/__iterator/reverse_iterator.h
===================================================================
--- libcxx/include/__iterator/reverse_iterator.h
+++ libcxx/include/__iterator/reverse_iterator.h
@@ -86,7 +86,7 @@
     template <class _Up, class = __enable_if_t<
         !is_same<_Up, _Iter>::value &&
         is_convertible<_Up const&, _Iter>::value &&
-        is_assignable<_Up const&, _Iter>::value
+        is_assignable<_Iter, _Up const&>::value
     > >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reverse_iterator& operator=(const reverse_iterator<_Up>& __u) {
@@ -111,7 +111,7 @@
     template <class _Up, class = __enable_if_t<
         !is_same<_Up, _Iter>::value &&
         is_convertible<_Up const&, _Iter>::value &&
-        is_assignable<_Up const&, _Iter>::value
+        is_assignable<_Iter, _Up const&>::value
     > >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reverse_iterator& operator=(const reverse_iterator<_Up>& __u) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113417.385542.patch
Type: text/x-patch
Size: 2300 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20211108/d6b52c60/attachment.bin>