[libcxx-commits] [PATCH] D67900: [libc++] Use builtin type traits whenever possible

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Thanks for your patience! This is looking good, but I do have some comments.



================
Comment at: libcxx/include/type_traits:561
 template <class _Tp, class _Up>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_same_v
     = is_same<_Tp, _Up>::value;
----------------
I think it's reasonable to change the `meow_v` variable templates to use intrinsics in this patch too, WDYT? Especially since you're doing it for `is_integral_v` below.


================
Comment at: libcxx/include/type_traits:660
+template <class _Tp>
+struct is_const : _BoolConstant<__is_const(_Tp)> { };
+
----------------
`_LIBCPP_TEMPLATE_VIS` (here and elsewhere)


================
Comment at: libcxx/include/type_traits:758
+template <class _Tp>
+struct __libcpp_is_void : _BoolConstant<__is_same(_Tp, void)> { };
+
----------------
I would remove that altogether since it's not used.


================
Comment at: libcxx/include/type_traits:766
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_void
     : public __libcpp_is_void<typename remove_cv<_Tp>::type> {};
 
----------------
We could use this instead:

```
template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_void : is_same<typename remove_cv<_Tp::type, void> {};
```

Arguably, only old compilers won't use the intrinsic, so I think this simplification (especially the removal of `__libcpp_is_void`) is worth it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67900