[libcxx-commits] [PATCH] D97283: [libcxx][type_traits] is_unsigned is false for enum types

Fri Mar 5 22:39:29 PST 2021

zoecarver added a comment.

Thank you for adding all these tests, they're great!

@tmatheson was this causing problems for you in the wild, or did you just notice that there was an issue? If the latter, I think we should just take the tests from this patch and only enable them on ToT clang for the next six months. That way we don't add (more) branches to this code that we'll inevitably forget to remove in six months.

================
Comment at: libcxx/include/type_traits:1459
 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) &&
+                                                          !__is_enum(_Tp);
----------------
zoecarver wrote:
> ldionne wrote:
> > Have we asked the compiler folks why `__is_unsigned` is implemented that way? This looks like a simple Clang bug to me.
> > 
> > @zoecarver  did you implement this intrinsic or was it there already?
> Yeah, I [[ https://github.com/llvm/llvm-project/commit/511dbd83d63ce0cf95e3ac7d07eb9444d92a02e8# | fixed this for __is_signed ]]. I think I just need to update __is_unsigned as well. I might be able to put up a patch for that later today. 
Here you go: D98104.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97283