Skip to content

Commit 2fe59d5

Browse files
[libc++][math] Fix acceptance of convertible types in std::isnan() and std::isinf() (#98952)
Following up on #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]: 5fd17ab
1 parent a07aba5 commit 2fe59d5

File tree

4 files changed

+47
-17
lines changed

4 files changed

+47
-17
lines changed

libcxx/include/__math/traits.h

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,22 @@ _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf
7979
return false;
8080
}
8181

82-
#ifdef _LIBCPP_PREFERRED_OVERLOAD
8382
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(float __x) _NOEXCEPT {
8483
return __builtin_isinf(__x);
8584
}
8685

87-
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
88-
isinf(double __x) _NOEXCEPT {
86+
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
87+
#ifdef _LIBCPP_PREFERRED_OVERLOAD
88+
_LIBCPP_PREFERRED_OVERLOAD
89+
#endif
90+
bool
91+
isinf(double __x) _NOEXCEPT {
8992
return __builtin_isinf(__x);
9093
}
9194

9295
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isinf(long double __x) _NOEXCEPT {
9396
return __builtin_isinf(__x);
9497
}
95-
#endif
9698

9799
// isnan
98100

@@ -106,20 +108,22 @@ _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan
106108
return false;
107109
}
108110

109-
#ifdef _LIBCPP_PREFERRED_OVERLOAD
110111
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(float __x) _NOEXCEPT {
111112
return __builtin_isnan(__x);
112113
}
113114

114-
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
115-
isnan(double __x) _NOEXCEPT {
115+
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI
116+
#ifdef _LIBCPP_PREFERRED_OVERLOAD
117+
_LIBCPP_PREFERRED_OVERLOAD
118+
#endif
119+
bool
120+
isnan(double __x) _NOEXCEPT {
116121
return __builtin_isnan(__x);
117122
}
118123

119124
_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isnan(long double __x) _NOEXCEPT {
120125
return __builtin_isnan(__x);
121126
}
122-
#endif
123127

124128
// isnormal
125129

libcxx/test/std/numerics/c.math/cmath.pass.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -708,15 +708,16 @@ void test_isinf()
708708
static_assert((std::is_same<decltype(std::isinf((float)0)), bool>::value), "");
709709

710710
typedef decltype(std::isinf((double)0)) DoubleRetType;
711-
#if !defined(__linux__) || defined(__clang__)
712-
static_assert((std::is_same<DoubleRetType, bool>::value), "");
713-
#else
711+
#if defined(__GLIBC__) && TEST_STD_VER == 03 && defined(TEST_COMPILER_CLANG)
714712
// GLIBC < 2.23 defines 'isinf(double)' with a return type of 'int' in
715713
// all C++ dialects. The test should tolerate this when libc++ can't work
716-
// around it.
714+
// around it via `_LIBCPP_PREFERRED_OVERLOAD`, which is only available
715+
// in modern versions of Clang, and not elsewhere.
717716
// See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
718717
static_assert((std::is_same<DoubleRetType, bool>::value
719718
|| std::is_same<DoubleRetType, int>::value), "");
719+
#else
720+
static_assert((std::is_same<DoubleRetType, bool>::value), "");
720721
#endif
721722

722723
static_assert((std::is_same<decltype(std::isinf(0)), bool>::value), "");
@@ -794,15 +795,16 @@ void test_isnan()
794795
static_assert((std::is_same<decltype(std::isnan((float)0)), bool>::value), "");
795796

796797
typedef decltype(std::isnan((double)0)) DoubleRetType;
797-
#if !defined(__linux__) || defined(__clang__)
798-
static_assert((std::is_same<DoubleRetType, bool>::value), "");
799-
#else
800-
// GLIBC < 2.23 defines 'isinf(double)' with a return type of 'int' in
798+
#if defined(__GLIBC__) && TEST_STD_VER == 03 && defined(TEST_COMPILER_CLANG)
799+
// GLIBC < 2.23 defines 'isnan(double)' with a return type of 'int' in
801800
// all C++ dialects. The test should tolerate this when libc++ can't work
802-
// around it.
801+
// around it via `_LIBCPP_PREFERRED_OVERLOAD`, which is only available
802+
// in modern versions of Clang, and not elsewhere.
803803
// See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
804804
static_assert((std::is_same<DoubleRetType, bool>::value
805805
|| std::is_same<DoubleRetType, int>::value), "");
806+
#else
807+
static_assert((std::is_same<DoubleRetType, bool>::value), "");
806808
#endif
807809

808810
static_assert((std::is_same<decltype(std::isnan(0)), bool>::value), "");

libcxx/test/std/numerics/c.math/isinf.pass.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,21 @@ struct TestInt {
6262
}
6363
};
6464

65+
template <typename T>
66+
struct ConvertibleTo {
67+
operator T() const { return T(); }
68+
};
69+
6570
int main(int, char**) {
6671
types::for_each(types::floating_point_types(), TestFloat());
6772
types::for_each(types::integral_types(), TestInt());
6873

74+
// Make sure we can call `std::isinf` with convertible types
75+
{
76+
assert(!std::isinf(ConvertibleTo<float>()));
77+
assert(!std::isinf(ConvertibleTo<double>()));
78+
assert(!std::isinf(ConvertibleTo<long double>()));
79+
}
80+
6981
return 0;
7082
}

libcxx/test/std/numerics/c.math/isnan.pass.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,21 @@ struct TestInt {
6262
}
6363
};
6464

65+
template <typename T>
66+
struct ConvertibleTo {
67+
operator T() const { return T(); }
68+
};
69+
6570
int main(int, char**) {
6671
types::for_each(types::floating_point_types(), TestFloat());
6772
types::for_each(types::integral_types(), TestInt());
6873

74+
// Make sure we can call `std::isnan` with convertible types
75+
{
76+
assert(!std::isnan(ConvertibleTo<float>()));
77+
assert(!std::isnan(ConvertibleTo<double>()));
78+
assert(!std::isnan(ConvertibleTo<long double>()));
79+
}
80+
6981
return 0;
7082
}

0 commit comments

Comments
 (0)