[libcxx-commits] [PATCH] D67900: [libc++] Use builtin type traits whenever possible

[Tiancong Wang via libcxx-commits]

Thank you for the reply! Does it make we want to revert this patch, or send
another patch to fix the behavior in this patch? Sorry that I don't know
what to expect from here.

Best wishes,
Tiancong


On Wed, Apr 1, 2020 at 12:12 AM Eric Fiselier <eric at efcs.ca> wrote:

> We should revert the change for `is_arithmetic` and `is_floating_point`.
>
> Marshall and I have discussed the __float128 issue previously, but we
> couldn't reach an agreement that it should be handled as an extension.
> For that reason, I think we should go back to the old behavior.
>
> /Eric
>
> On Wed, Apr 1, 2020 at 1:00 AM Zoe Carver via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> zoecarver added a comment.
>>
>> @tcwang here's you're reproducer: https://godbolt.org/z/RJtTBG
>>
>> Looks like we don't have any `__float128` tests not only in numeric
>> limits but in libc++ altogether.
>>
>> Anyway, to address the problem: `is_arithmetic_v<__float128>` is true
>> after this patch and false before this patch. It seems like returning true
>> is the correct behavior so the change should probably happen in numeric
>> limits. It's not entirely clear what that change should be, though. Do we
>> want to support `__float128` in `numeric_limits`? We don't seem to support
>> it anywhere else. Maybe add a `static_assert` so that future bugs are
>> easier to track down?
>>
>> In the meantime, `numeric_limits` just returned `0` for `numeric_limits<
>> __float128 >::min()` so maybe you can replace the current use with that?
>>
>>
>> Repository:
>>   rG LLVM Github Monorepo
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D67900/new/
>>
>> https://reviews.llvm.org/D67900
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200401/7ac28771/attachment-0001.html>