[libcxx-commits] [libcxx] [libc++][math] Fix acceptance of convertible types in `std::isnan()` and `std::isinf()` (PR #98952)

Robin Caloudis via libcxx-commits libcxx-commits at lists.llvm.org
Mon Aug 5 12:14:59 PDT 2024


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

>From a9e74327c43a2fa4207ccc61644b201c8d4c8b65 Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Tue, 30 Jul 2024 22:15:28 +0200
Subject: [PATCH 1/5] Test convertibles in std::isinf()

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

diff --git a/libcxx/test/std/numerics/c.math/isinf.pass.cpp b/libcxx/test/std/numerics/c.math/isinf.pass.cpp
index e935b53187fe6..e5169e8056c2e 100644
--- a/libcxx/test/std/numerics/c.math/isinf.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/isinf.pass.cpp
@@ -62,9 +62,21 @@ 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::isinf` with convertible types
+  {
+    assert(!std::isinf(ConvertibleTo<float>()));
+    assert(!std::isinf(ConvertibleTo<double>()));
+    assert(!std::isinf(ConvertibleTo<long double>()));
+  }
+
   return 0;
 }

>From 4d4dadfd106b305278d819ea4fce4e2b76c58cef Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Tue, 30 Jul 2024 22:15:54 +0200
Subject: [PATCH 2/5] Test convertibles in std::isnan()

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

diff --git a/libcxx/test/std/numerics/c.math/isnan.pass.cpp b/libcxx/test/std/numerics/c.math/isnan.pass.cpp
index fffb124645862..e4ccab1243e56 100644
--- a/libcxx/test/std/numerics/c.math/isnan.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/isnan.pass.cpp
@@ -62,9 +62,21 @@ 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::isnan` with convertible types
+  {
+    assert(!std::isnan(ConvertibleTo<float>()));
+    assert(!std::isnan(ConvertibleTo<double>()));
+    assert(!std::isnan(ConvertibleTo<long double>()));
+  }
+
   return 0;
 }

>From c9f6d8265a0b24457dbd6dac8bb3a0dc624d1589 Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Tue, 30 Jul 2024 22:17:58 +0200
Subject: [PATCH 3/5] Use _LIBCPP_PREFERRED_OVERLOAD only for glibc + cxx03 and
 bionic

---
 libcxx/include/__math/traits.h | 58 ++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 10 deletions(-)

diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 27ec52ecef022..612e207b3d47f 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -69,30 +69,49 @@ _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI boo
 
 // isinf
 
-template <class _A1, __enable_if_t<is_arithmetic<_A1>::value && numeric_limits<_A1>::has_infinity, int> = 0>
+template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
 _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(_A1 __x) _NOEXCEPT {
   return __builtin_isinf((typename __promote<_A1>::type)__x);
 }
 
-template <class _A1, __enable_if_t<is_arithmetic<_A1>::value && !numeric_limits<_A1>::has_infinity, int> = 0>
+template <class _A1, __enable_if_t<is_integral<_A1>::value, int> = 0>
 _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(_A1) _NOEXCEPT {
   return false;
 }
 
-#ifdef _LIBCPP_PREFERRED_OVERLOAD
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(float __x) _NOEXCEPT {
   return __builtin_isinf(__x);
 }
 
-_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
-isinf(double __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
+// When libc++ is layered on top of glibc on Linux, glibc's `math.h` is included. When compiling with
+// `-std=c++03`, this header brings the function declaration of `isinf(double)` with return type of
+// `int` into scope. This differs from the C99 standard as only a macro of the form `#define isinf(arg)`
+// is expected. Therefore, libc++ needs to respect the presense of this `double` overload with return type
+// `int` and cannot redefine it with return type `bool` like it is supposed to be, as it will conflict with
+// the declaration already in scope. For `-std=c++11` and beyond this issue is fixed, as glibc guards the
+// `double` overload of `isinf` by preprocessor macros.
+//
+// When libc++ is layered on top of Bionic's libc, `math.h` exposes a function prototype for `isnan(double)`
+// with return type `int`. This function prototype in Bionic's libc is not guarded by any preprocessor macros
+// and will therefore conflict.
+//
+// `_LIBCPP_PREFERRED_OVERLOAD` specifies that a given overload is a better match than an otherwise equally good
+// function declaration. This is implemented in modern versions of Clang via `__attribute__((__enable_if__))`, and
+// not elsewhere. See https://github.com/llvm/llvm-project/commit/5fd17ab1b093f6b59aabb27f6c2c2278e65c2707 for
+// details. We use `_LIBCPP_PREFERRED_OVERLOAD` to define overloads in the global namespace that displace the
+// overloads provided by the C libraries mentioned above.
+#if ((defined(_LIBCPP_CXX03_LANG) && defined(__GLIBC__)) || defined(__BIONIC__)) && defined(_LIBCPP_PREFERRED_OVERLOAD)
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+    bool
+    isinf(double __x) _NOEXCEPT {
   return __builtin_isinf(__x);
 }
 
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(long double __x) _NOEXCEPT {
   return __builtin_isinf(__x);
 }
-#endif
 
 // isnan
 
@@ -106,20 +125,39 @@ _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan
   return false;
 }
 
