[libcxx-commits] [PATCH] D103198: [libc++] Add a CI configuration where we test the PSTL integration

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a subscriber: rodgert.
ldionne added a comment.
This revision now requires changes to proceed.

In D103198#3673665 <https://reviews.llvm.org/D103198#3673665>, @h-vetinari wrote:

> How realistic is it that this still makes it into LLVM 15? I'd really like to start testing the pstl in our ecosystem a bit, happy to file bug reports too if so, but I'd need a way to be able to build it...

Unfortunately, I don't think we'll cherry-pick this to LLVM 15. Proper integration requires additional work, and that would be too much for LLVM 15 since we've already branched. We focused on Ranges and Format for LLVM 15. This should make it for LLVM 16 though.



================
Comment at: libcxx/include/algorithm:1673
+// TODO: Remove this include once we fix transitive includes of <algorithm>
+#  include <__functional/not_fn.h>
+#  include <__pstl_algorithm>
----------------
I think the PSTL should be more tightly integrated with libc++, and we should probably be including libc++'s internal headers directly from it to break these kinds of circular dependencies.

This is going to be non-trivial though because there was an intent to share the PSTL between libc++ and libstdc++ -- I don't know whether that is still desired, and we may need to revisit this if we want to ease the integration between libc++ and PSTL.

@rodgert What is the current state of PSTL within libstdc++? How do you integrate it nowadays? Back in the days, I think you had a script that basically copied the sources to libstdc++ -- if that's still the case, do you have plans to keep it up to date, and can you link to that script so we can take a look at how the integration works?


================
Comment at: libcxx/test/configs/llvm-libc++-with-pstl.cfg.in:10
+config.substitutions.append(('%{compile_flags}',
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{pstl-include} -I %{pstl-generated-include} -I %{libcxx}/test/support'
+))
----------------
We should be installing the PSTL headers to `include/c++/v1` so that we find them alongside other libc++ headers out-of-the-box, and users don't need to add an additional include.

I suspect that means we'll have to start running the tests against a fake-installation of libc++/libc++abi/PSTL/whatever, which is what we should do anyways because it's closer to what users get. I've been meaning to do that for a while, I'll see if I can do this in the next few weeks.


================
Comment at: pstl/include/pstl/internal/utils.h:27
 {
+#if defined(__cpp_exceptions) && __cpp_exceptions >= 199711L
     try
----------------
This bit (and a few others) can land separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103198