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

Thu Nov 11 10:39:05 PST 2021

miyuki marked an inline comment as done.
miyuki added inline comments.

================
Comment at: libcxx/include/__iterator/reverse_iterator.h:114
         is_convertible<_Up const&, _Iter>::value &&
-        is_assignable<_Up const&, _Iter>::value
+        is_assignable<_Iter, _Up const&>::value
     > >
----------------
ldionne wrote:
> Quuxplusone wrote:
> > C++17 actually doesn't constrain this `operator=` at all
> > https://timsong-cpp.github.io/cppwp/n4659/reverse.iter.op=
> > but C++20 added constraints
> > https://cplusplus.github.io/LWG/issue3435
> > which @ldionne added (in all modes) in e4d3a993c2675f46cbe99acd1bf6e6d39d9c1aee.
> > I believe we should make this constraint apply only in C++20, and follow the Standard's subtly different spec: `convertible_to, assignable_from` instead of `is_convertible, is_assignable`.
> > 
> > Scope creep: Any appetite for adding `operator<=>` at the same time, so that we could actually verify that `three_way_comparable_with<reverse_iterator<int*>, reverse_iterator<const int*>>` will be true after this patch? (I believe the only actually missing puzzle piece is `operator<=>`.)
> I added it in all modes because it's a LWG issue, not only a paper. I think that is what we want to do, however you are right we can't implement it exactly per the spec if we backport it to before C++20 (because we don't have concepts then).
> Scope creep: Any appetite for adding operator<=>

Submitted as a separate patch: D113695

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113417/new/

https://reviews.llvm.org/D113417