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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D150538#4347015 <https://reviews.llvm.org/D150538#4347015>, @philnik wrote:

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

We actually discussed that. It would be indeed need to have some of the building blocks of invoke as builtins. The invoke itself might be more troublesome when WG21 changes the definition.

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

I tried to slice things differently but that seemed to fail. If you have suggestions I'm open to them.

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

The issue is that the tests for the modules test that the module contains the same declarations as provided by a "header", where "header" means `foo`, `__foo/*h`, `__fwd/foo.h`. These tests are important to validate when we add new declarations to a header we update the module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150538