[libcxx-commits] [PATCH] D150538: [libc++] Untangles invoke.

Tue May 16 10:41:27 PDT 2023

philnik added a comment.

In D150538#4346735 <https://reviews.llvm.org/D150538#4346735>, @ldionne wrote:

> In D150538#4342965 <https://reviews.llvm.org/D150538#4342965>, @philnik wrote:
>
>> I wouldn't consider this disentangled, since `__invoke` and the type traits are still very much dependent on each other.
>
> You were the one to add the comment about disentangling that code in D126593 <https://reviews.llvm.org/D126593>. Did you have something else in mind back then?

In a perfect world we'd have builtins for the traits and `invoke`, but that's non-trivial to implement (at least for me).

>> I also don't think that it makes sense to move `__invoke` into the `type_traits` module.
>
> I also agree this is kind of weird, but it seems like it is necessary. Another option would be to split into yet another header like `__functional/invoke_base.h` or `invoke_impl.h` or whatever. I do like the fact that this patch gets us to a point where the type traits are separated from the main `std::invoke` functionality. I don't think it's a *must*, but I kind of like it.

It's nice that the public interface is separated from the type traits, but I //really// don't like moving `__invoke` back into `<type_traits>`, since it shouldn't be there.

> I think this patch unblocked things for @Mordante with his work on the std module, so I think we need to find a way to unblock this, it's not just a refactoring for the sake of it.

I don't really understand how this is blocking though. Shouldn't it be enough to include `<type_traits>` and `<functional>` to implement the respective modules? All the symbols should still be available.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150538