[libc++][math.h] move #undefs to the top and guard explicitly against MSVCRT instead

[ldionne]

LGTM pending CI too.

All in all, this whole stack is pretty much a NFC refactoring, but it makes things a lot nicer. Thanks!

libcxx/include/math.h

304–306

Why isn't this below?

323

Here and for all the other ones. We don't gain anything from these comments anymore, they just clutter the code.

[EricWF]

This changes behavior.

Currently libc++ only provides its own definitions of a math function if there's a macro which is normally used to define it.
Now libc++ always provides those overloads. That could cause redefinition errors.

I know there's been a lot of churn in this header recently , so maybe the diff I'm reading is out of date, but please verify that's the case.

[ldionne]

In D137502#3941596, @EricWF wrote:

This changes behavior.

Currently libc++ only provides its own definitions of a math function if there's a macro which is normally used to define it.
Now libc++ always provides those overloads. That could cause redefinition errors.

I know there's been a lot of churn in this header recently , so maybe the diff I'm reading is out of date, but please verify that's the case.

Let's talk about signbit just to simplify things, but this applies to all of them AFAICT. My understanding is that C implementations are required to make this a macro: https://en.cppreference.com/w/c/numeric/math/signbit

If that were not the case, previously we would not have been defining signbit, so we would have been non-confirming because C's signbit only works with floating point, whereas C++'s signbit is supposed to work with integral types as well: https://en.cppreference.com/w/cpp/numeric/math/signbit

In other words, I'm not sure there are many C implementations that define them as functions, but I may be wrong. I know GCC and the macOS SDK both define them as macros.

[ldionne]

In addition, if a library does not define e.g. isnan at all (neither as a macro nor as a function), our <math.h> will currently do the wrong thing because we won't define isnan at all. After this patch, we'll define it using the builtins, which is what we want to be doing. This makes the library easier to port to freestanding platforms.

(This applies to all the functions touched by this patch, of course)

[philnik]

• Rebased
• Address comments

[ldionne]

This seems correct to me. I think it's not impossible that this will cause issues on some more obscure platforms that we don't know about and don't officially support, however I see no way of really figuring out what the issues would be without landing it and waiting to see whether it's actually causing problems. But from the CI and the few platforms we have downstream, this should not be a problem.

Adding libc++ vendors just for awareness.

[philnik]

Rebased

libcxx/include/math.h

 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 #include_next <math.h>
 
 #ifdef __cplusplus
 
+#    ifdef fpclassify
+#      undef fpclassify
+#    endif // fpclassify
+
 // 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/promote.h>
 #include <limits>
 #include <stdlib.h>
 #include <type_traits>
 
-// signbit
-
 #    ifdef signbit
 #      undef signbit
 #    endif // signbit
 
+#    ifdef isfinite
+#      undef isfinite
+#    endif // isfinite
+
+#    ifdef isinf
+#      undef isinf
+#    endif // isinf
+
+#    ifdef isnan
+#      undef isnan
+#    endif // isnan
+
+#    ifdef isnormal
+#      undef isnormal
+#    endif // isnormal
+
+#    ifdef isgreater
+#      undef isgreater
+#    endif // isgreater
+
+#    ifdef isgreaterequal
+#      undef isgreaterequal
+#    endif // isgreaterequal
+
+#    ifdef isless
+#      undef isless
+#    endif // isless
+
+#    ifdef islessequal
+#      undef islessequal
+#    endif // islessequal
+
+#    ifdef islessgreater
+#      undef islessgreater
+#    endif // islessgreater
+
+#    ifdef isunordered
+#      undef isunordered
+#    endif // isunordered
+
+// signbit
+
 template <class _A1, std::__enable_if_t<std::is_floating_point<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool signbit(_A1 __x) _NOEXCEPT {
   return __builtin_signbit(__x);
 }
 
 template <class _A1, std::__enable_if_t<std::is_integral<_A1>::value && std::is_signed<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool signbit(_A1 __x) _NOEXCEPT {
   return __x < 0;
 }
 
 template <class _A1, std::__enable_if_t<std::is_integral<_A1>::value && !std::is_signed<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool signbit(_A1) _NOEXCEPT {
   return false;
 }
 
 // fpclassify
 
-#    ifdef fpclassify
-#      undef fpclassify
-#    endif // fpclassify
-
 template <class _A1, std::__enable_if_t<std::is_floating_point<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI int fpclassify(_A1 __x) _NOEXCEPT {
   return __builtin_fpclassify(FP_NAN, FP_INFINITE, FP_NORMAL, FP_SUBNORMAL, FP_ZERO, __x);
 }
 
 template <class _A1, std::__enable_if_t<std::is_integral<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI int fpclassify(_A1 __x) _NOEXCEPT {
   return __x == 0 ? FP_ZERO : FP_NORMAL;
 }
 
-// isfinite
+// The MSVC runtime already provides these functions as templates
+#ifndef _LIBCPP_MSVCRT
 
-#    ifdef isfinite
-#      undef isfinite
+// isfinite
 
 template <class _A1,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::numeric_limits<_A1>::has_infinity, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isfinite(_A1 __x) _NOEXCEPT {
   return __builtin_isfinite((typename std::__promote<_A1>::type)__x);
 }
 
 template <class _A1,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && !std::numeric_limits<_A1>::has_infinity, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isfinite(_A1) _NOEXCEPT {
   return true;
 }
 
