[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D147175#4251852 <https://reviews.llvm.org/D147175#4251852>, @philnik wrote:

> In D147175#4251076 <https://reviews.llvm.org/D147175#4251076>, @aaron.ballman wrote:
>
>> One thing I can't quite determine is whether we expect to call this type trait often or not. Are either of the uses in libc++ going to be instantiated frequently? I'm trying to figure out whether we want the current implementation of `HasNonDeletedDefaultedEqualityComparison()` or whether we should bite the bullet up front and add another bit to `CXXRecordDecl::DefinitionData` so we compute this property once as the class is being formed and don't have to loop over all of the methods and bases each time. We do this for other special member functions, as with: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclCXX.h#L695
>
> We currently use `__is_trivially_equality_comparable` only for `std::equal` (and I'm working on using it for `std::find`), and I don't think there are a lot more places where we can use it. It's also only relevant for cases where we have raw pointers. Would the `enable_if` be evaluated, even if the arguments aren't raw pointers in the example below?
>
>   template <class T, class U, enable_if_t<is_same_v<remove_cvref_t<T>, remove_cvref_t<U>> && __is_trivially_equality_comparable(T)>>
>   bool equal(T* first1, T* last1, U* first2);
>   
>   <non-raw-pointer-overload>
>
> If not, then I don't think it makes sense to save the information, since it will most likely not be evaluated more than once or twice per type.

I believe that we deduce the type of `T` before we can instantiate any of the deduced template parameters, so I think the `enable_if` is not evaluated unless `T` is known to have a raw pointer type. Based on that, I don't think we need to steal any bits anywhere right now -- we can use the current approach and if it turns up on the hot path at some point, we can address the concern then. The clang bits LGTM, but I'll hold off on signing off until someone from libc++ approves as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175