[libcxx-commits] [PATCH] D60480: [WIP] integration of pstl into libc++

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D60480#1460728 <https://reviews.llvm.org/D60480#1460728>, @rodgert wrote:

> In D60480#1460300 <https://reviews.llvm.org/D60480#1460300>, @ldionne wrote:
>
> > The purpose of this review is to expose how I plan to integrate the parallel STL into libc++, and to discuss some changes that are desirable in pstl for this to be possible. This is a work in progress, obviously, and it's meant as a concrete support for having discussions.
> >
> > Concrete changes I think we need to make in pstl:
> >
> > - Rename all the files to double underscores.
>
>
> This isn't a libstdc++ convention, but I'm not opposed.


The problem with not doing that is that our headers could collide with user's header files.

> 
> 
>> - Move everything that's meant to be included directly by standard library implementations out of `internal/`, since that doesn't make sense. It's not internal, it's the API of PSTL (which is meant to be consumed by standard libraries).
> 
> I would like to maintain a clear separation between the bits that implement PSTL and the glue bits that tie into a given library. Currently everything in 'internal/' goes to libstdc++v3/include/pstl when I integrate with libstdc++v3. I don't want to be left guessing what those files are, I want them to move as a cohesive unit into the library's implementation detail. We can name whatever we want.

Right, I agree. What I meant (but that's not what I wrote) is that the glue headers and everything that is expected to be used from standard libraries should be out of `internal`. The backend and the config stuff can (and should) stay in `internal`. Do we agree here?

> 
> 
>> - We could provide the glue definitions and declarations as `.ipp` files that are meant to be included by standard libraries as-is. The Standard library could then define stuff like the namespace in which those declarations are injected, etc.
> 
> I would be interested in exploring this, subject to the caveat that whole of what's currently in 'internal' is an obvious package of related implementation detail that I can poke into libstdc++ as a unit.

Well, so only the glue part would be `.ipp`s. And I don't care if they're headers or `.ipp`s, the idea is just that I want to reverse the model from "libc++ includes pstl headers that define the parallel algorithms" to "pstl provides declarations that a standard library can copy-paste into their own headers via `#include`".

>> I also suggest:
>> 
>> - Removing the `<algorithm>` & friends headers from PSTL itself. Those don't belong there, they belong with the standard library.
>> 
>>   WDYT @rodgert ?
> 
> If we remove them, then we sort of lose the ability to test pstl outside of standard library, no? I'm not going to be testing or packaging libc++ most likely, but I am going to be contributing changes here and running tests within the context of libstdc++. Having the ability to test changes to the library independent of any integration with another standard library is still useful for me.

I believe those headers should be provided by a dummy standard library that we use only to run the tests. Basically we'd have a dummy standard library in `pstl/test` that would define these headers, and that's what we would run the tests against.

Lastly, about the `__PSTL_XXX` vs `_PSTL_XXX`: This patch is using what's in PSTL today, that's all. As a separate change, I thought about changing all `__PSTL_XXX` macros to be `_PSTL_XXX` to follow the more common convention (underscore-capital or double-underscore-lowercase), but this is a change separate from the integration into libc++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60480