[libcxx-commits] [PATCH] D149615: [libc++][PSTL] Implement std::transform

[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/test/std/algorithms/alg.modifying.operations/alg.transform/pstl.transform.binary.pass.cpp:22
+//               BinaryOperation binary_op);
+
+#include <algorithm>
----------------
We should also include a SFINAE test that checks that we SFINAE-out whenever the first argument is not an execution policy. We could do that in a single big test file for all the algorithms, like we did for some ranges algorithms.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.transform/pstl.transform.binary.pass.cpp:30
+
+template <class Iter1, class Iter2, class Iter3>
+struct Test {
----------------
I think we're missing tests for constraints here and in the other test.


================
Comment at: libcxx/test/support/type_algorithms.h:56-60
+template <template <class...> class T, class... Args>
+struct partial_instantiation {
+  template <class Other>
+  using type = T<Args..., Other>;
+};
----------------
I would suggest this instead:

```
template <template <class...> class F, class ...T>
struct partial_instantiation {
  template <class ...Args>
  using apply = F<T..., Args...>;
};
```

This suggests that we're doing partial application of the template `F`. `apply` is often used in these cases so it's kind of idiomatic (e.g. the old MPL and derivatives of it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149615