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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

Thank you for adding all these tests!



================
Comment at: libcxx/include/type_traits:1454
 
-#if __has_keyword(__is_unsigned)
+// Before clang 13 __is_unsigned returned true for enums with signed underlying type
+#if __has_keyword(__is_unsigned) &&                                            \
----------------
With other builtins that have had issues in earlier versions of Clang, such as `__is_pointer`,  we simply disable the fast path before Clang XX. I think we should do that here too. For the most part, the people who care about this type of performance are using ToT. Also, this will greatly reduce the complexity here, so we don't add any more branches to the already complicated implementation. WDYT?


================
Comment at: libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_signed.pass.cpp:58
+
+class NotEmpty {
+  virtual ~NotEmpty();
----------------
I know this wasn't you, but I really don't like the name `NotEmpty`. Maybe call this `VirtualDestructor` or something. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97283