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

[Alan Zhao via Phabricator via libcxx-commits]

ayzhao added a comment.

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++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127226