[libcxx-commits] [PATCH] D99041: [libcxx] moves `std::invoke` into `__functional_base`

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a comment.

In D99041#2641549 <https://reviews.llvm.org/D99041#2641549>, @Quuxplusone wrote:

> @ldionne wrote:
>
>> Is it possible to split it into a header `__functional/invoke.h`? If not, why?
>
> FWIW, I'd strongly oppose that. If we're going to start introducing dependencies on C++17 `invoke` (where the rest of libc++ right now takes dependencies only on C++11 `__invoke`), then don't want `__invoke` and `invoke` separated from each other //at all//. If we're going to treat `invoke` as synonymous with `__invoke`, then the synonyms belong together in the same header.

That has an easy solution: we move the `__invoke` components of `__functional_base[03]` to `__functional/invoke.h` and be done with it.

> Separately, I'm opposed to creating a "second style precedent" of `__functional/foo.h` when we already have the established style convention of `__functional_foo`. (But this is trivially solved by calling a hypothetical new header `__functional_invoke` instead of `__functional/invoke.h`.)

This isn't something that's set in stone; we can change it if it's deemed to be a better approach. I, for one, would very much prefer `__functional/...`, because it means we can then have editors collapse directories that aren't relevant. Having a dozen `__functional_*` complicates the top-level directory (especially if you look forward to a world where we also have `__type_traits_*`, `__utility_*`, etc., which I'm very much in favour of).

> Pragmatically, `__invoke` depends on `__invoke_return` etc., which right now are scattered among `__functional_base` and `__functional_base_03`. So moving `invoke` into a new header outside of `__functional_base`, //without// making that new header //itself// include `__functional_base`, seems like a relatively major task. (And of course if the new header //does// depend on `__functional_base` then you haven't gained anything, relative to this PR as-is.)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99041