[libcxx-commits] [clang] [libcxx] [Clang] Add __builtin_invoke and detect std::invoke as a builtin (PR #116709)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

> > 1. I'm not convinced the library part is true. The reality is that we support Clang and GCC, and if they both support the builtins (or provide different ones for the same feature) we remove our fallback implementations and thus reducing the complexity for libc++. GCC 15 already adds `__is_invocable`, so we would be able to at least simplify our traits. Whether the complexity is too high for the compiler is something the compiler maintainers should judge IMO. FWIW IMO The compiler implementations is actually much easier to understand than the mess we have in the library.
> 
> Does GCC implement (or have plans to implement) `__builtin_invoke`?

Not that I'm aware of.

> > 2. True. I'm not sure this is a huge burden though.
> 
> Well, our tests for `std::invoke` are not simple, and we'd have to do the same in Clang if we want to have the same amount of coverage.

AFAICT we have one test for `invoke` plus a constexpr test which should really just be rolled into the generic `invoke` test. That test is ~400 LoC, which is definitely not nothing, but also not unmanageable IMO. We could probably also simplify that by not explicitly listing every cv combination but instead using `types::for_each`.

> > 3. IMO that's not the case. We call a builtin. If that builtin is wrong we file a bug against the compiler. I don't really see the confusion here. It's exactly the same as with type traits.
> 
> Your PR description says that the compiler recognizes `std::invoke` itself as a builtin, is that right? It makes a difference to me whether `std::invoke` is a builtin directly or `std::invoke` calls `__builtin_invoke`.

Yes, it does. If that is the main point of contention we can move that into a separate patch. I don't expect the compile times to differ significantly between recognizing `std::invoke` as a builtin itself and having `std::invoke` use `__builtin_invoke` as the implementation. It would still be nice for improved debug performance and reduced debug info though.

What exactly is the difference you perceive between the two? I mean, sure, there is one, but if `std::invoke` just always calls `__builtin_invoke` there really isn't a meaningful one.

> > 4. I don't think this is just "technically pre-existing". There are bugs in the compiler builtins. In some cases we've waited until these bugs are fixed to use a builtin trait, but when the bug was pre-existing for a significant amount of time we didn't bother trying to work around it in the library. I'm not sure what you mean by "complex simplification".
> 
> Sorry, I meant complex specification. I mean that `invoke` is not like `is_constructible` -- it may be updated in the future. The updates would be to the library wording, and I'd find it really weird to have to go change the compiler builtin to account for what's effectively a library change.

Is your main concern here that we'd be limited in the library what we can change without the changing the compiler? If that is the case, would your concerns be alleviated if we could opt out of the builtin recognition for specific overloads, e.g. through an attribute?


https://github.com/llvm/llvm-project/pull/116709