[libcxx-commits] [PATCH] D127226: [libc++] Simplify type_traits and use more builtins

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D127226#3609649 <https://reviews.llvm.org/D127226#3609649>, @manojgupta wrote:

> @philnik @ldionne Can this patch please be reverted?  There are clear test cases that it breaks.

I agree that we should fix the problem, but I don't know how. The right approach seems to be to return true for `is_arithmetic_v<__float128>` and add a `__float128` specialization to `numeric_limits`. The problem there is that clang doesn't provide the macros to implement it properly. That would mean we have to implement it on a best-effort basis, which doesn't sound great IMO. The other option would be to return false for `is_arithmetic_v<__float128>`, which is obviously a lie. Neither option sounds great. I think both the current state of `is_arithmetic_v<__float128> == true` and the old state of `is_arithmetic_v<__float128> == false` are fundamentally broken. We don't have any tests for either of them, which means that it's a bug waiting to happen.
I will talk to @ldionne about this next week and after that either partially revert this patch and add regression tests or add a specialization to `numeric_limits`. IMO we shouldn't revert it right now, since it would only change the problem we have and not remove a problem. If you can't wait until Wednesday or Thursday you can always revert the diff downstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127226