From 2fe59d5259ec921668d87d111be55db0b047aa68 Mon Sep 17 00:00:00 2001 From: Robin Caloudis Date: Fri, 16 Aug 2024 17:18:49 +0200 Subject: [PATCH] [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 --- libcxx/include/__math/traits.h | 20 +++++++++++-------- .../test/std/numerics/c.math/cmath.pass.cpp | 20 ++++++++++--------- .../test/std/numerics/c.math/isinf.pass.cpp | 12 +++++++++++ .../test/std/numerics/c.math/isnan.pass.cpp | 12 +++++++++++ 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h index 27ec52ecef02..35c283cc9e21 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 6028aa5a2110..34c30fb998f4 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::value), ""); typedef decltype(std::isinf((double)0)) DoubleRetType; -#if !defined(__linux__) || defined(__clang__) - static_assert((std::is_same::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::value || std::is_same::value), ""); +#else + static_assert((std::is_same::value), ""); #endif static_assert((std::is_same::value), ""); @@ -794,15 +795,16 @@ void test_isnan() static_assert((std::is_same::value), ""); typedef decltype(std::isnan((double)0)) DoubleRetType; -#if !defined(__linux__) || defined(__clang__) - static_assert((std::is_same::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::value || std::is_same::value), ""); +#else + static_assert((std::is_same::value), ""); #endif static_assert((std::is_same::value), ""); diff --git a/libcxx/test/std/numerics/c.math/isinf.pass.cpp b/libcxx/test/std/numerics/c.math/isinf.pass.cpp index e935b53187fe..e5169e8056c2 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 +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())); + assert(!std::isinf(ConvertibleTo())); + assert(!std::isinf(ConvertibleTo())); + } + 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 fffb12464586..e4ccab1243e5 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 +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())); + assert(!std::isnan(ConvertibleTo())); + assert(!std::isnan(ConvertibleTo())); + } + return 0; }