[PATCH] D130867: [clang] adds builtin `std::invoke` and `std::invoke_r`

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think this is quite interesting, but it also raises the obvious question of: where do we stop? I think the following elements need to be taken into account:

1. Implementing more stuff in the compiler adds complexity to the compiler, and does not decrease the complexity of the library because libc++ still needs to implement `invoke` on other compilers.
2. It duplicates the tests. The test coverage added by this patch already exists in libc++ (or it should if we're missing it). This patch also seems to be only testing in C++17 mode, and only at compile-time. So technically, this has less coverage than libc++'s own tests for the same thing.
3. Duplicating functionality in libc++ and in the compiler is confusing. For instance, it sets up the stage for people not knowing where a bug in `std::invoke` should be fixed. Is it in the compiler? In the library? In both? And the set of folks who can fix a bug in the library is not the same as the set of folks who can do it in the compiler.

Regarding the benefits of this patch, do I understand correctly that it had a roughly 1 second improvement in total User Time for libc++, and roughly a 3 seconds regression with libstdc++? If my math's right, that's a roughly 0.1% improvement on libc++ and a 0.6% pessimization on libstdc++, both of which I would argue are kind of in the noise. Also, ranges-v3 is not a typical code base -- it uses metaprogramming and `invoke` more than a traditional C++ code base, so the overall effect of this is likely to be less than that on a regular code base. Does this make sense, or am I mis interpreting the data?

So I think this is quite interesting, however I do question the benefits this provides in relation with its downsides. In particular, I am not sure that overriding non-trivial libc++ code like this is desirable. We're extremely happy with things like type traits (whose specification is super simple) being implemented in the compiler, but I'm not so sure about things like `std::invoke`, or even `std::tuple`. Instead, I'd personally be more interested in having builtins that we can use *from our implementation* to simplify various parts of the implementation (and make it faster). Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130867