-#    endif // isfinite
-
 // isinf
 
-#    ifdef isinf
-#      undef isinf
-
 template <class _A1,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::numeric_limits<_A1>::has_infinity, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isinf(_A1 __x) _NOEXCEPT {
   return __builtin_isinf((typename std::__promote<_A1>::type)__x);
 }
 
 template <class _A1>
 inline _LIBCPP_HIDE_FROM_ABI
     typename std::enable_if< std::is_arithmetic<_A1>::value && !std::numeric_limits<_A1>::has_infinity, bool>::type
     isinf(_A1) _NOEXCEPT {
   return false;
 }
 
 #      ifdef _LIBCPP_PREFERRED_OVERLOAD
 inline _LIBCPP_HIDE_FROM_ABI bool isinf(float __x) _NOEXCEPT { return __builtin_isinf(__x); }
 
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool isinf(double __x) _NOEXCEPT { return __builtin_isinf(__x); }
 
 inline _LIBCPP_HIDE_FROM_ABI bool isinf(long double __x) _NOEXCEPT { return __builtin_isinf(__x); }
 #      endif
 
-#    endif // isinf
-
 // isnan
 
-#    ifdef isnan
-#      undef isnan
-
 template <class _A1, std::__enable_if_t<std::is_floating_point<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isnan(_A1 __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
 
 template <class _A1, std::__enable_if_t<std::is_integral<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isnan(_A1) _NOEXCEPT {
   return false;
 }
 
 #      ifdef _LIBCPP_PREFERRED_OVERLOAD
 inline _LIBCPP_HIDE_FROM_ABI bool isnan(float __x) _NOEXCEPT { return __builtin_isnan(__x); }
 
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool isnan(double __x) _NOEXCEPT { return __builtin_isnan(__x); }
 
 inline _LIBCPP_HIDE_FROM_ABI bool isnan(long double __x) _NOEXCEPT { return __builtin_isnan(__x); }
 #      endif
 
-#    endif // isnan
-
 // isnormal
 
-#    ifdef isnormal
-#      undef isnormal
-
 template <class _A1, std::__enable_if_t<std::is_floating_point<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isnormal(_A1 __x) _NOEXCEPT {
   return __builtin_isnormal(__x);
 }
 
 template <class _A1, std::__enable_if_t<std::is_integral<_A1>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isnormal(_A1 __x) _NOEXCEPT {
   return __x != 0;
 }
 
-#    endif // isnormal
-
 // isgreater
 
-#    ifdef isgreater
-#      undef isgreater
-
 template <class _A1,
           class _A2,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::is_arithmetic<_A2>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isgreater(_A1 __x, _A2 __y) _NOEXCEPT {
   typedef typename std::__promote<_A1, _A2>::type type;
   return __builtin_isgreater((type)__x, (type)__y);
 }
 
-#    endif // isgreater
-
 // isgreaterequal
 
-#    ifdef isgreaterequal
-#      undef isgreaterequal
-
 template <class _A1,
           class _A2,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::is_arithmetic<_A2>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isgreaterequal(_A1 __x, _A2 __y) _NOEXCEPT {
   typedef typename std::__promote<_A1, _A2>::type type;
   return __builtin_isgreaterequal((type)__x, (type)__y);
 }
 
-#    endif // isgreaterequal
-
 // isless
 
-#    ifdef isless
-#      undef isless
-
 template <class _A1,
           class _A2,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::is_arithmetic<_A2>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isless(_A1 __x, _A2 __y) _NOEXCEPT {
   typedef typename std::__promote<_A1, _A2>::type type;
   return __builtin_isless((type)__x, (type)__y);
 }
 
-#    endif // isless
-
 // islessequal
 
-#    ifdef islessequal
-#      undef islessequal
-
 template <class _A1,
           class _A2,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::is_arithmetic<_A2>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool islessequal(_A1 __x, _A2 __y) _NOEXCEPT {
   typedef typename std::__promote<_A1, _A2>::type type;
   return __builtin_islessequal((type)__x, (type)__y);
 }
 
-#    endif // islessequal
-
 // islessgreater
 
-#    ifdef islessgreater
-#      undef islessgreater
-
 template <class _A1,
           class _A2,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::is_arithmetic<_A2>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool islessgreater(_A1 __x, _A2 __y) _NOEXCEPT {
   typedef typename std::__promote<_A1, _A2>::type type;
   return __builtin_islessgreater((type)__x, (type)__y);
 }
 
-#    endif // islessgreater
-
 // isunordered
 
-#    ifdef isunordered
-#      undef isunordered
-
 template <class _A1,
           class _A2,
           std::__enable_if_t<std::is_arithmetic<_A1>::value && std::is_arithmetic<_A2>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI bool isunordered(_A1 __x, _A2 __y) _NOEXCEPT {
   typedef typename std::__promote<_A1, _A2>::type type;
   return __builtin_isunordered((type)__x, (type)__y);
 }
 
-#    endif // isunordered
+#endif // _LIBCPP_MSVCRT
 
 // abs
 //
 // handled in stdlib.h
 
 // div
 //
 // handled in stdlib.h