AK+LibJS: Remove null state from DeprecatedFlyString :^)

Author: https://github.com/DanShaders
Commit: https://github.com/SerenityOS/serenity/commit/8ac0e3f0e5
Pull-request: https://github.com/SerenityOS/serenity/pull/22926
Reviewed-by: https://github.com/ADKaster ✅
Reviewed-by: https://github.com/davidot ✅

AK/ByteString.cpp

@@ -338,7 +338,7 @@ ByteString escape_html_entities(StringView html)
 }
 
 ByteString::ByteString(DeprecatedFlyString const& string)
-    : m_impl(*(string.impl() ?: &StringImpl::the_empty_stringimpl()))
+    : m_impl(string.impl())
 {
 }
 

AK/ByteString.h

@@ -239,7 +239,7 @@ public:
         return StringImpl::the_empty_stringimpl();
     }
 
-    [[nodiscard]] StringImpl const* impl() const { return m_impl.ptr(); }
+    [[nodiscard]] NonnullRefPtr<StringImpl const> impl() const { return m_impl; }
 
     ByteString& operator=(ByteString&& other)
     {
@@ -325,12 +325,12 @@ private:
 
 template<>
 struct Traits<ByteString> : public DefaultTraits<ByteString> {
-    static unsigned hash(ByteString const& s) { return s.impl() ? s.impl()->hash() : 0; }
+    static unsigned hash(ByteString const& s) { return s.impl()->hash(); }
 };
 
 // FIXME: Rename this to indicate that it's about ASCII-only case insensitivity.
 struct CaseInsensitiveStringTraits : public Traits<ByteString> {
-    static unsigned hash(ByteString const& s) { return s.impl() ? s.impl()->case_insensitive_hash() : 0; }
+    static unsigned hash(ByteString const& s) { return s.impl()->case_insensitive_hash(); }
     static bool equals(ByteString const& a, ByteString const& b) { return a.equals_ignoring_ascii_case(b); }
 };
 

AK/DeprecatedFlyString.cpp

@@ -14,12 +14,10 @@
 
 namespace AK {
 
-struct DeprecatedFlyStringImplTraits : public Traits<StringImpl*> {
+struct DeprecatedFlyStringImplTraits : public Traits<StringImpl const*> {
-    static unsigned hash(StringImpl const* s) { return s ? s->hash() : 0; }
+    static unsigned hash(StringImpl const* s) { return s->hash(); }
     static bool equals(StringImpl const* a, StringImpl const* b)
     {
-        VERIFY(a);
-        VERIFY(b);
         return *a == *b;
     }
 };
@@ -37,23 +35,24 @@ void DeprecatedFlyString::did_destroy_impl(Badge<StringImpl>, StringImpl& impl)
 }
 
 DeprecatedFlyString::DeprecatedFlyString(ByteString const& string)
+    : m_impl(string.impl())
 {
-    if (string.impl()->is_fly()) {
+    if (string.impl()->is_fly())
-        m_impl = string.impl();
         return;
-    }
+
-    auto it = fly_impls().find(const_cast<StringImpl*>(string.impl()));
+    auto it = fly_impls().find(string.impl());
     if (it == fly_impls().end()) {
-        fly_impls().set(const_cast<StringImpl*>(string.impl()));
+        fly_impls().set(string.impl());
         string.impl()->set_fly({}, true);
         m_impl = string.impl();
     } else {
         VERIFY((*it)->is_fly());
-        m_impl = *it;
+        m_impl = **it;
     }
 }
 
 DeprecatedFlyString::DeprecatedFlyString(StringView string)
+    : m_impl(StringImpl::the_empty_stringimpl())
 {
     if (string.is_null())
         return;
@@ -67,7 +66,7 @@ DeprecatedFlyString::DeprecatedFlyString(StringView string)
         m_impl = new_string.impl();
     } else {
         VERIFY((*it)->is_fly());
-        m_impl = *it;
+        m_impl = **it;
     }
 }
 

AK/DeprecatedFlyString.h

@@ -13,7 +13,10 @@ namespace AK {
 
 class DeprecatedFlyString {
 public:
-    DeprecatedFlyString() = default;
+    DeprecatedFlyString()
+        : m_impl(StringImpl::the_empty_stringimpl())
+    {
+    }
     DeprecatedFlyString(DeprecatedFlyString const& other)
         : m_impl(other.impl())
     {
@@ -49,8 +52,7 @@ public:
         return *this;
     }
 
-    bool is_empty() const { return !m_impl || !m_impl->length(); }
+    bool is_empty() const { return !m_impl->length(); }
-    bool is_null() const { return !m_impl; }
 
     bool operator==(DeprecatedFlyString const& other) const { return m_impl == other.m_impl; }
 
@@ -60,12 +62,12 @@ public:
 
     bool operator==(char const*) const;
 
-    StringImpl const* impl() const { return m_impl; }
+    NonnullRefPtr<StringImpl const> impl() const { return m_impl; }
-    char const* characters() const { return m_impl ? m_impl->characters() : nullptr; }
+    char const* characters() const { return m_impl->characters(); }
-    size_t length() const { return m_impl ? m_impl->length() : 0; }
+    size_t length() const { return m_impl->length(); }
 
-    ALWAYS_INLINE u32 hash() const { return m_impl ? m_impl->existing_hash() : 0; }
+    ALWAYS_INLINE u32 hash() const { return m_impl->existing_hash(); }
-    ALWAYS_INLINE StringView view() const { return m_impl ? m_impl->view() : StringView {}; }
+    ALWAYS_INLINE StringView view() const { return m_impl->view(); }
 
     DeprecatedFlyString to_lowercase() const;
 
@@ -88,7 +90,7 @@ public:
     }
 
 private:
-    RefPtr<StringImpl const> m_impl;
+    NonnullRefPtr<StringImpl const> m_impl;
 };
 
 template<>

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -1261,7 +1261,6 @@ Completion ECMAScriptFunctionObject::ordinary_call_evaluate_body()
 
 void ECMAScriptFunctionObject::set_name(DeprecatedFlyString const& name)
 {
-    VERIFY(!name.is_null());
     auto& vm = this->vm();
     m_name = name;
     m_name_string = PrimitiveString::create(vm, m_name);

Userland/Libraries/LibJS/Runtime/PropertyKey.h

@@ -69,7 +69,6 @@ public:
         : m_type(Type::String)
         , m_string(DeprecatedFlyString(string))
     {
-        VERIFY(!m_string.is_null());
     }
 
     PropertyKey(DeprecatedFlyString string, StringMayBeNumber string_may_be_number = StringMayBeNumber::Yes)
@@ -77,7 +76,6 @@ public:
         , m_type(Type::String)
         , m_string(move(string))
     {
-        VERIFY(!m_string.is_null());
     }
 
     PropertyKey(NonnullGCPtr<Symbol> symbol)

Userland/Libraries/LibJS/Runtime/StringOrSymbol.h

@@ -33,7 +33,6 @@ public:
     StringOrSymbol(DeprecatedFlyString const& string)
         : m_string(string)
     {
-        VERIFY(!string.is_null());
     }
 
     ~StringOrSymbol()