cmath: Skip Libc for integral types in isinf, etc.

[dexonsmith]

Summary

For std::isinf, the standard requires effectively calling isinf as
double from Libc for integral types. But integral types are never
infinite; we don't need to call Libc to return false.

Also short-circuit other functions where Libc won't have interesting
answers: signbit, fpclassify, isfinite, isnan, and isnormal.

I added correctness tests for integral types since we're no longer
deferring to Libc.

[dexonsmith]

Ping.

[hfinkel]

libcxx/include/math.h

354–356

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.

404

Maybe we should predicate this, and other infinity-related functions, on std::numeric_limits<_A1>::has_infinity. That seems more general, and safer for potential user-defined types.

[dexonsmith]

Responding to Hal's comments.

[dexonsmith]

Marking Hal's comments as "Done". Thanks for the review.

[hfinkel]

I'm happy with this now. @EricWF , @mclow.lists , thoughts?

[EricWF]

LGTM minus inline comments.

libcxx/include/math.h

354–356

@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".

[mclow.lists]

libcxx/include/math.h

404

I don't think we need to worry about user-defined types. The standard limits the parameters to "arithmetic types" and in [basic.fundamental] it lists exactly what those types are.

[dexonsmith]

Marshall, that's what I assumed originally, but I figured Hal had some non-standard-but-worth-supporting use case in mind.

Hal, what do you think?

[dexonsmith]

Eric: somehow I never got an email notification for your review :/. If Marshall is okay with the current state, I'll document that before commit.

[hfinkel]

In D31561#731498, @dexonsmith wrote:

Marshall, that's what I assumed originally, but I figured Hal had some non-standard-but-worth-supporting use case in mind.

In this case, future-proofing and mathematical precision seemed aligned. I suspect that we'll end up with fixed-point types, etc. at some point, and users already certainly have such types (although it is true that using those types with the std:: math functions is not something we're required to support).

Hal, what do you think?

I think that it is fine either way. Marshall is certainly correct, there are no types we're required to support that makes this a meaningful distinction.

[dexonsmith]

Marshall, are you fine with figuring this out in-tree? Or should I revert to using std::is_floating_point for now?

[dexonsmith]

Since I haven't heard from Marshall and Hal's fine with the less-future-proof std::is_floating_point, I'll commit that and we can iterate in tree. Just running tests.

[dexonsmith]

Committed in r301060.

