-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[libc++] Workaround for a bug of overloads in MS UCRT's <math.h>
#149234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
MS UCRT seems confused on the status of LWG1327, and still provides pre-LWG1327 overload set the related math functions, which can't handle integer types as required. It is probably that UCRT won't fixed this in a near future, per https://developercommunity.visualstudio.com/t/10294165. Before C++20, libc++ worked around this bug by relying on `-fdelayed-template-parsing`. However, this non-conforming option is off by default since C++20. I think we should use `requires` instead.
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesMS UCRT seems confused on the status of LWG1327, and still provides pre-LWG1327 overload set the related math functions, which can't handle integer types as required. It is probably that UCRT won't fixed this in a near future, per Before C++20, libc++ worked around this bug by relying on Fixes #70225. Full diff: https://github.com/llvm/llvm-project/pull/149234.diff 5 Files Affected:
diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 4a6e58c6da8ad..00db2a8289fb3 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -189,6 +189,82 @@ template <class _A1, class _A2, __enable_if_t<is_arithmetic<_A1>::value && is_ar
return __builtin_isunordered((type)__x, (type)__y);
}
+// MS UCRT incorrectly defines some functions in a way not working with integer types. Until C++20, this was worked
+// around by -fdelayed-template-parsing. Since C++20, we can use standard feature "requires" instead.
+
+// TODO: Remove the workaround once UCRT fixes these functions. Note that this doesn't seem planned as of 2025-07 per
+// https://developercommunity.visualstudio.com/t/10294165.
+
+#if defined(_LIBCPP_MSVCRT) && _LIBCPP_STD_VER >= 20
+namespace __ucrt {
+template <class _A1>
+ requires is_integral_v<_A1>
+[[nodiscard]] inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isfinite(_A1) noexcept {
+ return true;
+}
+
+template <class _A1>
+ requires is_integral_v<_A1>
+[[nodiscard]] inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(_A1) noexcept {
+ return false;
+}
+
+template <class _A1>
+ requires is_integral_v<_A1>
+[[nodiscard]] inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(_A1) noexcept {
+ return false;
+}
+
+template <class _A1>
+ requires is_integral_v<_A1>
+[[nodiscard]] inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnormal(_A1 __x) noexcept {
+ return __x != 0;
+}
+
+template <class _A1, class _A2>
+ requires is_arithmetic_v<_A1> && is_arithmetic_v<_A2>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool isgreater(_A1 __x, _A2 __y) noexcept {
+ using type = __promote_t<_A1, _A2>;
+ return __builtin_isgreater((type)__x, (type)__y);
+}
+
+template <class _A1, class _A2>
+ requires is_arithmetic_v<_A1> && is_arithmetic_v<_A2>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool isgreaterequal(_A1 __x, _A2 __y) noexcept {
+ using type = __promote_t<_A1, _A2>;
+ return __builtin_isgreaterequal((type)__x, (type)__y);
+}
+
+template <class _A1, class _A2>
+ requires is_arithmetic_v<_A1> && is_arithmetic_v<_A2>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool isless(_A1 __x, _A2 __y) noexcept {
+ using type = __promote_t<_A1, _A2>;
+ return __builtin_isless((type)__x, (type)__y);
+}
+
+template <class _A1, class _A2>
+ requires is_arithmetic_v<_A1> && is_arithmetic_v<_A2>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool islessequal(_A1 __x, _A2 __y) noexcept {
+ using type = __promote_t<_A1, _A2>;
+ return __builtin_islessequal((type)__x, (type)__y);
+}
+
+template <class _A1, class _A2>
+ requires is_arithmetic_v<_A1> && is_arithmetic_v<_A2>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool islessgreater(_A1 __x, _A2 __y) noexcept {
+ using type = __promote_t<_A1, _A2>;
+ return __builtin_islessgreater((type)__x, (type)__y);
+}
+
+template <class _A1, class _A2>
+ requires is_arithmetic_v<_A1> && is_arithmetic_v<_A2>
+[[nodiscard]] inline _LIBCPP_HIDE_FROM_ABI bool isunordered(_A1 __x, _A2 __y) noexcept {
+ using type = __promote_t<_A1, _A2>;
+ return __builtin_isunordered((type)__x, (type)__y);
+}
+} // namespace __ucrt
+#endif
+
} // namespace __math
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/math.h b/libcxx/include/math.h
index 929bef6385043..f9e1b4ef2e352 100644
--- a/libcxx/include/math.h
+++ b/libcxx/include/math.h
@@ -427,6 +427,23 @@ using std::__math::islessgreater;
using std::__math::isnan;
using std::__math::isnormal;
using std::__math::isunordered;
+# elif _LIBCPP_STD_VER >= 20
+// MS UCRT incorrectly defines some functions in a way not working with integer types. Until C++20, this was worked
+// around by -fdelayed-template-parsing. Since C++20, we can use standard feature "requires" instead.
+
+// TODO: Remove the workaround once UCRT fixes these functions. Note that this doesn't seem planned as of 2025-07 per
+// https://developercommunity.visualstudio.com/t/10294165.
+
+using std::__math::__ucrt::isfinite;
+using std::__math::__ucrt::isgreater;
+using std::__math::__ucrt::isgreaterequal;
+using std::__math::__ucrt::isinf;
+using std::__math::__ucrt::isless;
+using std::__math::__ucrt::islessequal;
+using std::__math::__ucrt::islessgreater;
+using std::__math::__ucrt::isnan;
+using std::__math::__ucrt::isnormal;
+using std::__math::__ucrt::isunordered;
# endif // _LIBCPP_MSVCRT
// We have to provide double overloads for <math.h> to work on platforms that don't provide the full set of math
diff --git a/libcxx/test/libcxx/fuzzing/random.pass.cpp b/libcxx/test/libcxx/fuzzing/random.pass.cpp
index cb074bd60fdc8..79ab7ac41151c 100644
--- a/libcxx/test/libcxx/fuzzing/random.pass.cpp
+++ b/libcxx/test/libcxx/fuzzing/random.pass.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-// This test fails because Clang no longer enables -fdelayed-template-parsing
-// by default on Windows with C++20 (#69431).
-// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
-
// UNSUPPORTED: c++03, c++11
#include <cassert>
diff --git a/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp b/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp
index 1ba0063c1dada..3934e5177da14 100644
--- a/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp
+++ b/libcxx/test/std/depr/depr.c.headers/math_h.pass.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-// This test fails because Clang no longer enables -fdelayed-template-parsing
-// by default on Windows with C++20 (#69431).
-// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
-
// <math.h>
// GCC warns about signbit comparing `bool_v < 0`, which we're testing
diff --git a/libcxx/test/std/numerics/c.math/cmath.pass.cpp b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
index 48c2918802fc3..1f7c697784c10 100644
--- a/libcxx/test/std/numerics/c.math/cmath.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-// This test fails because Clang no longer enables -fdelayed-template-parsing
-// by default on Windows with C++20 (#69431).
-// XFAIL: msvc && (clang-18 || clang-19 || clang-20 || clang-21)
-
// <cmath>
#include <cmath>
|
@@ -189,6 +189,82 @@ template <class _A1, class _A2, __enable_if_t<is_arithmetic<_A1>::value && is_ar | |||
return __builtin_isunordered((type)__x, (type)__y); | |||
} | |||
|
|||
// MS UCRT incorrectly defines some functions in a way not working with integer types. Until C++20, this was worked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@StephanTLavavej Do you think there is any way we might be able to get a fix in UCRT for this?
@frederick-vs-ja Since this is a bug in UCRT which has been reported and closed as "not to be fixed", I am tempted to say that keeping these tests as XFAIL
and being non-conforming on UCRT might be acceptable. If they don't think it's worth fixing the issue, I'm not sure it's worth us doing a (pretty involved) workaround for it either.
MS UCRT seems confused on the status of LWG1327, and still provides pre-LWG1327 overload set the related math functions, which can't handle integer types as required. It is probably that UCRT won't fixed this in a near future, per
https://developercommunity.visualstudio.com/t/10294165.
Before C++20, libc++ worked around this bug by relying on
-fdelayed-template-parsing
. However, this non-conforming option is off by default since C++20. I think we should userequires
instead.Fixes #70225.