<div dir="ltr">We should revert the change for `is_arithmetic` and `is_floating_point`.<div><br></div><div>Marshall and I have discussed the __float128 issue previously, but we couldn't reach an agreement that it should be handled as an extension.</div><div>For that reason, I think we should go back to the old behavior.</div><div><br></div><div>/Eric</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 1, 2020 at 1:00 AM Zoe Carver via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">zoecarver added a comment.<br>
<br>
@tcwang here's you're reproducer: <a href="https://godbolt.org/z/RJtTBG" rel="noreferrer" target="_blank">https://godbolt.org/z/RJtTBG</a><br>
<br>
Looks like we don't have any `__float128` tests not only in numeric limits but in libc++ altogether.<br>
<br>
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?<br>
<br>
In the meantime, `numeric_limits` just returned `0` for `numeric_limits< __float128 >::min()` so maybe you can replace the current use with that?<br>
<br>
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D67900/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D67900/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D67900" rel="noreferrer" target="_blank">https://reviews.llvm.org/D67900</a><br>
<br>
<br>
<br>
</blockquote></div>