[libcxx-commits] [PATCH] D130404: [libc++][ranges] implement `std::ranges::unique{_copy}`

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision as: var-const.
var-const added a comment.

LGTM, leaving the final approval to @ldionne.



================
Comment at: libcxx/include/__algorithm/adjacent_find.h:49
   typedef typename iterator_traits<_ForwardIterator>::value_type __v;
-  return _VSTD::adjacent_find(__first, __last, __equal_to<__v>());
+  return std::adjacent_find(__first, __last, __equal_to<__v>());
 }
----------------
Nit: move?


================
Comment at: libcxx/include/__algorithm/unique.h:39
+        *++__first = _IterOps<_AlgPolicy>::__iter_move(__i);
+    return std::pair<_Iter, _Iter>(std::move(++__first), std::move(__i));
+  }
----------------
Nit: I'd move the increment back into its own line, otherwise there's a little too much going on on this line.


================
Comment at: libcxx/include/__algorithm/unique_copy.h:80
 
-template <class _BinaryPredicate, class _InputIterator, class _ForwardIterator>
-_LIBCPP_CONSTEXPR_AFTER_CXX17 _ForwardIterator
-__unique_copy(_InputIterator __first, _InputIterator __last, _ForwardIterator __result, _BinaryPredicate __pred,
-              input_iterator_tag, forward_iterator_tag)
-{
-    if (__first != __last)
-    {
-        *__result = *__first;
-        while (++__first != __last)
-            if (!__pred(*__result, *__first))
-                *++__result = *__first;
-        ++__result;
-    }
-    return __result;
+template <class _AlgPolicy, class _BinaryPredicate, class _InputIterator, class _Sent, class _InputOutputIterator>
+_LIBCPP_CONSTEXPR_AFTER_CXX17 _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _InputOutputIterator> __unique_copy_impl(
----------------
Perhaps `InputOrOutputIterator`?


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.unique/ranges_unique.pass.cpp:129
+
+  // empty range
+  {
----------------
Optional: maybe check a single-element range for good measure?


================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_dangling.pass.cpp:89
   using std::ranges::unary_transform_result;
-  //using std::ranges::unique_copy_result;
+  using std::ranges::unique_copy_result;
 
----------------
Please also update `test/std/algorithms/ranges_result_alias_declarations.compile.pass.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130404