-#ifdef _LIBCPP_PREFERRED_OVERLOAD
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(float __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
 
-_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
-isnan(double __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
+// When libc++ is layered on top of glibc on Linux, glibc's `math.h` is included. When compiling with
+// `-std=c++03`, this header brings the function declaration of `isnan(double)` with return type of
+// `int` into scope. This differs from the C99 standard as only a macro of the form `#define isnan(arg)`
+// is expected. Therefore, libc++ needs to respect the presense of this `double` overload with return type
+// `int` and cannot redefine it with return type `bool` like it is supposed to be, as it will conflict with
+// the declaration already in scope. For `-std=c++11` and beyond this issue is fixed, as glibc guards the
+// `double` overload of `isnan` by preprocessor macros.
+//
+// When libc++ is layered on top of Bionic's libc, `math.h` exposes a function prototype for `isnan(double)`
+// with return type `int`. This function prototype in Bionic's libc is not guarded by any preprocessor macros
+// and will therefore conflict.
+//
+// `_LIBCPP_PREFERRED_OVERLOAD` specifies that a given overload is a better match than an otherwise equally good
+// function declaration. This is implemented in modern versions of Clang via `__attribute__((__enable_if__))`, and
+// not elsewhere. See https://github.com/llvm/llvm-project/commit/5fd17ab1b093f6b59aabb27f6c2c2278e65c2707 for
+// details. We use `_LIBCPP_PREFERRED_OVERLOAD` to define overloads in the global namespace that displace the
+// overloads provided by the C libraries mentioned above.
+#if ((defined(_LIBCPP_CXX03_LANG) && defined(__GLIBC__)) || defined(__BIONIC__)) && defined(_LIBCPP_PREFERRED_OVERLOAD)
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+    bool
+    isnan(double __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
 
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(long double __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
-#endif
 
 // isnormal
 

>From 74ab539ddaa2fe8a26a20dfc20bb920b9c039ee8 Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Tue, 30 Jul 2024 22:19:02 +0200
Subject: [PATCH 4/5] Test return type 'int' only for glibc + cxx03

---
 .../test/std/numerics/c.math/cmath.pass.cpp   | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libcxx/test/std/numerics/c.math/cmath.pass.cpp b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
index 9379084499792..8e1bb42b3ddee 100644
--- a/libcxx/test/std/numerics/c.math/cmath.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
@@ -705,15 +705,16 @@ void test_isinf()
     static_assert((std::is_same<decltype(std::isinf((float)0)), bool>::value), "");
 
     typedef decltype(std::isinf((double)0)) DoubleRetType;
-#if !defined(__linux__) || defined(__clang__)
-    static_assert((std::is_same<DoubleRetType, bool>::value), "");
-#else
+#if defined(__GLIBC__) && defined(_LIBCPP_CXX03_LANG) && !defined(_LIBCPP_PREFERRED_OVERLOAD)
     // GLIBC < 2.23 defines 'isinf(double)' with a return type of 'int' in
     // all C++ dialects. The test should tolerate this when libc++ can't work
-    // around it.
+    // around it via `_LIBCPP_PREFERRED_OVERLOAD`, which is only available
+    // in modern versions of Clang, and not elsewhere.
     // See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
     static_assert((std::is_same<DoubleRetType, bool>::value
                 || std::is_same<DoubleRetType, int>::value), "");
+#else
+    static_assert((std::is_same<DoubleRetType, bool>::value), "");
 #endif
 
     static_assert((std::is_same<decltype(std::isinf(0)), bool>::value), "");
@@ -791,15 +792,16 @@ void test_isnan()
     static_assert((std::is_same<decltype(std::isnan((float)0)), bool>::value), "");
 
     typedef decltype(std::isnan((double)0)) DoubleRetType;
-#if !defined(__linux__) || defined(__clang__)
-    static_assert((std::is_same<DoubleRetType, bool>::value), "");
-#else
-    // GLIBC < 2.23 defines 'isinf(double)' with a return type of 'int' in
+#if defined(__GLIBC__) && defined(_LIBCPP_CXX03_LANG) && !defined(_LIBCPP_PREFERRED_OVERLOAD)
+    // GLIBC < 2.23 defines 'isnan(double)' with a return type of 'int' in
     // all C++ dialects. The test should tolerate this when libc++ can't work
-    // around it.
+    // around it via `_LIBCPP_PREFERRED_OVERLOAD`, which is only available
+    // in modern versions of Clang, and not elsewhere.
     // See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
     static_assert((std::is_same<DoubleRetType, bool>::value
                 || std::is_same<DoubleRetType, int>::value), "");
+#else
+    static_assert((std::is_same<DoubleRetType, bool>::value), "");
 #endif
 
     static_assert((std::is_same<decltype(std::isnan(0)), bool>::value), "");

>From 66c8861d231d71a4eca154d01042a46c1326b7ed Mon Sep 17 00:00:00 2001
From: Robin Caloudis <robin.caloudis at gmx.de>
Date: Mon, 5 Aug 2024 21:14:35 +0200
Subject: [PATCH 5/5] Revert "Use _LIBCPP_PREFERRED_OVERLOAD only for glibc +
 cxx03 and bionic"

This reverts commit c9f6d8265a0b24457dbd6dac8bb3a0dc624d1589.
---
 libcxx/include/__math/traits.h | 58 ++++++----------------------------
 1 file changed, 10 insertions(+), 48 deletions(-)

diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 612e207b3d47f..27ec52ecef022 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -69,49 +69,30 @@ _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI boo
 
 // isinf
 
-template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
+template <class _A1, __enable_if_t<is_arithmetic<_A1>::value && numeric_limits<_A1>::has_infinity, int> = 0>
 _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(_A1 __x) _NOEXCEPT {
   return __builtin_isinf((typename __promote<_A1>::type)__x);
 }
 
-template <class _A1, __enable_if_t<is_integral<_A1>::value, int> = 0>
+template <class _A1, __enable_if_t<is_arithmetic<_A1>::value && !numeric_limits<_A1>::has_infinity, int> = 0>
 _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(_A1) _NOEXCEPT {
   return false;
 }
 
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(float __x) _NOEXCEPT {
   return __builtin_isinf(__x);
 }
 
-_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
-// When libc++ is layered on top of glibc on Linux, glibc's `math.h` is included. When compiling with
-// `-std=c++03`, this header brings the function declaration of `isinf(double)` with return type of
-// `int` into scope. This differs from the C99 standard as only a macro of the form `#define isinf(arg)`
-// is expected. Therefore, libc++ needs to respect the presense of this `double` overload with return type
-// `int` and cannot redefine it with return type `bool` like it is supposed to be, as it will conflict with
-// the declaration already in scope. For `-std=c++11` and beyond this issue is fixed, as glibc guards the
-// `double` overload of `isinf` by preprocessor macros.
-//
-// When libc++ is layered on top of Bionic's libc, `math.h` exposes a function prototype for `isnan(double)`
-// with return type `int`. This function prototype in Bionic's libc is not guarded by any preprocessor macros
-// and will therefore conflict.
-//
-// `_LIBCPP_PREFERRED_OVERLOAD` specifies that a given overload is a better match than an otherwise equally good
-// function declaration. This is implemented in modern versions of Clang via `__attribute__((__enable_if__))`, and
-// not elsewhere. See https://github.com/llvm/llvm-project/commit/5fd17ab1b093f6b59aabb27f6c2c2278e65c2707 for
-// details. We use `_LIBCPP_PREFERRED_OVERLOAD` to define overloads in the global namespace that displace the
-// overloads provided by the C libraries mentioned above.
-#if ((defined(_LIBCPP_CXX03_LANG) && defined(__GLIBC__)) || defined(__BIONIC__)) && defined(_LIBCPP_PREFERRED_OVERLOAD)
-_LIBCPP_PREFERRED_OVERLOAD
-#endif
-    bool
-    isinf(double __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
+isinf(double __x) _NOEXCEPT {
   return __builtin_isinf(__x);
 }
 
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(long double __x) _NOEXCEPT {
   return __builtin_isinf(__x);
 }
+#endif
 
 // isnan
 
@@ -125,39 +106,20 @@ _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan
   return false;
 }
 
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(float __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
 
-_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
-// When libc++ is layered on top of glibc on Linux, glibc's `math.h` is included. When compiling with
-// `-std=c++03`, this header brings the function declaration of `isnan(double)` with return type of
-// `int` into scope. This differs from the C99 standard as only a macro of the form `#define isnan(arg)`
-// is expected. Therefore, libc++ needs to respect the presense of this `double` overload with return type
-// `int` and cannot redefine it with return type `bool` like it is supposed to be, as it will conflict with
-// the declaration already in scope. For `-std=c++11` and beyond this issue is fixed, as glibc guards the
-// `double` overload of `isnan` by preprocessor macros.
-//
-// When libc++ is layered on top of Bionic's libc, `math.h` exposes a function prototype for `isnan(double)`
-// with return type `int`. This function prototype in Bionic's libc is not guarded by any preprocessor macros
-// and will therefore conflict.
-//
-// `_LIBCPP_PREFERRED_OVERLOAD` specifies that a given overload is a better match than an otherwise equally good
-// function declaration. This is implemented in modern versions of Clang via `__attribute__((__enable_if__))`, and
-// not elsewhere. See https://github.com/llvm/llvm-project/commit/5fd17ab1b093f6b59aabb27f6c2c2278e65c2707 for
-// details. We use `_LIBCPP_PREFERRED_OVERLOAD` to define overloads in the global namespace that displace the
-// overloads provided by the C libraries mentioned above.
-#if ((defined(_LIBCPP_CXX03_LANG) && defined(__GLIBC__)) || defined(__BIONIC__)) && defined(_LIBCPP_PREFERRED_OVERLOAD)
-_LIBCPP_PREFERRED_OVERLOAD
-#endif
-    bool
-    isnan(double __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
+isnan(double __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
 
 _LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(long double __x) _NOEXCEPT {
   return __builtin_isnan(__x);
 }
+#endif
 
 // isnormal
 



More information about the libcxx-commits mailing list