[libcxx-commits] [PATCH] D60480: [libc++] Integrate the PSTL into libc++

[Eric Fiselier via Phabricator via libcxx-commits]

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


================
Comment at: libcxx/include/algorithm:5790
+// otherwise just pull in forward declarations.
+#include <pstl/internal/pstl_config.h>
+#if defined(__PSTL_EXECUTION_POLICIES_DEFINED)
----------------
ldionne wrote:
> The same pattern is repeated in other files. Basically, we check whether `<execution>` has been included, and if so, then we provide all the definitions. Otherwise, we just provide forward declarations to avoid the compile-time overhead if you just `#include <algorithm>` but don't care about the parallel algorithms.
> 
I really really really dislike this pattern. We shouldn't do it.
I don't think this is how you should structure a library.

Are there other alternatives?


================
Comment at: libcxxabi/src/CMakeLists.txt:149
 endif()
 
+find_package(ParallelSTL)
----------------
Why does libc++abi depend on parallel algorithms? That seems backwards?


================
Comment at: pstl/test/pstl/header_inclusion_order_algorithm_0.pass.cpp:11
 // UNSUPPORTED: c++98, c++03, c++11, c++14
+// REQUIRES: parallel-algorithms
 
----------------
Does every test under `pstl/test/pstl` require "parallel-algorithms"? Because if so there are easier ways to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60480