[libcxx-commits] [PATCH] D97283: [libcxx][type_traits] is_unsigned is false for enum types
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Mar 15 06:36:39 PDT 2021
Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.
Ping @zoecarver because I think he needs to be one of the approvers here. But once it LGTH, I'm happy to be the second accepter.
Comment at: libcxx/include/type_traits:1455-1456
+// Before clang 13 __is_unsigned returned true for enums with signed underlying type
+#if __has_keyword(__is_unsigned) && \
+ (_LIBCPP_CLANG_VER >= 1300 || __has_keyword(__is_enum))
AFAIK, `_LIBCPP_CLANG_VER >= 1300` logically implies `__has_keyword(__is_enum)`. Therefore, just testing the latter is sufficient. So I was going to suggest the edit to
#if __has_keyword(__is_unsigned) && __has_keyword(__is_enum)
- On compilers which //are// Clang, all supported Clangs definitely have `__is_enum`.
- On compilers which are //not// Clang, we never need to use `__is_enum`.
So it doesn't seem that that part of the condition is actually used, either.
I think the original condition `#if __has_keyword(__is_unsigned)` is now the //correct// condition. (And revert the comments on the `#else` and `#endif` lines accordingly.)
Comment at: libcxx/include/type_traits:1471-1472
template <class _Tp>
-_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_unsigned_v = __is_unsigned(_Tp);
+_LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_unsigned_v = __is_unsigned(_Tp) &&
Please duplicate the `#if _LIBCPP_CLANG_VER < 1300` workaround here, i.e., don't bother to use `__is_enum` unless we're on Clang <13.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits