[libcxx-commits] [PATCH] D120186: [pstl] Implementation of Grand Central Dispatch backend

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

Just some minor nits; the bulk of this is above my pay grade. :)



================
Comment at: pstl/include/pstl/internal/gcd/parallel_invoke.h:38
+                         {
+                             std::get<1> (*__d)();
+                         }
----------------
Here and line 34, extraneous whitespace. (Weird spacing throughout, honestly, but that's clang-format for you.)


================
Comment at: pstl/include/pstl/internal/gcd/parallel_merge.h:70
+   */
+    __parallel_merge_body(__size_x, __size_y, __xs, __xe, __ys, __ye, __zs, __comp, __leaf_merge);
+}
----------------
Here and throughout, FYI, the code is not ADL-proof, which means it'll have this problem:
https://quuxplusone.github.io/blog/2019/09/26/uglification-doesnt-stop-adl/
I'm sure this is a pre-existing problem throughout PSTL, though. (We try to avoid it in libc++ proper but I assume nobody's ever made a big deal about it in PSTL.)


================
Comment at: pstl/include/pstl/internal/gcd/parallel_stable_sort.h:109
+{
+    assert(__xe > __xs);
+
----------------
(1) It kinda looks like `__pattern_sort` (`pstl/include/pstl/internal/algorithm_impl.h` line 2198) can call this with `__xe == __xs`. But maybe I'm wrong.
(2) `_PSTL_ASSERT`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120186