[libcxx-commits] [PATCH] D129390: [lib++][ranges][NFC] Refactor `iterator_operations.h` to use tags.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

LGTM % nits and with green CI.



================
Comment at: libcxx/include/__algorithm/iterator_operations.h:76
+  static void iter_swap(_Iter1&& __a, _Iter2&& __b) {
+    std::iter_swap(std::forward<_Iter1>(__a), std::forward<_Iter2>(__b));
+  }
----------------
Is there a reason you don't use the customization point? AFAIK `iter_swap` has to be implemented properly or not at all, just like `swap`.


================
Comment at: libcxx/include/__algorithm/lower_bound.h:31
 
-template <class _IterOps, class _Iter, class _Sent, class _Type, class _Proj, class _Comp>
+template <class _AlgFamily, class _Iter, class _Sent, class _Type, class _Proj, class _Comp>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
----------------
`_AlgPolicy`?


================
Comment at: libcxx/include/__algorithm/lower_bound.h:51
 _LIBCPP_NODISCARD_EXT inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
-_ForwardIterator lower_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Compare __comp) {
+_ForwardIterator lower_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value_, _Compare __comp) {
   static_assert(__is_callable<_Compare, decltype(*__first), const _Tp&>::value,
----------------
There shouldn't be any underscores here.


================
Comment at: libcxx/include/__algorithm/set_intersection.h:37
 
-template < class _IterOper, class _Compare, class _InIter1, class _Sent1, class _InIter2, class _Sent2, class _OutIter>
+template <class _Policy, class _Compare, class _InIter1, class _Sent1, class _InIter2, class _Sent2, class _OutIter>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17 __set_intersection_result<_InIter1, _InIter2, _OutIter>
----------------
I think `_AlgPolicy` would be a bit better and more consistent with `_RangesAlgPolicy` and `_ClassicAlgPolicy`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129390