[libcxx-commits] [PATCH] D99041: [libcxx] moves `std::invoke` into `__functional_base`
Christopher Di Bella via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Mar 21 12:52:10 PDT 2021
cjdb added a comment.
In D99041#2640282 <https://reviews.llvm.org/D99041#2640282>, @Quuxplusone wrote:
> - It's confusing to me that the header `__invoke` doesn't actually define `__invoke`, but rather `invoke`.
🤷 that's already `__functional_base`. I named it `__invoke` instead of `invoke` so users don't accidentally include it. I could move it to `__functional/invoke`, which is 1000x clearer IMNSHO (see below).
> - Most libc++-internal consumers actually care about `__invoke`, not `invoke`. (`__invoke` is so fundamental that I would have expected it in `type_traits` with the rest of the fundamentals, but it looks like it's in `__functional_base` instead.)
Yeah, that's a good musing. See below.
> - I suggest that you just move `invoke` into the existing `__functional_base` header, alongside `ref` et al. That'll be a much smaller patch, and also unifies `invoke` and `__invoke` in the same place, which is helpful to maintainers.
Good observation! I was under the impression that `<type_traits>` depended on `__functional_base` and not the other way around, so I've applied this in spite of my opinion below.
> - I'd like your opinion (and maybe @tcanens too?) on whether libc++ could just use `_VSTD::__invoke` inside all the invocable concepts, instead of `_VSTD::invoke`. Using `__invoke` wouldn't help much AFAICT. It would //very slightly, negligibly// lessen the compile-time burden, and it would reduce the coupling between these concepts and the rest of the C++17 library. (This would merely help libc++ support a "C++14 plus concepts" mode, which is a crazy and pointless idea.) But I'm still wondering whether it would hurt.
Considered this, but chickened out because a change to `invoke` might not reflect updates to `invocable` without manual intervention. I'd prefer to keep concepts verbatim, even though I strongly suspect any changes to `invoke` will likely go through //`INVOKE`// Also note that @ldionne has strong opinions on keeping things as close to wording as possible.
> - We should try to get to the point where no C++ standard header includes `<functional>`.
Agreed, +100, etc.. I'm considering writing to libc++ mailing to suggest we move all names into their own headers (e.g. `<__functional/invoke>`, `<__functional/function>`, etc.) and the standard headers just include `<__header_name/*>` (where `*` is a placeholder and //not// what we really include). I don't have the time to do this myself, but will review any and all patches that incrementally move us in that direction. It also gives us a rare opportunity to review the libc++ clang-format style, if the group wants change (I'm ambivalent, but it's a point worth noting //before// we start the endeavour).
> I might even get nerdsniped into making that patch...
No objections from me. See above ;-)
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits