[libcxx-commits] [PATCH] D129859: [libc++] Implement ranges::{prev, next}_permutation

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/CMakeLists.txt:114
   __algorithm/ranges_move_backward.h
+  __algorithm/ranges_next_permutation.h
   __algorithm/ranges_none_of.h
----------------
could you please update the RangeAlgorithms.csv file?


================
Comment at: libcxx/include/__algorithm/ranges_prev_permutation.h:40-62
+__prev_permutation_impl(_Iter __first, _Sent __sent, _Comp& __comp, _Proj& __proj) {
+  auto __last = ranges::next(__first, __sent);
+  auto __i = __last;
+  if (__first == __last || __first == --__i)
+    return {__last, false};
+  while (true) {
+    auto __ip1 = __i;
----------------
I think this algorithm is complex enough to reuse the classic one. what do you think?


================
Comment at: libcxx/include/__algorithm/ranges_prev_permutation.h:55
+      ranges::reverse(__ip1, __last);
+      return {__last, true};
+    }
----------------
`std::move(__last)`


================
Comment at: libcxx/include/__algorithm/ranges_prev_permutation.h:59
+      ranges::reverse(__first, __last);
+      return {__last, false};
+    }
----------------
`std::move(__last)`


================
Comment at: libcxx/test/support/test_iterators.h:758
+
+  constexpr friend void iter_swap(SwapCountingIterator& lhs, SwapCountingIterator& rhs) {
+    std::ranges::iter_swap(lhs.iter_, rhs.iter_);
----------------
take the arguments by const ref, as `indirectly_swappable` requires const


================
Comment at: libcxx/test/support/test_iterators.h:760
+    std::ranges::iter_swap(lhs.iter_, rhs.iter_);
+    ++*lhs.counter_;
+  }
----------------
why does the `rhs` counter not incremented?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129859