idk icon indicating copy to clipboard operation
idk copied to clipboard

Replace `std::invoke` usages with a new `__ztd_detail::__invoke`

Open TerensTare opened this issue 3 years ago • 4 comments

<functional> is a huge header, containing (but not limited to) hash, reference_wrapper, function, invoke and much more. Most of the time all these classes are not needed at the same place at the same time, so a #include <functional> may be something expensive, especially on headers (for example reference_wrapper.hpp). Thus I propose to add a new header on ztd/idk/detail named invoke.hpp which provides an implementation of std::invoke/std::invoke_result/std::is_invocable named __ztd_detail::{__invoke, __invoke_result, __is_invocable} and replace the use of the equivalent std:: classes over the library. A reference implementation of std::invoke can be found here.

TerensTare avatar Jul 05 '21 17:07 TerensTare

Does this also mean we throw std::reference_wrapper out the window so we can never include <functional>? What about the hashes for stuff?

ThePhD avatar Jul 05 '21 21:07 ThePhD

Does this also mean we throw std::reference_wrapper out the window so we can never include <functional>?

Not necessarily. We can conditionally include <functional> if std::reference_wrapper is constexpr.

What about the hashes for stuff?

I guess we cannot "replace" std::hash so we are forced to #include <functional> on that case. On the other hand, std::invoke is a function that does not depend on user customization (like std::hash does) thus it can be (relatively) easily replaced by an equivalent function (like the proposed __ztd_detail::__invoke). I believe this will help on reducing compile times.

TerensTare avatar Jul 06 '21 08:07 TerensTare

This is on my radar, but I haven't had the energy to tackle it. Sorry; I don't mean to let you think I had forgotten! Also, as far as invoke goes, there's an EXTRA speedy implementation here:

https://github.com/eggs-cpp/invoke

ThePhD avatar Nov 09 '21 19:11 ThePhD

Sorry; I don't mean to let you think I had forgotten!

No worries, I understand that not every problem can be tackled within a second.

I looked at the implementation you linked and it looks pretty solid. However, there's something that concerns me:

I missed it at first, but std::invoke is defined in terms of std::reference_wrapper. This brings us back to square 0 given std::reference_wrapper is defined in <functional>. As I mentioned previously, conditionally #include-ing <functional> if std::reference_wrapper is constexpr can help on avoiding the header on pre-20 standards. If providing a modified implementation of eggs::invoke inside ztd is considered, ztd::<INVOKE> can further be defined in terms of ztd::reference_wrapper and explicitly document this difference from std::invoke, as it can and will break code using the standard ref-wrapper+ztd::<INVOKE> compiled with a pre-20 standard.

TerensTare avatar Nov 09 '21 20:11 TerensTare