[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4837
+    // Enum types should always return false (same as UTT_IsSigned).
+    return !T->isEnumeralType() && T->isUnsignedIntegerType();
 
----------------
Quuxplusone wrote:
> FWIW, I'd lose the parenthetical comment, and I'd write the conditions as respectively
> 
>     return T->isUnsignedIntegerType() && !T->isEnumeralType();
> 
>     return T->isFloatingType() || (T->isSignedIntegerType() && !T->isEnumeralType());
> 
> both because "T not being an enum type" is expected to be usually true, and because I think `(!x && y)` runs the risk of being misread as `!(x && y)`, whereas `(x && !y)` is easy on the brain.
Fair enough. I originally was thinking, if we have to perform this check roughly 50% of this time, either way, it makes sense to do the much faster check first. But, if we assume that we almost never receive an enum type, then what you're saying makes sense. I'll fix it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98104