[libcxx-commits] [libcxx] [libc++][math] Provide overloads for cv-unqualified floating point types for `std::signbit` (PR #106566)

[Robin Caloudis via libcxx-commits]

https://github.com/robincaloudis updated https://github.com/llvm/llvm-project/pull/106566

>From 78048246033dea428e77fa08fdf21a529835eca5 Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Thu, 5 Sep 2024 19:21:04 +0200
Subject: [PATCH 1/3] Test floating point overloads

---
 libcxx/test/std/numerics/c.math/signbit.pass.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/libcxx/test/std/numerics/c.math/signbit.pass.cpp b/libcxx/test/std/numerics/c.math/signbit.pass.cpp
index c85033e363ce55..a8a566f7de6434 100644
--- a/libcxx/test/std/numerics/c.math/signbit.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/signbit.pass.cpp
@@ -70,9 +70,22 @@ struct TestInt {
   }
 };
 
+template <typename T>
+struct ConvertibleTo {
+  operator T() const { return T(); }
+};
+
 int main(int, char**) {
   types::for_each(types::floating_point_types(), TestFloat());
   types::for_each(types::integral_types(), TestInt());
 
+  // Make sure we can call `std::signbit` with convertible types. This checks
+  // whether overloads for all cv-unqualified floating-point types are working
+  // as expected.
+  {
+    assert(!std::signbit(ConvertibleTo<float>()));
+    assert(!std::signbit(ConvertibleTo<double>()));
+    assert(!std::signbit(ConvertibleTo<long double>()));
+  }
   return 0;
 }

>From 528be9e9b5026b5adf610e9af5b1902b84433c60 Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Thu, 5 Sep 2024 19:26:24 +0200
Subject: [PATCH 2/3] Add floating point overloads for std::signbit

By using `_LIBCPP_PREFERRED_OVERLOAD` we make
sure that a given overload is a better match
than an otherwise equally good function
declaration.

Why is there an equally good function
declaration in the first place? Underlying the
Windows SDK is the UCRT, the universal C runtime,
which clang-cl makes use of. The UCRT should
provide only C library headers, but does on top
comes with overloads for all cv-unqualified floating
point types (float, double, long double) for
`std::signbit()` in https://github.com/microsoft/win32metadata/blob/e012b29924c53aa941fc010850b68331b0c3ea80/generation/WinSDK/RecompiledIdlHeaders/ucrt/corecrt_math.h#L309-L322.
In a certain way, this can be seen as a deviation
from the C standard. We need to work around it.
---
 libcxx/include/__math/traits.h | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 3d4f14fc9cd552..f3b1f03110ab71 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -34,8 +34,30 @@ namespace __math {
 #  define _LIBCPP_SIGNBIT_CONSTEXPR
 #endif
 
-template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
-_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI bool signbit(_A1 __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+    bool
+    signbit(float __x) _NOEXCEPT {
+  return __builtin_signbit(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+    bool
+    signbit(double __x) _NOEXCEPT {
+  return __builtin_signbit(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+    bool
+    signbit(long double __x) _NOEXCEPT {
   return __builtin_signbit(__x);
 }
 

>From 15e151867d44d42e43332f0f2408781c465b8cf6 Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Sat, 7 Sep 2024 13:51:44 +0200
Subject: [PATCH 3/3] Encapsulate the use of macro and leave comment

---
 libcxx/include/__math/traits.h | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index f3b1f03110ab71..a957df1785ac2d 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -34,30 +34,27 @@ namespace __math {
 #  define _LIBCPP_SIGNBIT_CONSTEXPR
 #endif
 
-_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+// The universal C runtime (UCRT) in the WinSDK provides overloads for all floating point types
+// for std::signbit(). We need to work around it as the compilation would otherwise error out
+// due to duplicated definitions for clang-cl builds.
 #ifdef _LIBCPP_PREFERRED_OVERLOAD
-_LIBCPP_PREFERRED_OVERLOAD
+#  define LIBCPP_SIGNBIT_OVERLOAD _LIBCPP_PREFERRED_OVERLOAD
+#else
+#  define LIBCPP_SIGNBIT_OVERLOAD
 #endif
-    bool
-    signbit(float __x) _NOEXCEPT {
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI LIBCPP_SIGNBIT_OVERLOAD bool
+signbit(float __x) _NOEXCEPT {
   return __builtin_signbit(__x);
 }
 
-_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
-#ifdef _LIBCPP_PREFERRED_OVERLOAD
-_LIBCPP_PREFERRED_OVERLOAD
-#endif
-    bool
-    signbit(double __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI LIBCPP_SIGNBIT_OVERLOAD bool
+signbit(double __x) _NOEXCEPT {
   return __builtin_signbit(__x);
 }
 
-_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
-#ifdef _LIBCPP_PREFERRED_OVERLOAD
-_LIBCPP_PREFERRED_OVERLOAD
-#endif
-    bool
-    signbit(long double __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI LIBCPP_SIGNBIT_OVERLOAD bool
+signbit(long double __x) _NOEXCEPT {
   return __builtin_signbit(__x);
 }