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

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a comment.

In D127226#3593385 <https://reviews.llvm.org/D127226#3593385>, @ayzhao wrote:

> In D127226#3592731 <https://reviews.llvm.org/D127226#3592731>, @philnik wrote:
>
>> In D127226#3590332 <https://reviews.llvm.org/D127226#3590332>, @ayzhao wrote:
>>
>>> This is causing build breakages in Chrome as we use `std::numeric_limits<__float128>`.
>>>
>>> After this change, `std::is_arithmetic<__float128>::value` is now `true` instead of `false` [0]. Observe that for GCC's `libstdc++`, the value is also `false`.
>>>
>>> This causes `std::numeric_limits<__float128>` to fail to compile [1]. This is because `std::numeric_limits` inherits from `__libcpp_numeric_limits`, which refers to `std::is_arithmetic` in its template parameters. The change from `false` to `true` causes `libc++` to instantiate some illegal template usages of `__float128`.
>>>
>>> [0]: https://godbolt.org/z/EjnY8Th1d  Note: at the time of this comment, the output of x86-64 clang(trunk) returned 1 - everything else returns 0.
>>> [1]: https://godbolt.org/z/94PhTsabf Note: at the time of this comment, the output of x86-64 clang(trunk) is https://pastebin.com/w0kRFqB0
>>> [2]: https://github.com/llvm/llvm-project/blob/911841f717eb8acaccf4f3deb5f85fbf6903f55f/libcxx/include/limits#L144-L145
>>
>> It's not clear to me what you are asking for. Do you think `std::is_arithmetic_v<__float128>` should return `false`?  Do you want a `numeric_limits` specialization of `__float128`?
>
> I don't mean to make a specific recommendation here as I'm not as familiar with libc++. My main point is that this patch causes `numeric_limits<__float128>` (and very likely many other non-standard types) to fail to build, and that this breakage should be fixed in one way or another. The points I made referencing `std::is_arithmetic` were to explain why this patch causes the aforementioned build breakage. Whether or not `std::is_arithmetic_v<__float128>` should return `false` is something that I'd rather have someone more experienced decide, but I will note that the behavior is now different between libc++ and libstdc++.

`is_arithmetic_v<__float128>` should be `true` if `__float128` is a compiler-supported floating-point number IMO. It should be noted that `__is_integral(__int128)` and `__is_arithmetic(__int128)` are both `true`, so I don't see why `__float128` should be any different.

However, this probably means `numeric_limits<__float128>` will need a specialisation, if it doesn't already have one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127226