[libcxx-commits] [PATCH] D117660: [libc++] Fix a typo in reverse_iterator::operator=

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: ldionne, libc++.
Quuxplusone added a project: libc++.
Quuxplusone requested review of this revision.
Herald added a subscriber: libcxx-commits.
Herald added 1 blocking reviewer(s): libc++.

We should be checking `is_assignable<It&, ...>`. `is_assignable<It, ...>` checks for an rvalue left-hand side, which is basically never assignable-to.
Found while looking into https://cplusplus.github.io/LWG/issue3435 .

I don't think this is worth testing, because it's just a matter of whether we call `it.operator=(jt)` (correct) or `it.operator=(It(jt))` (current behavior). Also, the behavior apparently changes again in C++20. Also, there's implementation divergence because of our dumb `_LIBCPP_ABI_NO_ITERATOR_BASES` thing (so we make twice as many copies as we need to — btw this is going to be an extra mess as soon as we need to support move-only iterators in `reverse_iterator`)
https://godbolt.org/z/5TKvshM3f

But I'd like to get the typo-fix landed now, so that it doesn't fall through the cracks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117660

Files:
  libcxx/include/__iterator/reverse_iterator.h


Index: libcxx/include/__iterator/reverse_iterator.h
===================================================================
--- libcxx/include/__iterator/reverse_iterator.h
+++ libcxx/include/__iterator/reverse_iterator.h
@@ -77,7 +77,7 @@
     template <class _Up, class = __enable_if_t<
         !is_same<_Up, _Iter>::value &&
         is_convertible<_Up const&, _Iter>::value &&
-        is_assignable<_Iter, _Up const&>::value
+        is_assignable<_Iter&, _Up const&>::value
     > >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
     reverse_iterator& operator=(const reverse_iterator<_Up>& __u) {
@@ -102,7 +102,7 @@
     template <class _Up, class = __enable_if_t<
         !is_same<_Up, _Iter>::value &&
         is_convertible<_Up const&, _Iter>::value &&
-        is_assignable<_Iter, _Up const&>::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: D117660.401170.patch
Type: text/x-patch
Size: 1016 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220119/d37cf584/attachment.bin>