libcxx/include/math.h

 
 #ifdef __cplusplus
 
 // We support including .h headers inside 'extern "C"' contexts, so switch
 // back to C++ linkage before including these C++ headers.
 extern "C++" {
 
 #include <type_traits>
+#include <limits>
 
 // signbit
 
 #ifdef signbit
 
 template <class _A1>
 _LIBCPP_ALWAYS_INLINE
 bool
 __libcpp_signbit(_A1 __lcpp_x) _NOEXCEPT
 {
     return signbit(__lcpp_x);
 }
 
 #undef signbit
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::is_integral<_A1>::value,
+    bool>::type
 signbit(_A1 __lcpp_x) _NOEXCEPT
 {
     return __libcpp_signbit((typename std::__promote<_A1>::type)__lcpp_x);
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<
+    std::is_integral<_A1>::value && std::is_signed<_A1>::value, bool>::type
+signbit(_A1 __lcpp_x) _NOEXCEPT
+{ return __lcpp_x < 0; }
+
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<
+    std::is_integral<_A1>::value && !std::is_signed<_A1>::value, bool>::type
+signbit(_A1) _NOEXCEPT
+{ return false; }
+
 #elif defined(_LIBCPP_MSVCRT)
 
 template <typename _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::is_integral<_A1>::value,
+    bool>::type
 signbit(_A1 __lcpp_x) _NOEXCEPT
 {
   return ::signbit(static_cast<typename std::__promote<_A1>::type>(__lcpp_x));
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<
+    std::is_integral<_A1>::value && std::is_signed<_A1>::value, bool>::type
+signbit(_A1 __lcpp_x) _NOEXCEPT
+{ return __lcpp_x < 0; }
+
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<
+    std::is_integral<_A1>::value && !std::is_signed<_A1>::value, bool>::type
+signbit(_A1) _NOEXCEPT
+{ return false; }
+
 #endif  // signbit
 
 // fpclassify
 
 #ifdef fpclassify
 
 template <class _A1>
 _LIBCPP_ALWAYS_INLINE
 int
 __libcpp_fpclassify(_A1 __lcpp_x) _NOEXCEPT
 {
     return fpclassify(__lcpp_x);
 }
 
 #undef fpclassify
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, int>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::is_integral<_A1>::value,
+    int>::type
 fpclassify(_A1 __lcpp_x) _NOEXCEPT
 {
     return __libcpp_fpclassify((typename std::__promote<_A1>::type)__lcpp_x);
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<std::is_integral<_A1>::value, int>::type
+fpclassify(_A1 __lcpp_x) _NOEXCEPT
+{ return __lcpp_x == 0 ? FP_ZERO : FP_NORMAL; }
+
 #elif defined(_LIBCPP_MSVCRT)
 
 template <typename _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, int>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::is_integral<_A1>::value,
+    bool>::type
 fpclassify(_A1 __lcpp_x) _NOEXCEPT
 {
   return ::fpclassify(static_cast<typename std::__promote<_A1>::type>(__lcpp_x));
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<std::is_integral<_A1>::value, int>::type
+fpclassify(_A1 __lcpp_x) _NOEXCEPT
+{ return __lcpp_x == 0 ? FP_ZERO : FP_NORMAL; }
+
 #endif  // fpclassify
 
 // isfinite
 
 #ifdef isfinite
 
 template <class _A1>
 _LIBCPP_ALWAYS_INLINE
 bool
 __libcpp_isfinite(_A1 __lcpp_x) _NOEXCEPT
 {
     return isfinite(__lcpp_x);
 }
 
 #undef isfinite
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && std::numeric_limits<_A1>::has_infinity,
+    bool>::type
 isfinite(_A1 __lcpp_x) _NOEXCEPT
 {
     return __libcpp_isfinite((typename std::__promote<_A1>::type)__lcpp_x);
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::numeric_limits<_A1>::has_infinity,
+    bool>::type
+isfinite(_A1) _NOEXCEPT
+{ return true; }
+
 #endif  // isfinite
 
 // isinf
 
 #ifdef isinf
 
 template <class _A1>
 _LIBCPP_ALWAYS_INLINE
 bool
 __libcpp_isinf(_A1 __lcpp_x) _NOEXCEPT
 {
     return isinf(__lcpp_x);
 }
 
 #undef isinf
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && std::numeric_limits<_A1>::has_infinity,
+    bool>::type
 isinf(_A1 __lcpp_x) _NOEXCEPT
 {
     return __libcpp_isinf((typename std::__promote<_A1>::type)__lcpp_x);
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::numeric_limits<_A1>::has_infinity,
+    bool>::type
+isinf(_A1) _NOEXCEPT
+{ return false; }
+
 #endif  // isinf
 
 // isnan
 
 #ifdef isnan
 
 template <class _A1>
 _LIBCPP_ALWAYS_INLINE
 bool
 __libcpp_isnan(_A1 __lcpp_x) _NOEXCEPT
 {
     return isnan(__lcpp_x);
 }
 
 #undef isnan
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::is_integral<_A1>::value,
+    bool>::type
 isnan(_A1 __lcpp_x) _NOEXCEPT
 {
     return __libcpp_isnan((typename std::__promote<_A1>::type)__lcpp_x);
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<std::is_integral<_A1>::value, bool>::type
+isnan(_A1) _NOEXCEPT
+{ return false; }
+
 #endif  // isnan
 
 // isnormal
 
 #ifdef isnormal
 
 template <class _A1>
 _LIBCPP_ALWAYS_INLINE
 bool
 __libcpp_isnormal(_A1 __lcpp_x) _NOEXCEPT
 {
     return isnormal(__lcpp_x);
 }
 
 #undef isnormal
 
 template <class _A1>
 inline _LIBCPP_INLINE_VISIBILITY
-typename std::enable_if<std::is_arithmetic<_A1>::value, bool>::type
+typename std::enable_if<
+    std::is_arithmetic<_A1>::value && !std::is_integral<_A1>::value,
+    bool>::type
 isnormal(_A1 __lcpp_x) _NOEXCEPT
 {
     return __libcpp_isnormal((typename std::__promote<_A1>::type)__lcpp_x);
 }
 
+template <class _A1>
+inline _LIBCPP_INLINE_VISIBILITY
+typename std::enable_if<std::is_integral<_A1>::value, bool>::type
+isnormal(_A1 __lcpp_x) _NOEXCEPT
+{ return __lcpp_x != 0; }
+
 #endif  // isnormal
 
 // isgreater
 
 #ifdef isgreater
 
 template <class _A1, class _A2>
 _LIBCPP_ALWAYS_INLINE

libcxx/test/std/numerics/c.math/cmath.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // <cmath>
 
 #include <cmath>
+#include <limits>
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 #include "hexfloat.h"
 
 // convertible to int/float/double/etc
 template <class T, int N=0>

 #error signbit defined
 #endif
     static_assert((std::is_same<decltype(std::signbit((float)0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::signbit((double)0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::signbit(0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::signbit((long double)0)), bool>::value), "");
     static_assert((std::is_same<decltype(signbit(Ambiguous())), Ambiguous>::value), "");
     assert(std::signbit(-1.0) == true);
+    assert(std::signbit(0u) == false);
+    assert(std::signbit(std::numeric_limits<unsigned>::max()) == false);
+    assert(std::signbit(0) == false);
+    assert(std::signbit(1) == false);
+    assert(std::signbit(-1) == true);
+    assert(std::signbit(std::numeric_limits<int>::max()) == false);
+    assert(std::signbit(std::numeric_limits<int>::min()) == true);
 }
 
 void test_fpclassify()
 {
 #ifdef fpclassify
 #error fpclassify defined
 #endif
     static_assert((std::is_same<decltype(std::fpclassify((float)0)), int>::value), "");
     static_assert((std::is_same<decltype(std::fpclassify((double)0)), int>::value), "");
     static_assert((std::is_same<decltype(std::fpclassify(0)), int>::value), "");
     static_assert((std::is_same<decltype(std::fpclassify((long double)0)), int>::value), "");
     static_assert((std::is_same<decltype(fpclassify(Ambiguous())), Ambiguous>::value), "");
     assert(std::fpclassify(-1.0) == FP_NORMAL);
+    assert(std::fpclassify(0) == FP_ZERO);
+    assert(std::fpclassify(1) == FP_NORMAL);
+    assert(std::fpclassify(-1) == FP_NORMAL);
+    assert(std::fpclassify(std::numeric_limits<int>::max()) == FP_NORMAL);
+    assert(std::fpclassify(std::numeric_limits<int>::min()) == FP_NORMAL);
 }
 
 void test_isfinite()
 {
 #ifdef isfinite
 #error isfinite defined
 #endif
     static_assert((std::is_same<decltype(std::isfinite((float)0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isfinite((double)0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isfinite(0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isfinite((long double)0)), bool>::value), "");
     static_assert((std::is_same<decltype(isfinite(Ambiguous())), Ambiguous>::value), "");
     assert(std::isfinite(-1.0) == true);
+    assert(std::isfinite(0) == true);
+    assert(std::isfinite(1) == true);
+    assert(std::isfinite(-1) == true);
+    assert(std::isfinite(std::numeric_limits<int>::max()) == true);
+    assert(std::isfinite(std::numeric_limits<int>::min()) == true);
 }
 
 void test_isnormal()
 {
 #ifdef isnormal
 #error isnormal defined
 #endif
     static_assert((std::is_same<decltype(std::isnormal((float)0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isnormal((double)0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isnormal(0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isnormal((long double)0)), bool>::value), "");
     static_assert((std::is_same<decltype(isnormal(Ambiguous())), Ambiguous>::value), "");
     assert(std::isnormal(-1.0) == true);
+    assert(std::isnormal(0) == false);
+    assert(std::isnormal(1) == true);
+    assert(std::isnormal(-1) == true);
+    assert(std::isnormal(std::numeric_limits<int>::max()) == true);
+    assert(std::isnormal(std::numeric_limits<int>::min()) == true);
 }
 
 void test_isgreater()
 {
 #ifdef isgreater
 #error isgreater defined
 #endif
     static_assert((std::is_same<decltype(std::isgreater((float)0, (float)0)), bool>::value), "");
#else
     // See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
     static_assert((std::is_same<DoubleRetType, bool>::value
                 || std::is_same<DoubleRetType, int>::value), "");
 #endif
 
     static_assert((std::is_same<decltype(std::isinf(0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isinf((long double)0)), bool>::value), "");
     assert(std::isinf(-1.0) == false);
+    assert(std::isinf(0) == false);
+    assert(std::isinf(1) == false);
+    assert(std::isinf(-1) == false);
+    assert(std::isinf(std::numeric_limits<int>::max()) == false);
+    assert(std::isinf(std::numeric_limits<int>::min()) == false);
 }
 
 void test_isless()
 {
 #ifdef isless
 #error isless defined
 #endif
     static_assert((std::is_same<decltype(std::isless((float)0, (float)0)), bool>::value), "");
#else
     // See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
     static_assert((std::is_same<DoubleRetType, bool>::value
                 || std::is_same<DoubleRetType, int>::value), "");
 #endif
 
     static_assert((std::is_same<decltype(std::isnan(0)), bool>::value), "");
     static_assert((std::is_same<decltype(std::isnan((long double)0)), bool>::value), "");
     assert(std::isnan(-1.0) == false);
+    assert(std::isnan(0) == false);
+    assert(std::isnan(1) == false);
+    assert(std::isnan(-1) == false);
+    assert(std::isnan(std::numeric_limits<int>::max()) == false);
+    assert(std::isnan(std::numeric_limits<int>::min()) == false);
 }
 
 void test_isunordered()
 {
 #ifdef isunordered
 #error isunordered defined
 #endif
     static_assert((std::is_same<decltype(std::isunordered((float)0, (float)0)), bool>::value), "");