[libcxx-commits] [libcxx] 2fe59d5 - [libc++][math] Fix acceptance of convertible types in `std::isnan()` and `std::isinf()` (#98952)
via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Aug 16 08:18:53 PDT 2024
Author: Robin Caloudis
Date: 2024-08-16T11:18:49-04:00
New Revision: 2fe59d5259ec921668d87d111be55db0b047aa68
URL: https://github.com/llvm/llvm-project/commit/2fe59d5259ec921668d87d111be55db0b047aa68
DIFF: https://github.com/llvm/llvm-project/commit/2fe59d5259ec921668d87d111be55db0b047aa68.diff
LOG: [libc++][math] Fix acceptance of convertible types in `std::isnan()` and `std::isinf()` (#98952)
Following up on https://github.com/llvm/llvm-project/pull/98841.
Changes:
- Properly test convertible types for `std::isnan()` and `std::inf()`
- Tighten conditional in `cmath.pass.cpp` (Find insights on `_LIBCPP_PREFERRED_OVERLOAD` below)
- Tighten preprocessor guard in `traits.h`
Insights into why `_LIBCPP_PREFERRED_OVERLOAD` is needed:
(i) 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)` [1] and `isnan(double)` [2]
into scope. This differs from the C99 Standard as only the macros
`#define isnan(arg)` and `#define isinf(arg)` are expected.
Therefore, libc++ needs to respect the presense of the `double` overload
and cannot redefine it as it will conflict with the declaration already
in scope. For `-std=c++11` and beyond this issue is fixed, as glibc
guards both the `isinf` and `isnan` by preprocessor macros.
(ii) When libc++ is layered on top of Bionic's libc, `math.h` exposes a
function prototype for `isinf(double)` with return type `int`. This
function prototype in Bionic's libc is not guarded by any preprocessor
macros [3].
`_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 [4] 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.
[1]: https://github.com/bminor/glibc/blob/fe9408087583fd7a6f61bb0dbcf2fd4e83186afa/math/bits/mathcalls.h#L185-L194
[2]: https://github.com/bminor/glibc/blob/fe9408087583fd7a6f61bb0dbcf2fd4e83186afa/math/bits/mathcalls.h#L222-L231
[3]: https://cs.android.com/android/platform/superproject/+/master:bionic/libc/include/math.h;l=322-323;drc=master?hl=fr-BE%22https:%2F%2Fsupport.google.com%2Fmerchants%2Fanswer%2F188494%5C%22%22https:%2F%2Fsupport.google.com%2Fmerchants%2Fanswer%2F188494%5C%22
[4]: https://github.com/llvm/llvm-project/commit/5fd17ab1b093f6b59aabb27f6c2c2278e65c2707
Added:
Modified:
libcxx/include/__math/traits.h
libcxx/test/std/numerics/c.math/cmath.pass.cpp
libcxx/test/std/numerics/c.math/isinf.pass.cpp
libcxx/test/std/numerics/c.math/isnan.pass.cpp
Removed:
################################################################################
diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 27ec52ecef022e..35c283cc9e21ce 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -79,20 +79,22 @@ _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf
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
+#ifdef _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 +108,22 @@ _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
+#ifdef _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
diff --git a/libcxx/test/std/numerics/c.math/cmath.pass.cpp b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
index 6028aa5a211034..34c30fb998f474 100644
--- a/libcxx/test/std/numerics/c.math/cmath.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/cmath.pass.cpp
@@ -708,15 +708,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__) && TEST_STD_VER == 03 && defined(TEST_COMPILER_CLANG)
// 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), "");
@@ -794,15 +795,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__) && TEST_STD_VER == 03 && defined(TEST_COMPILER_CLANG)
+ // 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), "");
diff --git a/libcxx/test/std/numerics/c.math/isinf.pass.cpp b/libcxx/test/std/numerics/c.math/isinf.pass.cpp
index e935b53187fe6c..e5169e8056c2ea 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;
}
diff --git a/libcxx/test/std/numerics/c.math/isnan.pass.cpp b/libcxx/test/std/numerics/c.math/isnan.pass.cpp
index fffb1246458629..e4ccab1243e567 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;
}
More information about the libcxx-commits
mailing list