AK: Store data in FlyString as StringBase

Unfortunately, it is not clear to me how to split this commit into
several atomic ones.

AK/FlyString.cpp

@@ -15,20 +15,10 @@ namespace AK {
 
 static auto& all_fly_strings()
 {
-    static Singleton<HashMap<StringView, uintptr_t>> table;
+    static Singleton<HashMap<StringView, Detail::StringBase>> table;
     return *table;
 }
 
-FlyString::FlyString()
-    : m_data(String {}.to_fly_string_data({}))
-{
-}
-
-FlyString::~FlyString()
-{
-    String::unref_fly_string_data({}, m_data);
-}
-
 ErrorOr<FlyString> FlyString::from_utf8(StringView string)
 {
     return FlyString { TRY(String::from_utf8(string)) };
@@ -37,21 +27,19 @@ ErrorOr<FlyString> FlyString::from_utf8(StringView string)
 FlyString::FlyString(String const& string)
 {
     if (string.is_short_string()) {
-        m_data = string.to_fly_string_data({});
+        m_data = string;
         return;
     }
 
     auto it = all_fly_strings().find(string.bytes_as_string_view());
     if (it == all_fly_strings().end()) {
-        m_data = string.to_fly_string_data({});
+        m_data = string;
 
         all_fly_strings().set(string.bytes_as_string_view(), m_data);
         string.did_create_fly_string({});
     } else {
         m_data = it->value;
     }
-
-    String::ref_fly_string_data({}, m_data);
 }
 
 FlyString& FlyString::operator=(String const& string)
@@ -60,36 +48,6 @@ FlyString& FlyString::operator=(String const& string)
     return *this;
 }
 
-FlyString::FlyString(FlyString const& other)
-    : m_data(other.m_data)
-{
-    String::ref_fly_string_data({}, m_data);
-}
-
-FlyString& FlyString::operator=(FlyString const& other)
-{
-    if (this != &other) {
-        m_data = other.m_data;
-        String::ref_fly_string_data({}, m_data);
-    }
-
-    return *this;
-}
-
-FlyString::FlyString(FlyString&& other)
-    : m_data(other.m_data)
-{
-    other.m_data = String {}.to_fly_string_data({});
-}
-
-FlyString& FlyString::operator=(FlyString&& other)
-{
-    m_data = other.m_data;
-    other.m_data = String {}.to_fly_string_data({});
-
-    return *this;
-}
-
 bool FlyString::is_empty() const
 {
     return bytes_as_string_view().is_empty();
@@ -97,7 +55,7 @@ bool FlyString::is_empty() const
 
 unsigned FlyString::hash() const
 {
-    return String::fly_string_data_to_hash({}, m_data);
+    return m_data.hash();
 }
 
 u32 FlyString::ascii_case_insensitive_hash() const
@@ -112,7 +70,8 @@ FlyString::operator String() const
 
 String FlyString::to_string() const
 {
-    return String::fly_string_data_to_string({}, m_data);
+    Detail::StringBase copy = m_data;
+    return String(move(copy));
 }
 
 Utf8View FlyString::code_points() const
@@ -127,7 +86,7 @@ ReadonlyBytes FlyString::bytes() const
 
 StringView FlyString::bytes_as_string_view() const
 {
-    return String::fly_string_data_to_string_view({}, m_data);
+    return m_data.bytes();
 }
 
 bool FlyString::operator==(FlyString const& other) const
@@ -137,10 +96,7 @@ bool FlyString::operator==(FlyString const& other) const
 
 bool FlyString::operator==(String const& other) const
 {
-    if (m_data == other.to_fly_string_data({}))
+    return m_data == other;
-        return true;
-
-    return bytes_as_string_view() == other.bytes_as_string_view();
 }
 
 bool FlyString::operator==(StringView string) const
@@ -158,7 +114,7 @@ void FlyString::did_destroy_fly_string_data(Badge<Detail::StringData>, StringVie
     all_fly_strings().remove(string_data);
 }
 
-uintptr_t FlyString::data(Badge<String>) const
+Detail::StringBase FlyString::data(Badge<String>) const
 {
     return m_data;
 }

AK/FlyString.h

@@ -16,9 +16,11 @@
 namespace AK {
 
 class FlyString {
+    AK_MAKE_DEFAULT_MOVABLE(FlyString);
+    AK_MAKE_DEFAULT_COPYABLE(FlyString);
+
 public:
-    FlyString();
+    FlyString() = default;
-    ~FlyString();
 
     static ErrorOr<FlyString> from_utf8(StringView);
     template<typename T>
@@ -28,12 +30,6 @@ public:
     FlyString(String const&);
     FlyString& operator=(String const&);
 
-    FlyString(FlyString const&);
-    FlyString& operator=(FlyString const&);
-
-    FlyString(FlyString&&);
-    FlyString& operator=(FlyString&&);
-
     [[nodiscard]] bool is_empty() const;
     [[nodiscard]] unsigned hash() const;
     [[nodiscard]] u32 ascii_case_insensitive_hash() const;
@@ -53,7 +49,7 @@ public:
     [[nodiscard]] int operator<=>(FlyString const& other) const;
 
     static void did_destroy_fly_string_data(Badge<Detail::StringData>, StringView);
-    [[nodiscard]] uintptr_t data(Badge<String>) const;
+    [[nodiscard]] Detail::StringBase data(Badge<String>) const;
 
     // This is primarily interesting to unit tests.
     [[nodiscard]] static size_t number_of_fly_strings();
@@ -80,9 +76,7 @@ public:
     }
 
 private:
-    // This will hold either the pointer to the Detail::StringData it represents or the raw bytes of
+    Detail::StringBase m_data;
-    // an inlined short string.
-    uintptr_t m_data { 0 };
 };
 
 template<>

AK/String.cpp

@@ -41,12 +41,19 @@ StringData::StringData(StringData const& superstring, size_t start, size_t byte_
 
 StringData::~StringData()
 {
-    if (m_is_fly_string)
-        FlyString::did_destroy_fly_string_data({}, bytes_as_string_view());
     if (m_substring)
         substring_data().superstring->unref();
 }
 
+void StringData::unref() const
+{
+    if (m_is_fly_string && m_ref_count == 2) {
+        m_is_fly_string = false; // Otherwise unref from did_destory_fly_string_data will cause infinite recursion.
+        FlyString::did_destroy_fly_string_data({}, bytes_as_string_view());
+    }
+    RefCounted::unref();
+}
+
 constexpr size_t allocation_size_for_string_data(size_t length)
 {
     return sizeof(StringData) + (sizeof(char) * length);
@@ -227,9 +234,7 @@ Optional<size_t> String::find_byte_offset(StringView substring, size_t from_byte
 
 bool String::operator==(FlyString const& other) const
 {
-    if (reinterpret_cast<uintptr_t>(m_data) == other.data({}))
+    return static_cast<StringBase const&>(*this) == other.data({});
-        return true;
-    return bytes_as_string_view() == other.bytes_as_string_view();
 }
 
 bool String::operator==(StringView other) const
@@ -367,67 +372,6 @@ unsigned Traits<String>::hash(String const& string)
     return string.hash();
 }
 
-String String::fly_string_data_to_string(Badge<FlyString>, uintptr_t const& data)
-{
-    if (has_short_string_bit(data))
-        return String { *reinterpret_cast<ShortString const*>(&data) };
-
-    auto const* string_data = reinterpret_cast<Detail::StringData const*>(data);
-    return String { NonnullRefPtr<Detail::StringData const>(*string_data) };
-}
-
-StringView String::fly_string_data_to_string_view(Badge<FlyString>, uintptr_t const& data)
-{
-    if (has_short_string_bit(data)) {
-        auto const* short_string = reinterpret_cast<ShortString const*>(&data);
-        return short_string->bytes();
-    }
-
-    auto const* string_data = reinterpret_cast<Detail::StringData const*>(data);
-    return string_data->bytes_as_string_view();
-}
-
-u32 String::fly_string_data_to_hash(Badge<FlyString>, uintptr_t const& data)
-{
-    if (has_short_string_bit(data)) {
-        auto const* short_string = reinterpret_cast<ShortString const*>(&data);
-        auto bytes = short_string->bytes();
-        return string_hash(reinterpret_cast<char const*>(bytes.data()), bytes.size());
-    }
-
-    auto const* string_data = reinterpret_cast<Detail::StringData const*>(data);
-    return string_data->hash();
-}
-
-uintptr_t String::to_fly_string_data(Badge<FlyString>) const
-{
-    return reinterpret_cast<uintptr_t>(m_data);
-}
-
-void String::ref_fly_string_data(Badge<FlyString>, uintptr_t data)
-{
-    if (has_short_string_bit(data))
-        return;
-
-    auto const* string_data = reinterpret_cast<Detail::StringData const*>(data);
-    string_data->ref();
-}
-
-void String::unref_fly_string_data(Badge<FlyString>, uintptr_t data)
-{
-    if (has_short_string_bit(data))
-        return;
-
-    auto const* string_data = reinterpret_cast<Detail::StringData const*>(data);
-    string_data->unref();
-}
-
-void String::did_create_fly_string(Badge<FlyString>) const
-{
-    VERIFY(!is_short_string());
-    m_data->set_fly_string(true);
-}
-
 ByteString String::to_byte_string() const
 {
     return ByteString(bytes_as_string_view());

AK/String.h

@@ -178,15 +178,6 @@ public:
         return builder.to_string();
     }
 
-    [[nodiscard]] static String fly_string_data_to_string(Badge<FlyString>, uintptr_t const&);
-    [[nodiscard]] static StringView fly_string_data_to_string_view(Badge<FlyString>, uintptr_t const&);
-    [[nodiscard]] static u32 fly_string_data_to_hash(Badge<FlyString>, uintptr_t const&);
-    [[nodiscard]] uintptr_t to_fly_string_data(Badge<FlyString>) const;
-
-    static void ref_fly_string_data(Badge<FlyString>, uintptr_t);
-    static void unref_fly_string_data(Badge<FlyString>, uintptr_t);
-    void did_create_fly_string(Badge<FlyString>) const;
-
     // FIXME: Remove these once all code has been ported to String
     [[nodiscard]] ByteString to_byte_string() const;
     static ErrorOr<String> from_byte_string(ByteString const&);
@@ -195,6 +186,8 @@ public:
     static ErrorOr<String> from_byte_string(T&&) = delete;
 
 private:
+    friend FlyString;
+
     using ShortString = Detail::ShortString;
 
     explicit constexpr String(StringBase&& base)

AK/StringBase.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/Badge.h>
 #include <AK/StringBase.h>
 #include <AK/StringInternals.h>
 
@@ -81,9 +82,19 @@ bool StringBase::operator==(StringBase const& other) const
 {
     if (is_short_string())
         return m_data == other.m_data;
+    if (other.is_short_string())
+        return false;
+    if (m_data->is_fly_string() && other.m_data->is_fly_string())
+        return m_data == other.m_data;
     return bytes() == other.bytes();
 }
 
+void StringBase::did_create_fly_string(Badge<FlyString>) const
+{
+    VERIFY(!is_short_string());
+    m_data->set_fly_string(true);
+}
+
 ErrorOr<Bytes> StringBase::replace_with_uninitialized_buffer(size_t byte_count)
 {
     if (byte_count <= MAX_SHORT_STRING_BYTE_COUNT)

AK/StringBase.h

@@ -68,15 +68,12 @@ public:
 
     [[nodiscard]] bool operator==(StringBase const&) const;
 
+    void did_create_fly_string(Badge<FlyString>) const;
+
 protected:
     // NOTE: If the least significant bit of the pointer is set, this is a short string.
     static constexpr uintptr_t SHORT_STRING_FLAG = 1;
 
-    static constexpr bool has_short_string_bit(uintptr_t data)
-    {
-        return (data & SHORT_STRING_FLAG) != 0;
-    }
-
     explicit StringBase(NonnullRefPtr<Detail::StringData const>);
 
     explicit constexpr StringBase(ShortString short_string)

AK/StringInternals.h

@@ -25,6 +25,8 @@ public:
 
     void operator delete(void* ptr);
 
+    void unref() const;
+
     ~StringData();
 
     SubstringData const& substring_data() const