From 620b73b3d5bb6cf9d78d8b5ce456a9d65f2159cc Mon Sep 17 00:00:00 2001
From: asynts <asynts@gmail.com>
Date: Sun, 20 Dec 2020 17:00:50 +0100
Subject: [PATCH] AK+Format: Accept unsigned long in replacement fields.

I ran into this exact but at least twenty times in Serenity alone. The
C++ Standard dictates that 'unsigned long' and 'unsigned long long' are
distinct types even though on most platforms they are usually both 64
bit integers.

Also it wasn't possible to evaluate IsIntegral<T> for types that were
not integers since it used MakeUnsigned<T> internally.
---
 AK/Format.h             | 43 ++++++++++++++++++++++++++---------------
 AK/StdLibExtras.h       |  4 ++++
 AK/Tests/TestFormat.cpp | 11 +++++++++++
 3 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/AK/Format.h b/AK/Format.h
index 9ff0fa7077..b269d70ac6 100644
--- a/AK/Format.h
+++ b/AK/Format.h
@@ -60,25 +60,36 @@ struct TypeErasedParameter {
         Custom
     };
 
+    static Type get_type_from_size(size_t size, bool is_unsigned)
+    {
+        if (is_unsigned) {
+            if (size == 1)
+                return Type::UInt8;
+            if (size == 2)
+                return Type::UInt16;
+            if (size == 4)
+                return Type::UInt32;
+            if (size == 8)
+                return Type::UInt64;
+        } else {
+            if (size == 1)
+                return Type::Int8;
+            if (size == 2)
+                return Type::Int16;
+            if (size == 4)
+                return Type::Int32;
+            if (size == 8)
+                return Type::Int64;
+        }
+
+        ASSERT_NOT_REACHED();
+    }
+
     template<typename T>
     static Type get_type()
     {
-        if (IsSame<T, u8>::value)
-            return Type::UInt8;
-        if (IsSame<T, u16>::value)
-            return Type::UInt16;
-        if (IsSame<T, u32>::value)
-            return Type::UInt32;
-        if (IsSame<T, u64>::value)
-            return Type::UInt64;
-        if (IsSame<T, i8>::value)
-            return Type::Int8;
-        if (IsSame<T, i16>::value)
-            return Type::Int16;
-        if (IsSame<T, i32>::value)
-            return Type::Int32;
-        if (IsSame<T, i64>::value)
-            return Type::Int64;
+        if (IsIntegral<T>::value)
+            return get_type_from_size(sizeof(T), IsUnsigned<T>::value);
 
         return Type::Custom;
     }
diff --git a/AK/StdLibExtras.h b/AK/StdLibExtras.h
index e6b58ebfee..154dea5e22 100644
--- a/AK/StdLibExtras.h
+++ b/AK/StdLibExtras.h
@@ -326,6 +326,7 @@ constexpr T&& forward(typename RemoveReference<T>::Type&& param) noexcept
 
 template<typename T>
 struct MakeUnsigned {
+    using Type = void;
 };
 template<>
 struct MakeUnsigned<signed char> {
@@ -507,6 +508,9 @@ using Void = void;
 template<typename... _Ignored>
 constexpr auto DependentFalse = false;
 
+template<typename T>
+using IsUnsigned = IsSame<T, MakeUnsigned<T>>;
+
 }
 
 using AK::AddConst;
diff --git a/AK/Tests/TestFormat.cpp b/AK/Tests/TestFormat.cpp
index 23431cf730..c88a006964 100644
--- a/AK/Tests/TestFormat.cpp
+++ b/AK/Tests/TestFormat.cpp
@@ -29,6 +29,12 @@
 #include <AK/String.h>
 #include <AK/StringBuilder.h>
 
+TEST_CASE(is_integral_works_properly)
+{
+    EXPECT(!IsIntegral<const char*>::value);
+    EXPECT(IsIntegral<unsigned long>::value);
+}
+
 TEST_CASE(format_string_literals)
 {
     EXPECT_EQ(String::formatted("prefix-{}-suffix", "abc"), "prefix-abc-suffix");
@@ -124,6 +130,11 @@ TEST_CASE(replacement_field)
     EXPECT_EQ(String::formatted("{:0{}}", 1, 3), "001");
 }
 
+TEST_CASE(replacement_field_regression)
+{
+    EXPECT_EQ(String::formatted("{:{}}", "", static_cast<unsigned long>(6)), "      ");
+}
+
 TEST_CASE(complex_string_specifiers)
 {
     EXPECT_EQ(String::formatted("{:.8}", "123456789"), "12345678");