[libcxx-commits] [PATCH] D148553: [libc++] Use the __is_trivially_equality_comparable builtin

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM w/ comments and tests.



================
Comment at: libcxx/include/__string/constexpr_c_functions.h:44
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 int
 __constexpr_memcmp(const _Tp* __lhs, const _Up* __rhs, size_t __count) {
+  static_assert(__libcpp_is_trivially_lexicographically_comparable<_Tp, _Up>::value,
----------------
I don't think you have any tests for this.


================
Comment at: libcxx/include/__string/constexpr_c_functions.h:55
     while (__count != 0) {
       if (*__lhs < *__rhs)
         return -1;
----------------
ldionne wrote:
> Is there a reason why we can't simply cast to `unsigned char` here when we do the comparison? That is what `__builtin_memcmp` does in spirit, and it might resolve a bunch of difficulties.
As discussed, we'd have to get rid of the `static_assert` and we don't want to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148553