[libcxx-commits] [libcxx] [libc++][math] Fix acceptance of convertible types in `std::isnan()` and `std::isinf()` (PR #98952)
via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jul 30 13:21:57 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Robin Caloudis (robincaloudis)
<details>
<summary>Changes</summary>
Following up on https://github.com/llvm/llvm-project/pull/98841.
## Why
(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)`](https://github.com/bminor/glibc/blob/fe9408087583fd7a6f61bb0dbcf2fd4e83186afa/math/bits/mathcalls.h#L185-L194) and [`isnan(double)`](https://github.com/bminor/glibc/blob/fe9408087583fd7a6f61bb0dbcf2fd4e83186afa/math/bits/mathcalls.h#L222-L231) into scope. This differs from the [C99](https://en.cppreference.com/w/c/numeric/math/isnan) standard as only the macros `#define isnan(arg)`/`#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 (See links above).
(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](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).
## What
`_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.
* Properly test convertible types for `std::isnan()` and `std::inf()`
* Sneaked in a little code style improvement: `__enable_if_t<is_arithmetic<_A1>::value && !numeric_limits<_A1>::has_infinity` == `__enable_if_t<is_integral<_A1>::value`
* Use `_LIBCPP_PREFERRED_OVERLOAD` only for relevant situations such that it does not shadow other things
* Tighten conditional in `cmath.pass.cpp`
---
Full diff: https://github.com/llvm/llvm-project/pull/98952.diff
4 Files Affected:
- (modified) libcxx/include/__math/traits.h (+48-10)
- (modified) libcxx/test/std/numerics/c.math/cmath.pass.cpp (+11-9)
- (modified) libcxx/test/std/numerics/c.math/isinf.pass.cpp (+12)
- (modified) libcxx/test/std/numerics/c.math/isnan.pass.cpp (+12)
``````````diff
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
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), "");
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;
}
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;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/98952
More information about the libcxx-commits
mailing list