[libcxx-commits] [PATCH] D130070: [libc++][ranges] Implement `std::ranges::partition_{point, copy}`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_partition_copy.h:43
+  _LIBCPP_HIDE_FROM_ABI constexpr
+  static partition_copy_result<_InIter, _OutIter1, _OutIter2> __partition_copy_fn_impl(
+      _InIter&& __first, _Sent&& __last, _OutIter1&& __out_true, _OutIter2&& __out_false,
----------------
ldionne wrote:
> var-const wrote:
> > These two algorithms seem borderline when choosing whether to delegate or reimplement. I'm quite open to delegate if you feel it would be beneficial.
> I would have a preference for delegating. I'm also fine with a TODO to do it after the release, since you have it implemented right now and it works (according to the tests).
Thanks! Added a TODO.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/ranges_partition_copy.pass.cpp:270
+  }
 
   return true;
----------------
huixie90 wrote:
> perhaps also tests "Complexity: Exactly last - first applications of pred and proj."
I want to consolidate these checks as well, but realistically won't have time before the branch cut. Added the test.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/ranges_partition_point.pass.cpp:166
+      assert(result_no_proj == expected_no_proj);
+      auto result_with_proj = std::ranges::partition_point(in, is_odd, x2);
+      assert(result_with_proj == expected_with_proj);
----------------
huixie90 wrote:
> can we have at least one test where the comparator is not `is_odd`?
Done (also in `partition` and `stable_partition`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130070