[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
Tue Jul 30 13:21:21 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/4] 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/4] 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/4] 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 b92fb0513e26ad943cbeac224c07750d332bf63d 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/4] 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), "");
More information about the libcxx-commits
mailing list