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

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>`.

(See here.)

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 ;-)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99041