[libc-commits] [libc] [llvm] [libc] Remove UB specializations of type traits for `BigInt` (PR #84035)

[Guillaume Chatelet via libc-commits]

================
@@ -912,66 +933,169 @@ template <> class numeric_limits<Int<128>> {
   LIBC_INLINE_VAR static constexpr int digits = 128;
 };
 
-// Provides is_integral of U/Int<128>, U/Int<192>, U/Int<256>.
-template <size_t Bits, bool Signed, typename T>
-struct is_integral<BigInt<Bits, Signed, T>> : cpp::true_type {};
-
-// Provides is_unsigned of UInt<128>, UInt<192>, UInt<256>.
-template <size_t Bits, bool Signed, typename T>
-struct is_unsigned<BigInt<Bits, Signed, T>> : cpp::bool_constant<!Signed> {};
-
-template <size_t Bits, bool Signed, typename T>
-struct make_unsigned<BigInt<Bits, Signed, T>>
-    : type_identity<BigInt<Bits, false, T>> {};
-
-template <size_t Bits, bool Signed, typename T>
-struct make_signed<BigInt<Bits, Signed, T>>
-    : type_identity<BigInt<Bits, true, T>> {};
-
-namespace internal {
-template <typename T> struct is_custom_uint : cpp::false_type {};
+// type traits to determine whether a T is a cpp::BigInt.
+template <typename T> struct is_big_int : cpp::false_type {};
 
 template <size_t Bits, bool Signed, typename T>
-struct is_custom_uint<BigInt<Bits, Signed, T>> : cpp::true_type {};
-} // namespace internal
-
-// bit_cast to UInt
-// Note: The standard scheme for SFINAE selection is to have exactly one
-// function instanciation valid at a time. This is usually done by having a
-// predicate in one function and the negated predicate in the other one.
-// e.g.
-// template<typename = cpp::enable_if_t< is_custom_uint<To>::value == true> ...
-// template<typename = cpp::enable_if_t< is_custom_uint<To>::value == false> ...
-//
-// Unfortunately this would make the default 'cpp::bit_cast' aware of
-// 'is_custom_uint' (or any other customization). To prevent exposing all
-// customizations in the original function, we create a different function with
-// four 'typename's instead of three - otherwise it would be considered as a
-// redeclaration of the same function leading to "error: template parameter
-// redefines default argument".
-template <typename To, typename From,
-          typename = cpp::enable_if_t<sizeof(To) == sizeof(From) &&
-                                      cpp::is_trivially_copyable<To>::value &&
-                                      cpp::is_trivially_copyable<From>::value>,
-          typename = cpp::enable_if_t<internal::is_custom_uint<To>::value>>
-LIBC_INLINE constexpr To bit_cast(const From &from) {
+struct is_big_int<BigInt<Bits, Signed, T>> : cpp::true_type {};
+
+template <class T>
+LIBC_INLINE_VAR constexpr bool is_big_int_v = is_big_int<T>::value;
+
+// Specialization of cpp::bit_cast ('bit.h') from T to BigInt.
+template <typename To, typename From>
+LIBC_INLINE constexpr cpp::enable_if_t<
+    (sizeof(To) == sizeof(From)) && cpp::is_trivially_copyable<To>::value &&
+        cpp::is_trivially_copyable<From>::value && is_big_int<To>::value,
+    To>
+bit_cast(const From &from) {
   To out;
   using Storage = decltype(out.val);
   out.val = cpp::bit_cast<Storage>(from);
   return out;
 }
 
-// bit_cast from UInt
-template <
-    typename To, size_t Bits,
-    typename = cpp::enable_if_t<sizeof(To) == sizeof(UInt<Bits>) &&
-                                cpp::is_trivially_constructible<To>::value &&
-                                cpp::is_trivially_copyable<To>::value &&
-                                cpp::is_trivially_copyable<UInt<Bits>>::value>>
-LIBC_INLINE constexpr To bit_cast(const UInt<Bits> &from) {
+// Specialization of cpp::bit_cast ('bit.h') from BigInt to T.
+template <typename To, size_t Bits>
+LIBC_INLINE constexpr cpp::enable_if_t<
+    sizeof(To) == sizeof(UInt<Bits>) &&
+        cpp::is_trivially_constructible<To>::value &&
+        cpp::is_trivially_copyable<To>::value &&
+        cpp::is_trivially_copyable<UInt<Bits>>::value,
+    To>
+bit_cast(const UInt<Bits> &from) {
   return cpp::bit_cast<To>(from.val);
 }
 
+// Specialization of cpp::has_single_bit ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, bool>
+has_single_bit(T value) {
+  for (auto word : value.val)
+    if (cpp::has_single_bit(word))
+      return true;
----------------
gchatelet wrote:

This is obviously false, thx a lot for catching it!
We [do have a test covering this method](https://github.com/llvm/llvm-project/blob/7a0acccd81df268dc7ad4c0358c42552789f19b4/libc/test/src/__support/CPP/bit_test.cpp#L25-L30) but it doesn't catch this bug in particular.

I'll update the test and fix the bug.

https://github.com/llvm/llvm-project/pull/84035