[libc++] Fix acceptance of convertible-to-{float,double,long double} in std::isfinite()

[robincaloudis]

Closes #98816.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Robin Caloudis (robincaloudis)

Changes

Closes #98816.

---

Full diff: https://github.com/llvm/llvm-project/pull/98841.diff

2 Files Affected:

• (modified) libcxx/include/__math/traits.h (+14)
• (modified) libcxx/test/std/numerics/c.math/isfinite.pass.cpp (+33)

diff --git a/libcxx/include/__math/traits.h b/libcxx/include/__math/traits.h
index a448266797557..07e5a74ac2362 100644
--- a/libcxx/include/__math/traits.h
+++ b/libcxx/include/__math/traits.h
@@ -55,6 +55,20 @@ _LIBCPP_NODISCARD _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isfin
   return true;
 }
 
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isfinite(float __x) _NOEXCEPT {
+  return __builtin_isfinite(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI _LIBCPP_PREFERRED_OVERLOAD bool
+isfinite(double __x) _NOEXCEPT {
+  return __builtin_isfinite(__x);
+}
+
+_LIBCPP_NODISCARD inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isfinite(long double __x) _NOEXCEPT {
+  return __builtin_isfinite(__x);
+}
+#endif
 // isinf
 
 template <class _A1, __enable_if_t<is_arithmetic<_A1>::value && numeric_limits<_A1>::has_infinity, int> = 0>
diff --git a/libcxx/test/std/numerics/c.math/isfinite.pass.cpp b/libcxx/test/std/numerics/c.math/isfinite.pass.cpp
index 6bbc3aaac6d13..56d9550348a9b 100644
--- a/libcxx/test/std/numerics/c.math/isfinite.pass.cpp
+++ b/libcxx/test/std/numerics/c.math/isfinite.pass.cpp
@@ -62,9 +62,42 @@ struct TestInt {
   }
 };
 
+struct ConvertibleFloat {
+    int value;
+
+    ConvertibleFloat(int v) : value(v) {}
+
+    operator float() const {
+        return static_cast<float>(value);
+    }
+};
+
+struct ConvertibleDouble {
+    int value;
+
+    ConvertibleDouble(int v) : value(v) {}
+
+    operator double() const {
+        return static_cast<double>(value);
+    }
+};
+
+struct ConvertibleLongDouble {
+    int value;
+
+    ConvertibleLongDouble(int v) : value(v) {}
+
+    operator long double() const {
+        return static_cast<long double>(value);
+    }
+};
+
 int main(int, char**) {
   types::for_each(types::floating_point_types(), TestFloat());
   types::for_each(types::integral_types(), TestInt());
+  assert(std::isfinite(ConvertibleFloat(0)));
+  assert(std::isfinite(ConvertibleDouble(0)));
+  assert(std::isfinite(ConvertibleLongDouble(0)));
 
   return 0;
 }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[AngryLoki]

Fails in gcc test... Looks like std::isnan _LIBCPP_PREFERRED_OVERLOAD also never worked in gcc too and it was not covered with tests. I guess it is possible to implement it in gcc (example: https://godbolt.org/z/vcrGWzcvj). But I don't know if this level of complication is needed. libstdc++ has no such thing as "preferred overload" and just fails with call to 'isfinite' is ambiguous when there are multiple user-defined conversion functions.

[ldionne]

Thanks for the patch! Is there a reason why we must apply _LIBCPP_PREFERRED_OVERLOAD at all? This might be a naive question, but GCC implements the same thing without any trickery so I wonder why we need _LIBCPP_PREFERRED_OVERLOAD in the first place.

[robincaloudis]

Thanks for the hint! You two are completely right. _LIBCPP_PREFERRED_OVERLOAD is not needed. So far, all tests look good.

Once this is merged, I will also properly test the convertible types for std::{isnan, isinf}in #98952 as they seem to be broken due to the usage of _LIBCPP_PREFERRED_OVERLOAD.

[philnik777]

Could you also check the other traits to make sure they're correctly implemented?

[robincaloudis]

Could you also check the other traits to make sure they're correctly implemented?

Sure, I'll check them.

[ldionne]

LGTM once CI is done running. MacOS is a bit backed up right now due to an outage but it should get there eventually. Thanks!

[ldionne]

Sorry, @philnik777 and I got in a race condition. Please make sure to satisfy Nikolas' requests as well. Thanks :-)

[ldionne]

Can be merged once CI is done running.

[robincaloudis]

CI is done running. Could you merge this PR? I lack write access. Thanks!

[AngryLoki]

That was quick! Thanks everyone!