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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a subscriber: rsmith.
ldionne added a comment.

In D97283#2606794 <https://reviews.llvm.org/D97283#2606794>, @tmatheson wrote:

>> make libc++ conditionally use just `__is_unsigned` or a combination of `__is_unsigned && !__is_enum` based on the version of Clang
>
> That's exactly what I was trying to avoid :) I'm reluctant to change the builtin for the following reasons:
>
> - Since this is marked as an Embarcadero builtin, it might break whatever that is and I'm not sure who to ask about it
> - I didn't know what the compiler support policy is so was hedging on the side of not breaking the interface
> - `__is_unsigned && !__is_enum` will work for both builtin behaviours
> - Keep the patch small
>
> If this is the preferred approach though and the above are not concerns, I can also change the builtin and add the extra `#ifdef`.

That's my preferred approach, however I think we need to bring in someone from Clang like @rsmith to discuss this. Richard, currently, the `__is_unsigned` builtin returns true for enumeration types - do you think it's reasonable to fix that bug, or should we keep it to avoid breaking folks who might rely on it?

IMO: If we think it's a bug in the standard library that `std::is_unsigned` returns true for enumerations, and if we think it's serious enough that we want to fix it (even though it might break some people who were relying on the old behaviour in tortuous ways), then I think we should fix it in Clang too. Having the two disagree does not improve anything (in fact libc++ might be amongst the only users of `__is_unsigned`). That's my opinion, but I'd like to know yours.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97283