From 2457118024a9d3b39178bba0e92c34923b381f20 Mon Sep 17 00:00:00 2001 From: Jonne Ransijn Date: Mon, 28 Oct 2024 22:53:16 +0100 Subject: [PATCH] AK: Add template specializations for `Optional<{,Fly}String>` Slice the size of `Optional<{,Fly}String>` in half by introducing `UINTPTR_MAX` as an invalid bit pattern for these values. --- AK/FlyString.cpp | 2 + AK/FlyString.h | 118 ++++++++++++++++++++++++++ AK/Forward.h | 6 ++ AK/Optional.h | 4 +- AK/String.h | 112 +++++++++++++++++++++++++ AK/StringBase.cpp | 7 ++ AK/StringBase.h | 8 ++ Tests/AK/TestOptional.cpp | 172 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 427 insertions(+), 2 deletions(-) diff --git a/AK/FlyString.cpp b/AK/FlyString.cpp index 55fb34dd47d..13f08ddfc4e 100644 --- a/AK/FlyString.cpp +++ b/AK/FlyString.cpp @@ -50,6 +50,8 @@ FlyString FlyString::from_utf8_without_validation(ReadonlyBytes string) FlyString::FlyString(String const& string) { + ASSERT(!string.is_invalid()); + if (string.is_short_string()) { m_data = string; return; diff --git a/AK/FlyString.h b/AK/FlyString.h index fd2c21e51a9..a0963e6b831 100644 --- a/AK/FlyString.h +++ b/AK/FlyString.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -80,12 +81,129 @@ public: } private: + friend class Optional; + + explicit FlyString(nullptr_t) + : m_data(Detail::StringBase(nullptr)) + { + } + explicit FlyString(Detail::StringBase data) : m_data(move(data)) { } Detail::StringBase m_data; + + bool is_invalid() const { return m_data.is_invalid(); } +}; + +template<> +class Optional : public OptionalBase { + template + friend class Optional; + +public: + using ValueType = FlyString; + + Optional() = default; + + template V> + Optional(V) { } + + Optional(Optional const& other) + { + if (other.has_value()) + m_value = other.m_value; + } + + Optional(Optional&& other) + : m_value(other.m_value) + { + } + + template + requires(!IsSame>) + explicit(!IsConvertible) Optional(U&& value) + requires(!IsSame, Optional> && IsConstructible) + : m_value(forward(value)) + { + } + + template V> + Optional& operator=(V) + { + clear(); + return *this; + } + + Optional& operator=(Optional const& other) + { + if (this != &other) { + clear(); + m_value = other.m_value; + } + return *this; + } + + Optional& operator=(Optional&& other) + { + if (this != &other) { + clear(); + m_value = other.m_value; + } + return *this; + } + + template + ALWAYS_INLINE bool operator==(Optional const& other) const + { + return has_value() == other.has_value() && (!has_value() || value() == other.value()); + } + + template + ALWAYS_INLINE bool operator==(O const& other) const + { + return has_value() && value() == other; + } + + void clear() + { + m_value = FlyString(nullptr); + } + + [[nodiscard]] bool has_value() const + { + return !m_value.is_invalid(); + } + + [[nodiscard]] FlyString& value() & + { + VERIFY(has_value()); + return m_value; + } + + [[nodiscard]] FlyString const& value() const& + { + VERIFY(has_value()); + return m_value; + } + + [[nodiscard]] FlyString value() && + { + return release_value(); + } + + [[nodiscard]] FlyString release_value() + { + VERIFY(has_value()); + FlyString released_value = m_value; + clear(); + return released_value; + } + +private: + FlyString m_value = FlyString(nullptr); }; template<> diff --git a/AK/Forward.h b/AK/Forward.h index 1bc4d95d37c..68b5af27b55 100644 --- a/AK/Forward.h +++ b/AK/Forward.h @@ -125,6 +125,12 @@ class NonnullOwnPtr; template class Optional; +template<> +class Optional; + +template<> +class Optional; + template class RefPtr; diff --git a/AK/Optional.h b/AK/Optional.h index 0e2c39b7f83..1bb6441bcbe 100644 --- a/AK/Optional.h +++ b/AK/Optional.h @@ -211,7 +211,7 @@ public: template requires(IsConstructible && !IsSpecializationOf && !IsSpecializationOf) ALWAYS_INLINE explicit Optional(Optional const& other) - : m_has_value(other.m_has_value) + : m_has_value(other.has_value()) { if (other.has_value()) new (&m_storage) T(other.value()); @@ -219,7 +219,7 @@ public: template requires(IsConstructible && !IsSpecializationOf && !IsSpecializationOf) ALWAYS_INLINE explicit Optional(Optional&& other) - : m_has_value(other.m_has_value) + : m_has_value(other.has_value()) { if (other.has_value()) new (&m_storage) T(other.release_value()); diff --git a/AK/String.h b/AK/String.h index 93c04983fe7..9461d16e875 100644 --- a/AK/String.h +++ b/AK/String.h @@ -211,6 +211,7 @@ public: private: friend class ::AK::FlyString; + friend class Optional; using ShortString = Detail::ShortString; @@ -218,6 +219,117 @@ private: : StringBase(move(base)) { } + + explicit constexpr String(nullptr_t) + : StringBase(nullptr) + { + } +}; + +template<> +class Optional : public OptionalBase { + template + friend class Optional; + +public: + using ValueType = String; + + Optional() = default; + + template V> + Optional(V) { } + + Optional(Optional const& other) + { + if (other.has_value()) + m_value = other.m_value; + } + + Optional(Optional&& other) + : m_value(move(other.m_value)) + { + } + + template + requires(!IsSame>) + explicit(!IsConvertible) Optional(U&& value) + requires(!IsSame, Optional> && IsConstructible) + : m_value(forward(value)) + { + } + + template V> + Optional& operator=(V) + { + clear(); + return *this; + } + + Optional& operator=(Optional const& other) + { + if (this != &other) { + m_value = other.m_value; + } + return *this; + } + + Optional& operator=(Optional&& other) + { + if (this != &other) { + m_value = move(other.m_value); + } + return *this; + } + + template + ALWAYS_INLINE bool operator==(Optional const& other) const + { + return has_value() == other.has_value() && (!has_value() || value() == other.value()); + } + + template + ALWAYS_INLINE bool operator==(O const& other) const + { + return has_value() && value() == other; + } + + void clear() + { + m_value = String(nullptr); + } + + [[nodiscard]] bool has_value() const + { + return !m_value.is_invalid(); + } + + [[nodiscard]] String& value() & + { + VERIFY(has_value()); + return m_value; + } + + [[nodiscard]] String const& value() const& + { + VERIFY(has_value()); + return m_value; + } + + [[nodiscard]] String value() && + { + return release_value(); + } + + [[nodiscard]] String release_value() + { + VERIFY(has_value()); + String released_value = m_value; + clear(); + return released_value; + } + +private: + String m_value { nullptr }; }; template<> diff --git a/AK/StringBase.cpp b/AK/StringBase.cpp index 622266d1764..dd453bc4cf0 100644 --- a/AK/StringBase.cpp +++ b/AK/StringBase.cpp @@ -58,6 +58,7 @@ StringBase& StringBase::operator=(StringBase const& other) ReadonlyBytes StringBase::bytes() const { + ASSERT(!is_invalid()); if (is_short_string()) return m_short_string.bytes(); return m_data->bytes(); @@ -65,6 +66,7 @@ ReadonlyBytes StringBase::bytes() const u32 StringBase::hash() const { + ASSERT(!is_invalid()); if (is_short_string()) { auto bytes = this->bytes(); return string_hash(reinterpret_cast(bytes.data()), bytes.size()); @@ -74,6 +76,7 @@ u32 StringBase::hash() const size_t StringBase::byte_count() const { + ASSERT(!is_invalid()); if (is_short_string()) return m_short_string.byte_count_and_short_string_flag >> 1; return m_data->byte_count(); @@ -81,6 +84,7 @@ size_t StringBase::byte_count() const bool StringBase::operator==(StringBase const& other) const { + ASSERT(!is_invalid()); if (is_short_string()) return m_data == other.m_data; if (other.is_short_string()) @@ -92,6 +96,7 @@ bool StringBase::operator==(StringBase const& other) const void StringBase::replace_with_string_builder(StringBuilder& builder) { + ASSERT(!is_invalid()); if (builder.length() <= MAX_SHORT_STRING_BYTE_COUNT) { return replace_with_new_short_string(builder.length(), [&](Bytes buffer) { builder.string_view().bytes().copy_to(buffer); @@ -105,6 +110,7 @@ void StringBase::replace_with_string_builder(StringBuilder& builder) ErrorOr StringBase::replace_with_uninitialized_buffer(size_t byte_count) { + ASSERT(!is_invalid()); if (byte_count <= MAX_SHORT_STRING_BYTE_COUNT) return replace_with_uninitialized_short_string(byte_count); @@ -116,6 +122,7 @@ ErrorOr StringBase::replace_with_uninitialized_buffer(size_t byte_count) ErrorOr StringBase::substring_from_byte_offset_with_shared_superstring(size_t start, size_t length) const { + ASSERT(!is_invalid()); VERIFY(start + length <= byte_count()); if (length == 0) diff --git a/AK/StringBase.h b/AK/StringBase.h index e6086be2f72..39687e61b94 100644 --- a/AK/StringBase.h +++ b/AK/StringBase.h @@ -75,6 +75,8 @@ public: [[nodiscard]] ALWAYS_INLINE FlatPtr raw(Badge) const { return bit_cast(m_data); } protected: + bool is_invalid() const { return m_invalid_tag == UINTPTR_MAX; } + template ErrorOr replace_with_new_string(size_t byte_count, Func&& callback) { @@ -107,6 +109,11 @@ private: explicit StringBase(NonnullRefPtr); + explicit constexpr StringBase(nullptr_t) + : m_invalid_tag(UINTPTR_MAX) + { + } + explicit constexpr StringBase(ShortString short_string) : m_short_string(short_string) { @@ -129,6 +136,7 @@ private: union { ShortString m_short_string; Detail::StringData const* m_data { nullptr }; + uintptr_t m_invalid_tag; }; }; diff --git a/Tests/AK/TestOptional.cpp b/Tests/AK/TestOptional.cpp index e71221b132a..cb17d62debd 100644 --- a/Tests/AK/TestOptional.cpp +++ b/Tests/AK/TestOptional.cpp @@ -8,7 +8,9 @@ #include #include +#include #include +#include #include TEST_CASE(basic_optional) @@ -269,3 +271,173 @@ TEST_CASE(comparison_reference) EXPECT_EQ(opt1, opt2); EXPECT_NE(opt1, opt3); } + +TEST_CASE(string_specialization) +{ + EXPECT_EQ(sizeof(Optional), sizeof(String)); + + { + Optional foo; + + EXPECT(!foo.has_value()); + + foo = "long_enough_to_be_allocated"_string; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo = "initial_value"_string; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "initial_value"sv); + + foo = "long_enough_to_be_allocated"_string; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT(!foo.has_value()); + + String bar = "long_enough_to_be_allocated"_string; + foo = bar; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT(!foo.has_value()); + + Optional bar = "long_enough_to_be_allocated"_string; + foo = bar; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + EXPECT(bar.has_value()); + EXPECT_EQ(bar.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT(!foo.has_value()); + + foo = Optional { "long_enough_to_be_allocated"_string }; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo = "long_enough_to_be_allocated"_string; + + EXPECT_EQ(foo.value_or("fallback_value"_string), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT_EQ(foo.value_or("fallback_value"_string), "fallback_value"sv); + } + + { + EXPECT_EQ((Optional { "long_enough_to_be_allocated"_string }).value_or("fallback_value"_string), "long_enough_to_be_allocated"sv); + } + + { + EXPECT_EQ((Optional {}).value_or("fallback_value"_string), "fallback_value"sv); + } +} + +TEST_CASE(flystring_specialization) +{ + EXPECT_EQ(sizeof(Optional), sizeof(FlyString)); + + { + Optional foo; + + EXPECT(!foo.has_value()); + + foo = "long_enough_to_be_allocated"_fly_string; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo = "initial_value"_fly_string; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "initial_value"sv); + + foo = "long_enough_to_be_allocated"_fly_string; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT(!foo.has_value()); + + FlyString bar = "long_enough_to_be_allocated"_fly_string; + foo = bar; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT(!foo.has_value()); + + Optional bar = "long_enough_to_be_allocated"_fly_string; + foo = bar; + + EXPECT(bar.has_value()); + EXPECT_EQ(bar.value(), "long_enough_to_be_allocated"sv); + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT(!foo.has_value()); + + foo = Optional { "long_enough_to_be_allocated"_fly_string }; + + EXPECT(foo.has_value()); + EXPECT_EQ(foo.value(), "long_enough_to_be_allocated"sv); + } + + { + Optional foo = "long_enough_to_be_allocated"_fly_string; + + EXPECT_EQ(foo.value_or("fallback_value"_fly_string), "long_enough_to_be_allocated"sv); + } + + { + Optional foo; + + EXPECT_EQ(foo.value_or("fallback_value"_fly_string), "fallback_value"sv); + } + + { + EXPECT_EQ((Optional { "long_enough_to_be_allocated"_fly_string }).value_or("fallback_value"_fly_string), "long_enough_to_be_allocated"sv); + } + + { + EXPECT_EQ((Optional {}).value_or("fallback_value"_fly_string), "fallback_value"sv); + } +}