[libcxx-commits] [PATCH] D128864: [libc++] Fix algorihtms which use reverse_iterator

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__iterator/reverse_iterator.h:365-371
+  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const {
+    if constexpr (is_pointer_v<_Iter>) {
+      return std::prev(__iter_);
+    } else {
+      return std::prev(__iter_).operator->();
+    }
+  }
----------------
philnik wrote:
> huixie90 wrote:
> > could you please clarify why SFIANE is removed and how do you prevent error on instantiating the class template with an `_Iter` that does not support `->`? (assuming all non-template member functions are generated when class template is instantiated)
> The new SFINAE in C++20 is the whole reason this PR exists at all. That's the part that breaks the classic algorithms. Regarding `operator->`: that has to exist for the classic algorithms. See https://en.cppreference.com/w/cpp/named_req/InputIterator.
Then this class is incorrectly constrained (the static assert bit) the input `_Iter` has to model the c++17 bidi iterator, because c++20 bidi iterator wouldn’t work. It may not have `->` , and it may have prvalue `reference`, which is not allowed in c++ 17 bidi

So the input `Iter` has to be c++17 and the result iterator is also c++17. Why not just call it c++17 reverse iterator. 

Regarding random access operations not sfinae , when you pass in c++17 bidi only , wouldn’t it error on instantiating those member functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128864