EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land.
LGTM minus inline comments. ================ Comment at: libcxx/include/math.h:354 inline _LIBCPP_INLINE_VISIBILITY -typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type +typename std::enable_if<std::is_floating_point<_A1>::value, bool>::type signbit(_A1 __lcpp_x) _NOEXCEPT ---------------- hfinkel wrote: > I think that it would be safer / easier to understand if we used > !std::is_integral here. It is true for all standard types that > std::is_arithmetic implies is_integral or is_floating_point, but it seems > likely that some implementations have arithemtic types that are neither (e.g. > fixed-point types), and user-defined types seem likely. @dexonsmith Could you please codify this point in a comment above the function. My initial reaction was "Why aren't we using `std::is_floating_point` instead". https://reviews.llvm.org/D31561 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
