diff --git a/AK/Format.cpp b/AK/Format.cpp index 95ad46d0f4..20d8066119 100644 --- a/AK/Format.cpp +++ b/AK/Format.cpp @@ -217,16 +217,40 @@ bool FormatParser::consume_replacement_field(size_t& index) return true; } +ErrorOr FormatBuilder::append(char ch) +{ + if (m_string_builder) + return m_string_builder->try_append(ch); + m_utf16_builder->append_ascii(ch); + return {}; +} + +ErrorOr FormatBuilder::append(StringView string) +{ + if (m_string_builder) + return m_string_builder->try_append(string); + m_utf16_builder->append(Utf16String::from_utf8_without_validation(string)); + return {}; +} + +ErrorOr FormatBuilder::append(Utf16View const& string) +{ + if (m_string_builder) + return m_string_builder->try_append(string); + m_utf16_builder->append(string); + return {}; +} + ErrorOr FormatBuilder::put_padding(char fill, size_t amount) { for (size_t i = 0; i < amount; ++i) - TRY(m_builder.try_append(fill)); + TRY(append(fill)); return {}; } ErrorOr FormatBuilder::put_literal(StringView value) { for (size_t i = 0; i < value.length(); ++i) { - TRY(m_builder.try_append(value[i])); + TRY(append(value[i])); if (value[i] == '{' || value[i] == '}') ++i; } @@ -247,22 +271,28 @@ ErrorOr FormatBuilder::put_string( value = value.substring_view(0, used_by_string); if (align == Align::Left || align == Align::Default) { - TRY(m_builder.try_append(value)); + TRY(append(value)); TRY(put_padding(fill, used_by_padding)); } else if (align == Align::Center) { auto const used_by_left_padding = used_by_padding / 2; auto const used_by_right_padding = ceil_div(used_by_padding, 2); TRY(put_padding(fill, used_by_left_padding)); - TRY(m_builder.try_append(value)); + TRY(append(value)); TRY(put_padding(fill, used_by_right_padding)); } else if (align == Align::Right) { TRY(put_padding(fill, used_by_padding)); - TRY(m_builder.try_append(value)); + TRY(append(value)); } return {}; } +ErrorOr FormatBuilder::put_string(Utf16View const& value) +{ + TRY(append(value)); + return {}; +} + ErrorOr FormatBuilder::put_u64( u64 value, u8 base, @@ -307,25 +337,25 @@ ErrorOr FormatBuilder::put_u64( auto const put_prefix = [&]() -> ErrorOr { if (is_negative) - TRY(m_builder.try_append('-')); + TRY(append('-')); else if (sign_mode == SignMode::Always) - TRY(m_builder.try_append('+')); + TRY(append('+')); else if (sign_mode == SignMode::Reserved) - TRY(m_builder.try_append(' ')); + TRY(append(' ')); if (prefix) { if (base == 2) { if (upper_case) - TRY(m_builder.try_append("0B"sv)); + TRY(append("0B"sv)); else - TRY(m_builder.try_append("0b"sv)); + TRY(append("0b"sv)); } else if (base == 8) { - TRY(m_builder.try_append("0"sv)); + TRY(append("0"sv)); } else if (base == 16) { if (upper_case) - TRY(m_builder.try_append("0X"sv)); + TRY(append("0X"sv)); else - TRY(m_builder.try_append("0x"sv)); + TRY(append("0x"sv)); } } return {}; @@ -333,7 +363,7 @@ ErrorOr FormatBuilder::put_u64( auto const put_digits = [&]() -> ErrorOr { for (size_t i = 0; i < used_by_digits; ++i) - TRY(m_builder.try_append(buffer[i])); + TRY(append(buffer[i])); return {}; }; @@ -795,7 +825,7 @@ ErrorOr FormatBuilder::put_hexdump(ReadonlyBytes bytes, size_t width, char TRY(put_padding(fill, 4)); for (size_t j = i - min(i, width); j < i; ++j) { auto ch = bytes[j]; - TRY(m_builder.try_append(ch >= 32 && ch <= 127 ? ch : '.')); // silly hack + TRY(append(ch >= 32 && ch <= 127 ? ch : '.')); // silly hack } return {}; }; @@ -827,9 +857,10 @@ ErrorOr vformat(StringBuilder& builder, StringView fmtstr, TypeErasedForma ErrorOr vformat(Utf16StringBuilder& builder, StringView fmtstr, TypeErasedFormatParams& params) { - StringBuilder string_builder(StringBuilder::Mode::UTF16); - TRY(vformat(string_builder, fmtstr, params)); - builder.append(string_builder.to_utf16_string()); + FormatBuilder fmtbuilder { builder }; + FormatParser parser { fmtstr }; + + TRY(vformat_impl(params, fmtbuilder, parser)); return {}; } diff --git a/AK/Format.h b/AK/Format.h index 09ad70adaf..f43acc87b2 100644 --- a/AK/Format.h +++ b/AK/Format.h @@ -217,7 +217,12 @@ public: }; explicit FormatBuilder(StringBuilder& builder) - : m_builder(builder) + : m_string_builder(&builder) + { + } + + explicit FormatBuilder(Utf16StringBuilder& builder) + : m_utf16_builder(&builder) { } @@ -231,6 +236,7 @@ public: size_t min_width = 0, size_t max_width = NumericLimits::max(), char fill = ' '); + ErrorOr put_string(Utf16View const& value); ErrorOr put_u64( u64 value, @@ -306,12 +312,22 @@ public: StringBuilder const& builder() const { - return m_builder; + VERIFY(m_string_builder); + return *m_string_builder; + } + StringBuilder& builder() + { + VERIFY(m_string_builder); + return *m_string_builder; } - StringBuilder& builder() { return m_builder; } private: - StringBuilder& m_builder; + ErrorOr append(char); + ErrorOr append(StringView); + ErrorOr append(Utf16View const&); + + StringBuilder* m_string_builder { nullptr }; + Utf16StringBuilder* m_utf16_builder { nullptr }; ErrorOr put_f64_with_precision( double value, diff --git a/AK/Time.cpp b/AK/Time.cpp index 1e66dd0628..31106f0831 100644 --- a/AK/Time.cpp +++ b/AK/Time.cpp @@ -703,9 +703,9 @@ ErrorOr UnixDateTime::to_string(StringView format, LocalTime local_time) Utf16String UnixDateTime::to_utf16_string(StringView format, LocalTime local_time) const { - StringBuilder builder(StringBuilder::Mode::UTF16); + StringBuilder builder; MUST(to_string_impl(builder, format, local_time)); - return builder.to_utf16_string(); + return Utf16String::from_utf8_without_validation(MUST(builder.to_string())); } ByteString UnixDateTime::to_byte_string(StringView format, LocalTime local_time) const diff --git a/AK/Utf16String.cpp b/AK/Utf16String.cpp index b984d8fb93..2fc5087abd 100644 --- a/AK/Utf16String.cpp +++ b/AK/Utf16String.cpp @@ -25,7 +25,7 @@ Utf16String Utf16String::from_utf8_with_replacement_character(StringView utf8_st if (utf8_view.validate(AllowLonelySurrogates::No)) return Utf16String::from_utf8_without_validation(utf8_string); - StringBuilder builder(StringBuilder::Mode::UTF16); + Utf16StringBuilder builder; for (auto code_point : utf8_view) { if (is_unicode_surrogate(code_point)) @@ -34,7 +34,7 @@ Utf16String Utf16String::from_utf8_with_replacement_character(StringView utf8_st builder.append_code_point(code_point); } - return builder.to_utf16_string(); + return builder.to_string(); } Utf16String Utf16String::from_ascii_without_validation(ReadonlyBytes ascii_string) @@ -158,9 +158,9 @@ Utf16String Utf16String::repeated(u32 code_point, size_t count) code_units[length_in_code_units++] = code_unit; }); - StringBuilder builder(StringBuilder::Mode::UTF16); + Utf16StringBuilder builder; builder.append_repeated({ code_units.data(), length_in_code_units }, count); - return builder.to_utf16_string(); + return builder.to_string(); } Utf16String Utf16String::to_well_formed() const @@ -180,7 +180,7 @@ String Utf16String::to_well_formed_utf8() const ErrorOr Formatter::format(FormatBuilder& builder, Utf16String const& utf16_string) { if (utf16_string.has_long_utf16_storage()) - return builder.builder().try_append(utf16_string.utf16_view()); + return builder.put_string(utf16_string.utf16_view()); return builder.put_string(utf16_string.ascii_view()); } diff --git a/AK/Utf16String.h b/AK/Utf16String.h index ce0081a1a6..3d3c4b8173 100644 --- a/AK/Utf16String.h +++ b/AK/Utf16String.h @@ -134,10 +134,16 @@ public: template ALWAYS_INLINE static Utf16String join(SeparatorType const& separator, CollectionType const& collection, StringView format = "{}"sv) { - StringBuilder builder(StringBuilder::Mode::UTF16); - builder.join(separator, collection, format); + Utf16StringBuilder builder; + bool first = true; + for (auto& item : collection) { + if (!first) + builder.appendff("{}", separator); + builder.appendff(format, item); + first = false; + } - return builder.to_utf16_string(); + return builder.to_string(); } static Utf16String repeated(u32 code_point, size_t count); diff --git a/AK/Utf16View.cpp b/AK/Utf16View.cpp index d8ac35bfcc..6beeff5f72 100644 --- a/AK/Utf16View.cpp +++ b/AK/Utf16View.cpp @@ -59,27 +59,27 @@ ErrorOr Utf16View::to_byte_string(AllowLonelySurrogates allow_lonely Utf16String Utf16View::to_ascii_lowercase() const { - StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units()); + Utf16StringBuilder builder(length_in_code_units()); for (size_t i = 0; i < length_in_code_units(); ++i) builder.append_code_unit(AK::to_ascii_lowercase(code_unit_at(i))); - return builder.to_utf16_string(); + return builder.to_string(); } Utf16String Utf16View::to_ascii_uppercase() const { - StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units()); + Utf16StringBuilder builder(length_in_code_units()); for (size_t i = 0; i < length_in_code_units(); ++i) builder.append_code_unit(AK::to_ascii_uppercase(code_unit_at(i))); - return builder.to_utf16_string(); + return builder.to_string(); } Utf16String Utf16View::to_ascii_titlecase() const { - StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units()); + Utf16StringBuilder builder(length_in_code_units()); bool next_is_upper = true; for (size_t i = 0; i < length_in_code_units(); ++i) { @@ -93,7 +93,7 @@ Utf16String Utf16View::to_ascii_titlecase() const next_is_upper = code_unit == u' '; } - return builder.to_utf16_string(); + return builder.to_string(); } Utf16String Utf16View::replace(char16_t needle, Utf16View const& replacement, ReplaceMode replace_mode) const @@ -106,7 +106,7 @@ Utf16String Utf16View::replace(Utf16View const& needle, Utf16View const& replace if (is_empty()) return {}; - StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units()); + Utf16StringBuilder builder(length_in_code_units()); auto remaining = *this; do { @@ -122,12 +122,12 @@ Utf16String Utf16View::replace(Utf16View const& needle, Utf16View const& replace } while (replace_mode == ReplaceMode::All && !remaining.is_empty()); builder.append(remaining); - return builder.to_utf16_string(); + return builder.to_string(); } Utf16String Utf16View::escape_html_entities() const { - StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units()); + Utf16StringBuilder builder(length_in_code_units()); for (auto code_point : *this) { if (code_point == '<') @@ -142,7 +142,7 @@ Utf16String Utf16View::escape_html_entities() const builder.append_code_point(code_point); } - return builder.to_utf16_string(); + return builder.to_string(); } bool Utf16View::is_ascii() const diff --git a/AK/Utf16View.h b/AK/Utf16View.h index 78b67bdc34..8619514d8a 100644 --- a/AK/Utf16View.h +++ b/AK/Utf16View.h @@ -687,7 +687,7 @@ template<> struct Formatter : Formatter { ErrorOr format(FormatBuilder& builder, Utf16View const& value) { - return builder.builder().try_append(value); + return builder.put_string(value); } }; diff --git a/Tests/AK/TestUtf16String.cpp b/Tests/AK/TestUtf16String.cpp index d02b805d0d..691925b38b 100644 --- a/Tests/AK/TestUtf16String.cpp +++ b/Tests/AK/TestUtf16String.cpp @@ -354,7 +354,7 @@ TEST_CASE(repeated) TEST_CASE(from_string_builder) { - StringBuilder builder(StringBuilder::Mode::UTF16); + Utf16StringBuilder builder; builder.append_code_point('a'); builder.append_code_point('b'); builder.append_code_point(0x1f600); @@ -363,7 +363,7 @@ TEST_CASE(from_string_builder) builder.append_code_point('c'); builder.append_code_point('d'); - auto string = builder.to_utf16_string(); + auto string = builder.to_string(); EXPECT_EQ(string.length_in_code_units(), 10uz); EXPECT_EQ(string.length_in_code_points(), 7uz); EXPECT_EQ(string, "ab😀𐀀🍕cd"sv); @@ -371,8 +371,8 @@ TEST_CASE(from_string_builder) TEST_CASE(from_string_builder_alignment) { - StringBuilder builder(StringBuilder::Mode::UTF16); - builder.append("\u00a0"sv); + Utf16StringBuilder builder; + builder.append(Utf16String::from_utf8_without_validation("\u00a0"sv)); builder.append(R"~~( )~~"sv); - auto string1 = builder.to_utf16_string(); + auto string1 = builder.to_string(); auto string2 = string1.to_utf8(); EXPECT_EQ(string1.code_unit_at(0), 0x00a0); @@ -415,10 +415,7 @@ TEST_CASE(from_ipc_stream) { auto data = u"hello 😀 there!"sv; - StringBuilder builder(StringBuilder::Mode::UTF16); - builder.append(data); - - auto buffer = MUST(builder.to_byte_buffer()); + auto buffer = MUST(ByteBuffer::copy({ reinterpret_cast(data.utf16_span().data()), data.length_in_code_units() * sizeof(char16_t) })); FixedMemoryStream stream { buffer.bytes() }; auto string = TRY_OR_FAIL(Utf16String::from_ipc_stream(stream, data.length_in_code_units(), false)); @@ -438,10 +435,7 @@ TEST_CASE(from_ipc_stream) { auto data = u"😀"sv; - StringBuilder builder(StringBuilder::Mode::UTF16); - builder.append(data); - - auto buffer = MUST(builder.to_byte_buffer()); + auto buffer = MUST(ByteBuffer::copy({ reinterpret_cast(data.utf16_span().data()), data.length_in_code_units() * sizeof(char16_t) })); FixedMemoryStream stream { buffer.bytes() }; auto result = Utf16String::from_ipc_stream(stream, data.length_in_code_units(), true); @@ -450,10 +444,7 @@ TEST_CASE(from_ipc_stream) { auto data = u"hello 😀 there!"sv; - StringBuilder builder(StringBuilder::Mode::UTF16); - builder.append(data); - - auto buffer = MUST(builder.to_byte_buffer()); + auto buffer = MUST(ByteBuffer::copy({ reinterpret_cast(data.utf16_span().data()), data.length_in_code_units() * sizeof(char16_t) })); FixedMemoryStream stream { buffer.bytes() }; auto result = Utf16String::from_ipc_stream(stream, data.length_in_code_units(), true); @@ -1251,17 +1242,16 @@ TEST_CASE(optional) EXPECT_EQ(released, u"well 😀 hello"sv); } -TEST_CASE(utf16_builder_clear_resets_ascii_flag) +TEST_CASE(utf16_string_builder_clear_resets_ascii_flag) { - // Regression test: StringBuilder(UTF16) must reset m_utf16_builder_is_ascii - // on clear(). Without this, a builder that previously held non-ASCII content - // stores subsequent ASCII as char16_t, and to_utf16_string() corrupts the - // first code unit via placement-new overlap in from_string_builder(). - StringBuilder builder(StringBuilder::Mode::UTF16); + // Regression test: Utf16StringBuilder must reset its ASCII state on clear(). + // Without this, a builder that previously held non-ASCII content stores + // subsequent ASCII as char16_t, corrupting the first code unit when adopted. + Utf16StringBuilder builder; // 1. Append a non-ASCII code point to force m_utf16_builder_is_ascii = false. builder.append_code_point(0x00D7); // U+00D7 MULTIPLICATION SIGN (×) - auto first = builder.to_utf16_string(); + auto first = builder.to_string(); EXPECT_EQ(first.length_in_code_units(), 1u); EXPECT_EQ(first.code_unit_at(0), 0x00D7); @@ -1271,7 +1261,7 @@ TEST_CASE(utf16_builder_clear_resets_ascii_flag) for (int i = 0; i < 100; ++i) builder.append_code_point(' '); - auto second = builder.to_utf16_string(); + auto second = builder.to_string(); EXPECT_EQ(second.length_in_code_units(), 101u); EXPECT_EQ(second.code_unit_at(0), 0x000A); // Must be newline, not null. EXPECT_EQ(second.code_unit_at(1), 0x0020); diff --git a/UI/Qt/StringUtils.cpp b/UI/Qt/StringUtils.cpp index 98d1f20b65..34c704e172 100644 --- a/UI/Qt/StringUtils.cpp +++ b/UI/Qt/StringUtils.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include @@ -44,9 +45,9 @@ QString qstring_from_utf16_string(Utf16View const& string) QString vqformatted(StringView format, AK::TypeErasedFormatParams& parameters) { - StringBuilder builder(StringBuilder::Mode::UTF16); + Utf16StringBuilder builder; MUST(vformat(builder, format, parameters)); - return qstring_from_utf16_string(builder.utf16_string_view()); + return qstring_from_utf16_string(builder.view()); } QByteArray qbytearray_from_ak_string(StringView ak_string)