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

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Apr 10 08:56:30 PDT 2019


ldionne added a comment.

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

> In D60480#1461369 <https://reviews.llvm.org/D60480#1461369>, @ldionne wrote:
>
> > In D60480#1460728 <https://reviews.llvm.org/D60480#1460728>, @rodgert wrote:
> >
> > > In D60480#1460300 <https://reviews.llvm.org/D60480#1460300>, @ldionne wrote:
> > >
> > > > 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.
>
>
> That would work for me...AND...we can probably remove the per-test #ifdef for the standalone checks, centralize it in the dummy standard headers in 'pstl/test' which would be much cleaner than what is currently there.


Ok, I'll submit a patch for that.

> 
> 
>> 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++.
> 
> JF and I discussed possibly some tooling for this sort of thing. I'm still interested in looking at that, I just don't see a lot of value in changing it by hand just to comply with conflicting conventions between standard library implementations.

I don't think the conventions used in PSTL should conform to the coding standard in any one standard library implementation. It's a library on its own. I think it should conform to its own coding standards, and in this case, I think those coding standard should be that of LLVM. Does this point of view make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60480





More information about the libcxx-commits mailing list