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

[Tomas Matheson via Phabricator via libcxx-commits]

tmatheson added inline comments.


================
Comment at: libcxx/include/type_traits:1461
+                                                          !__is_enum(_Tp);
 #endif
 
----------------
tmatheson wrote:
> Quuxplusone wrote:
> > Quuxplusone wrote:
> > > FWIW, I would prefer to hew closer to the actual standardese in https://eel.is/c++draft/meta#tab:meta.unary.prop and just use `__is_unsigned(_Tp) && is_arithmetic<_Tp>::value`.
> > > 
> > > It's also definitely worth opening a discussion with compiler vendors — it'd be great if every compiler's `__is_enum` builtin were updated to reflect the standard semantics, since that is literally the entire point of that builtin. :P
> > > 
> > (oops, of course I meant `s/__is_enum/__is_{un,}signed/` in my second paragraph)
> I agree it would read better with is_arithmetic, but I got the impression from https://reviews.llvm.org/D67900 that we should be using builtins where to keep compile times down. is_arithmetic is already used for the alternative implementation below, in the `#else // __has_keyword(__is_unsigned)` block. So it would make both implementations very similar to switch.
> It's also definitely worth opening a discussion with compiler vendors — it'd be great if every compiler's __is_unsigned builtin were updated to reflect the standard semantics, since that is literally the entire point of that builtin. :P

Are there compilers other than clang that use the same spelling for the builtin? Introducing version incompatibilities seems worse to me than sticking with slightly weird builtin semantics, but it is worth discussing. The code as written should continue to work if the semantics of __is_unsigned are updated in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97283