[libcxx-commits] [PATCH] D150128: [libc++][PSTL] Implement std::count{, _if}

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__algorithm/pstl_count.h:1
+//===----------------------------------------------------------------------===//
+//
----------------
We should update the backend documentation comment.


================
Comment at: libcxx/include/__algorithm/pstl_count.h:44
+      [&](_ForwardIterator __g_first, _ForwardIterator __g_last, _Predicate __g_pred) {
+        atomic<__iter_diff_t<_ForwardIterator>> __count = 0;
+        std::for_each(
----------------
I think we should implement this using `reduce` instead, like in the original PSTL. I think we should implement `std::reduce` before we implement this one.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/pstl.count.pass.cpp:55
+
+    { // test that a two-element range works
+      int a[] = {1, 3};
----------------
Can you also test a 3 element range with `{yes, no, yes}`? Right now, the tests would pass if we were counting the number of consecutive elements equal to the value, kind of like `equal_range` I guess.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/pstl.count_if.pass.cpp:55
+
+    { // test that a two-element range works
+      int a[] = {1, 3};
----------------
Same for `{yes no yes}`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150128