[libcxx-commits] [PATCH] D107584: [libc++][modularisation] Split up <concepts> into granular headers.

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

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/__concepts/invocable.h:34-35
+
+template<class _Fn, class... _Args>
+concept regular_invocable = invocable<_Fn, _Args...>;
+
----------------
cjdb wrote:
> Quuxplusone wrote:
> > cjdb wrote:
> > > Quuxplusone wrote:
> > > > cjdb wrote:
> > > > > It's not immediately clear that `regular_invocable` lives in `invocable.h`. Please either:
> > > > > 
> > > > > * split the file up further
> > > > > * consolidate with the other callable concepts and call the header `callable.h`
> > > > > 
> > > > > I'm indifferent to the chosen approach, but please be consistent in the approach you choose.
> > > > I prefer not to split `invocable` from `regular_invocable`, because they are (as far as libc++ is concerned) equivalent.
> > > > However, it's good to split this concept from `predicate` and `relation` (and the other concepts equivalent to `relation`) because //this// file doesn't depend on `__boolean_testable` but //those// concepts do.
> > > :shrug: I guess the same argument that I made for `equality_comparable.h` vs `equality_comparable_with.h` could be made here re not splitting further. Though, I'm not sure why you're opposed to `__boolean_testable` being in this header. Ditto for `object.h`, but we should keep the conversation in one place.
> > `__boolean_testable` is an ancestor of both `equality_comparable` and `predicate`.
> Yes, but `__boolean_testable` has its own header. I'm trying to work out why you're opposed to combining the all the callables into a single header, and you pointed at `__boolean_testable`.
I must not understand what you're asking, because it //seems// like this is really basic.
My initial PR had:
`invocable` and `regular_invocable` go in `invocable.h`, which does not include `boolean_testable.h`.
`predicate` and `relation` and `equivalence_relation` and so on go in `relation.h`, which does include `boolean_testable.h`.

My current PR splits `predicate` into `predicate.h`, purely on the basis of name-spelling, because you asked for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107584