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

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jul 7 17:01:09 PDT 2022


huixie90 added inline comments.


================
Comment at: libcxx/include/__iterator/reverse_iterator.h:339
+template <class _Iter>
+class _AlgRevIter {
+  _Iter __iter_;
----------------
question: is this class only supposed to be used in the classic algorithms? (that is why it doesn't need iter_move and iter_swap)


================
Comment at: libcxx/include/__iterator/reverse_iterator.h:349-350
+  using pointer = __iterator_pointer_type<_Iter>;
+  using iterator_concept =
+      _If<__is_cpp17_random_access_iterator<_Iter>::value, random_access_iterator_tag, bidirectional_iterator_tag>;
+  using value_type = iter_value_t<_Iter>;
----------------
why c++20 `iterator_concept ` is defined in terms of c++17 trait?


================
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->();
+    }
+  }
----------------
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)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128864



More information about the libcxx-commits mailing list