[libcxx-commits] [PATCH] D151841: [libc++][PSTL] Implement std::replace{, _if, _copy, _copy_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/CMakeLists.txt:84
   __algorithm/pstl_backends/cpu_backends/thread.h
   __algorithm/pstl_backends/cpu_backends/transform.h
   __algorithm/pstl_backends/cpu_backends/transform_reduce.h
----------------
Can we remove any old PSTL code with this patch?


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.replace.pass.cpp:29
+  template <class ExecutionPolicy>
+  void operator()(ExecutionPolicy&& policy) {
+    { // simple test
----------------
There needs to be a test with a non-empty range but no match. Here and everywhere.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.replace.pass.cpp:30
+  void operator()(ExecutionPolicy&& policy) {
+    { // simple test
+      std::array a = {1, 2, 3, 4, 5, 6, 7, 8};
----------------
Needs a test where you "match" more than one element in the range. For example

```
std::array a = {1, 2, 3, 4, 3, 3, 5, 6, 3};
// then replace 3 by 9, you need to end up with {1, 2, 9, 4, 9, 9, 5, 6, 9}
```

This comment applies to all the other algorithms as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151841