[libcxx-commits] [PATCH] D127226: [libc++] Simplify type_traits and use more builtins
Nikolas Klauser via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jun 24 18:00:11 PDT 2022
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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits