-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[libc++][math] Provide overloads for cv-unqualified floating point types for std::signbit
#106566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
libcxx/docs/Status/Cxx23Papers.csv
Outdated
@@ -39,7 +39,7 @@ | |||
"`P2401R0 <https://wg21.link/P2401R0>`__","Add a conditional ``noexcept`` specification to ``std::exchange``","2021-10 (Virtual)","|Complete|","14.0","" | |||
"","","","","","" | |||
"`P0323R12 <https://wg21.link/P0323R12>`__","``std::expected``","2022-02 (Virtual)","|Complete|","16.0","" | |||
"`P0533R9 <https://wg21.link/P0533R9>`__","``constexpr`` for ``<cmath>`` and ``<cstdlib>``","2022-02 (Virtual)","|In Progress|","","``isfinite``, ``isinf``, ``isnan`` and ``isnormal`` are implemented" | |||
"`P0533R9 <https://wg21.link/P0533R9>`__","``constexpr`` for ``<cmath>`` and ``<cstdlib>``","2022-02 (Virtual)","|In Progress|","","``isfinite``, ``isinf``, ``isnan``, ``isnormal`` and ``signbit`` are implemented" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not aware that we have GitHub issues for each paper/issue that are fully synchronized with the CSV status files and that we can encode custom notes in the CSV files via Github issues. Amazing. I will track the progress as subtask. Thanks for the friendly reminder!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robincaloudis Please let me know if you need help with creating the subtasks while you wait for commit access. You can ping me on Discord too (same account name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I will do so.
7041ead
to
8810607
Compare
By using `_LIBCPP_PREFERRED_OVERLOAD` we make sure that a given overload is a better match than an otherwise equally good function declaration. Why is there an equally good function declaration in the first place? Underlying the Windows SDK is the UCRT, the universal C runtime, which clang-cl makes use of. The UCRT should provide only C library headers, but does on top comes with overloads for all cv-unqualified floating point types (float, double, long double) for `std::signbit()` in https://github.com/microsoft/win32metadata/blob/e012b29924c53aa941fc010850b68331b0c3ea80/generation/WinSDK/RecompiledIdlHeaders/ucrt/corecrt_math.h#L309-L322. In a certain way, this can be seen as a deviation from the C standard. We need to work around it.
8810607
to
528be9e
Compare
@llvm/pr-subscribers-libcxx Author: Robin Caloudis (robincaloudis) ChangesWhyFollowing up on #105946, this patch provides the floating point overloads for What
Full diff: https://github.com/llvm/llvm-project/pull/106566.diff 2 Files Affected:
diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index 3d4f14fc9cd552..f3b1f03110ab71 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -34,8 +34,30 @@ namespace __math {
# define _LIBCPP_SIGNBIT_CONSTEXPR
#endif
-template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
-_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI bool signbit(_A1 __x) _NOEXCEPT {
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+ bool
+ signbit(float __x) _NOEXCEPT {
+ return __builtin_signbit(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+ bool
+ signbit(double __x) _NOEXCEPT {
+ return __builtin_signbit(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_SIGNBIT_CONSTEXPR _LIBCPP_HIDE_FROM_ABI
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_PREFERRED_OVERLOAD
+#endif
+ bool
+ signbit(long double __x) _NOEXCEPT {
return __builtin_signbit(__x);
}
diff --git a/libcxx/test/std/numerics/c.math/signbit.pass.cpp b/libcxx/test/std/numerics/c.math/signbit.pass.cpp
index c85033e363ce55..a8a566f7de6434 100644
--- a/libcxx/test/std/numerics/c.math/signbit.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/signbit.pass.cpp
@@ -70,9 +70,22 @@ 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::signbit` with convertible types. This checks
+ // whether overloads for all cv-unqualified floating-point types are working
+ // as expected.
+ {
+ assert(!std::signbit(ConvertibleTo<float>()));
+ assert(!std::signbit(ConvertibleTo<double>()));
+ assert(!std::signbit(ConvertibleTo<long double>()));
+ }
return 0;
}
|
15e1518
to
aef854f
Compare
aef854f
to
9c24e8d
Compare
5527761
to
d7e4b9b
Compare
Why
Following up on #105946, this patch provides the floating point overloads for
std::signbit
as defined by P0533R9.What
template<class = void>
as the universal C runtime (UCRT) needed for Clang-Cl comes with overloads for all cv-unqualified floating point types (float, double, long double) forstd::signbit()
by itself in the WinSDK. In a certain way, this can be seen as a deviation from the C standard. We need to work around it as the compilation would otherwise error out due to duplicated definitions.