https://github.com/Nerixyz created https://github.com/llvm/llvm-project/pull/144258
As part of https://github.com/llvm/llvm-project/pull/143177, I moved the non-libc++ specific formatting of `std::string`s out to `CxxStringTypes` as MSVC's STL `std::string` can also be thought of a pointer+size pair. I named this kind of string "string buffer". This PR picks that change, so the MSVC PR can be smaller. Unfortunately, libstdc++'s `std::string` does not fit this (it also uses a different string printer function). This resolves two FIXMEs in the libc++ tests, where empty u16 and u32 strings didn't have any prefix (u/U). >From 1d74dd1c8d8d768b7ddc22a368be164450362e5e Mon Sep 17 00:00:00 2001 From: Nerixyz <nerix...@outlook.de> Date: Sun, 15 Jun 2025 12:23:27 +0200 Subject: [PATCH] [LLDB] Consolidate C++ string buffer summaries --- .../Language/CPlusPlus/CxxStringTypes.cpp | 105 ++++++++++--- .../Language/CPlusPlus/CxxStringTypes.h | 12 ++ .../Plugins/Language/CPlusPlus/LibCxx.cpp | 147 ++++-------------- .../string/TestDataFormatterLibcxxString.py | 8 +- .../TestDataFormatterLibcxxStringView.py | 8 +- 5 files changed, 134 insertions(+), 146 deletions(-) diff --git a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp index fc17b76804d9f..4e7569cd8a388 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp @@ -116,15 +116,7 @@ bool lldb_private::formatters::WCharStringSummaryProvider( return false; // Get a wchar_t basic type from the current type system - CompilerType wchar_compiler_type = - valobj.GetCompilerType().GetBasicTypeFromAST(lldb::eBasicTypeWChar); - - if (!wchar_compiler_type) - return false; - - // Safe to pass nullptr for exe_scope here. - std::optional<uint64_t> size = - llvm::expectedToOptional(wchar_compiler_type.GetBitSize(nullptr)); + std::optional<uint64_t> size = GetWCharByteSize(*valobj.GetTargetSP()); if (!size) return false; const uint32_t wchar_size = *size; @@ -136,13 +128,13 @@ bool lldb_private::formatters::WCharStringSummaryProvider( options.SetPrefixToken("L"); switch (wchar_size) { - case 8: + case 1: return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF8>( options); - case 16: + case 2: return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF16>( options); - case 32: + case 4: return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF32>( options); default: @@ -177,15 +169,7 @@ bool lldb_private::formatters::WCharSummaryProvider( return false; // Get a wchar_t basic type from the current type system - CompilerType wchar_compiler_type = - valobj.GetCompilerType().GetBasicTypeFromAST(lldb::eBasicTypeWChar); - - if (!wchar_compiler_type) - return false; - - // Safe to pass nullptr for exe_scope here. - std::optional<uint64_t> size = - llvm::expectedToOptional(wchar_compiler_type.GetBitSize(nullptr)); + std::optional<uint64_t> size = GetWCharByteSize(*valobj.GetTargetSP()); if (!size) return false; const uint32_t wchar_size = *size; @@ -199,13 +183,13 @@ bool lldb_private::formatters::WCharSummaryProvider( options.SetBinaryZeroIsTerminator(false); switch (wchar_size) { - case 8: + case 1: return StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF8>( options); - case 16: + case 2: return StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF16>( options); - case 32: + case 4: return StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF32>( options); default: @@ -214,3 +198,76 @@ bool lldb_private::formatters::WCharSummaryProvider( } return true; } + +std::optional<uint64_t> +lldb_private::formatters::GetWCharByteSize(Target &target) { + TypeSystemClangSP scratch_ts_sp = + ScratchTypeSystemClang::GetForTarget(target); + if (!scratch_ts_sp) + return {}; + + return llvm::expectedToOptional( + scratch_ts_sp->GetBasicType(lldb::eBasicTypeWChar).GetByteSize(nullptr)); +} + +template <StringPrinter::StringElementType element_type> +bool lldb_private::formatters::StringBufferSummaryProvider( + Stream &stream, const TypeSummaryOptions &summary_options, + lldb::ValueObjectSP location_sp, uint64_t size, std::string prefix_token) { + + if (size == 0) { + stream.PutCString(prefix_token); + stream.PutCString("\"\""); + return true; + } + + if (!location_sp) + return false; + + StringPrinter::ReadBufferAndDumpToStreamOptions options(*location_sp); + + if (summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryCapped) { + const auto max_size = + location_sp->GetTargetSP()->GetMaximumSizeOfStringSummary(); + if (size > max_size) { + size = max_size; + options.SetIsTruncated(true); + } + } + + { + DataExtractor extractor; + const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size); + if (bytes_read < size) + return false; + + options.SetData(std::move(extractor)); + } + options.SetStream(&stream); + if (prefix_token.empty()) + options.SetPrefixToken(nullptr); + else + options.SetPrefixToken(prefix_token); + options.SetQuote('"'); + options.SetSourceSize(size); + options.SetBinaryZeroIsTerminator(false); + return StringPrinter::ReadBufferAndDumpToStream<element_type>(options); +} + +// explicit instantiations for all string element types +template bool +lldb_private::formatters::StringBufferSummaryProvider<StringElementType::ASCII>( + Stream &, const TypeSummaryOptions &, lldb::ValueObjectSP, uint64_t, + std::string); +template bool +lldb_private::formatters::StringBufferSummaryProvider<StringElementType::UTF8>( + Stream &, const TypeSummaryOptions &, lldb::ValueObjectSP, uint64_t, + std::string); +template bool +lldb_private::formatters::StringBufferSummaryProvider<StringElementType::UTF16>( + Stream &, const TypeSummaryOptions &, lldb::ValueObjectSP, uint64_t, + std::string); +template bool +lldb_private::formatters::StringBufferSummaryProvider<StringElementType::UTF32>( + Stream &, const TypeSummaryOptions &, lldb::ValueObjectSP, uint64_t, + std::string); diff --git a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.h b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.h index a2b606d28cac1..b596950a28b18 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.h +++ b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.h @@ -10,6 +10,7 @@ #ifndef LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_CXXSTRINGTYPES_H #define LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_CXXSTRINGTYPES_H +#include "lldb/DataFormatters/StringPrinter.h" #include "lldb/DataFormatters/TypeSummary.h" #include "lldb/Utility/Stream.h" #include "lldb/ValueObject/ValueObject.h" @@ -43,6 +44,17 @@ bool Char32SummaryProvider(ValueObject &valobj, Stream &stream, bool WCharSummaryProvider(ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options); // wchar_t +std::optional<uint64_t> GetWCharByteSize(Target &target); + +/// Print a summary for a string buffer to \a stream. +/// +/// The buffer consists of a data pointer, \a location_sp, and a known \a size. +template <StringPrinter::StringElementType element_type> +bool StringBufferSummaryProvider(Stream &stream, + const TypeSummaryOptions &summary_options, + lldb::ValueObjectSP location_sp, uint64_t size, + std::string prefix_token); + } // namespace formatters } // namespace lldb_private diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index 358cf7d78fa21..4fe6626a3cf8a 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -24,6 +24,7 @@ #include "lldb/ValueObject/ValueObject.h" #include "lldb/ValueObject/ValueObjectConstResult.h" +#include "Plugins/Language/CPlusPlus/CxxStringTypes.h" #include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h" #include "Plugins/TypeSystem/Clang/TypeSystemClang.h" #include "lldb/lldb-enumerations.h" @@ -535,70 +536,6 @@ ExtractLibcxxStringInfo(ValueObject &valobj) { return std::make_pair(size, location_sp); } -static bool -LibcxxWStringSummaryProvider(ValueObject &valobj, Stream &stream, - const TypeSummaryOptions &summary_options, - ValueObjectSP location_sp, size_t size) { - if (size == 0) { - stream.Printf("L\"\""); - return true; - } - if (!location_sp) - return false; - - StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj); - if (summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryCapped) { - const auto max_size = valobj.GetTargetSP()->GetMaximumSizeOfStringSummary(); - if (size > max_size) { - size = max_size; - options.SetIsTruncated(true); - } - } - - DataExtractor extractor; - const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size); - if (bytes_read < size) - return false; - - // std::wstring::size() is measured in 'characters', not bytes - TypeSystemClangSP scratch_ts_sp = - ScratchTypeSystemClang::GetForTarget(*valobj.GetTargetSP()); - if (!scratch_ts_sp) - return false; - - auto wchar_t_size = - scratch_ts_sp->GetBasicType(lldb::eBasicTypeWChar).GetByteSize(nullptr); - if (!wchar_t_size) - return false; - - options.SetData(std::move(extractor)); - options.SetStream(&stream); - options.SetPrefixToken("L"); - options.SetQuote('"'); - options.SetSourceSize(size); - options.SetBinaryZeroIsTerminator(false); - - switch (*wchar_t_size) { - case 1: - return StringPrinter::ReadBufferAndDumpToStream< - lldb_private::formatters::StringPrinter::StringElementType::UTF8>( - options); - break; - - case 2: - return StringPrinter::ReadBufferAndDumpToStream< - lldb_private::formatters::StringPrinter::StringElementType::UTF16>( - options); - break; - - case 4: - return StringPrinter::ReadBufferAndDumpToStream< - lldb_private::formatters::StringPrinter::StringElementType::UTF32>( - options); - } - return false; -} - bool lldb_private::formatters::LibcxxWStringSummaryProvider( ValueObject &valobj, Stream &stream, const TypeSummaryOptions &summary_options) { @@ -609,52 +546,22 @@ bool lldb_private::formatters::LibcxxWStringSummaryProvider( ValueObjectSP location_sp; std::tie(size, location_sp) = *string_info; - return ::LibcxxWStringSummaryProvider(valobj, stream, summary_options, - location_sp, size); -} - -template <StringPrinter::StringElementType element_type> -static bool -LibcxxStringSummaryProvider(ValueObject &valobj, Stream &stream, - const TypeSummaryOptions &summary_options, - std::string prefix_token, ValueObjectSP location_sp, - uint64_t size) { - - if (size == 0) { - stream.Printf("\"\""); - return true; - } - - if (!location_sp) + auto wchar_t_size = GetWCharByteSize(*valobj.GetTargetSP()); + if (!wchar_t_size) return false; - StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj); - - if (summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryCapped) { - const auto max_size = valobj.GetTargetSP()->GetMaximumSizeOfStringSummary(); - if (size > max_size) { - size = max_size; - options.SetIsTruncated(true); - } - } - - { - DataExtractor extractor; - const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size); - if (bytes_read < size) - return false; - - options.SetData(std::move(extractor)); + switch (*wchar_t_size) { + case 1: + return StringBufferSummaryProvider<StringPrinter::StringElementType::UTF8>( + stream, summary_options, location_sp, size, "L"); + case 2: + return StringBufferSummaryProvider<StringPrinter::StringElementType::UTF16>( + stream, summary_options, location_sp, size, "L"); + case 4: + return StringBufferSummaryProvider<StringPrinter::StringElementType::UTF32>( + stream, summary_options, location_sp, size, "L"); } - options.SetStream(&stream); - if (prefix_token.empty()) - options.SetPrefixToken(nullptr); - else - options.SetPrefixToken(prefix_token); - options.SetQuote('"'); - options.SetSourceSize(size); - options.SetBinaryZeroIsTerminator(false); - return StringPrinter::ReadBufferAndDumpToStream<element_type>(options); + return false; } template <StringPrinter::StringElementType element_type> @@ -669,8 +576,8 @@ LibcxxStringSummaryProvider(ValueObject &valobj, Stream &stream, ValueObjectSP location_sp; std::tie(size, location_sp) = *string_info; - return LibcxxStringSummaryProvider<element_type>( - valobj, stream, summary_options, prefix_token, location_sp, size); + return StringBufferSummaryProvider<element_type>( + stream, summary_options, location_sp, size, prefix_token); } template <StringPrinter::StringElementType element_type> static bool formatStringImpl(ValueObject &valobj, Stream &stream, @@ -742,8 +649,8 @@ static bool formatStringViewImpl(ValueObject &valobj, Stream &stream, return true; } - return LibcxxStringSummaryProvider<element_type>( - valobj, stream, summary_options, prefix_token, dataobj, size); + return StringBufferSummaryProvider<element_type>(stream, summary_options, + dataobj, size, prefix_token); } bool lldb_private::formatters::LibcxxStringViewSummaryProviderASCII( @@ -781,8 +688,22 @@ bool lldb_private::formatters::LibcxxWStringViewSummaryProvider( return true; } - return ::LibcxxWStringSummaryProvider(valobj, stream, summary_options, - dataobj, size); + auto wchar_t_size = GetWCharByteSize(*valobj.GetTargetSP()); + if (!wchar_t_size) + return false; + + switch (*wchar_t_size) { + case 1: + return StringBufferSummaryProvider<StringPrinter::StringElementType::UTF8>( + stream, summary_options, dataobj, size, "L"); + case 2: + return StringBufferSummaryProvider<StringPrinter::StringElementType::UTF16>( + stream, summary_options, dataobj, size, "L"); + case 4: + return StringBufferSummaryProvider<StringPrinter::StringElementType::UTF32>( + stream, summary_options, dataobj, size, "L"); + } + return false; } static bool diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py index 5c5cf4ca16b98..32764629d65a7 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py @@ -65,11 +65,9 @@ def cleanup(): '(%s::wstring) IHaveEmbeddedZerosToo = L"hello world!\\0てざ ル゜䋨ミ㠧槊 きゅへ狦穤襩 じゃ馩リョ 䤦監"' % ns, '(%s::u16string) u16_string = u"ß水氶"' % ns, - # FIXME: This should have a 'u' prefix. - '(%s::u16string) u16_empty = ""' % ns, + '(%s::u16string) u16_empty = u""' % ns, '(%s::u32string) u32_string = U"🍄🍅🍆🍌"' % ns, - # FIXME: This should have a 'U' prefix. - '(%s::u32string) u32_empty = ""' % ns, + '(%s::u32string) u32_empty = U""' % ns, "(%s::string *) null_str = nullptr" % ns, ], ) @@ -123,7 +121,7 @@ def cleanup(): % ns, '(%s::u16string) u16_string = u"ß水氶"' % ns, '(%s::u32string) u32_string = U"🍄🍅🍆🍌"' % ns, - '(%s::u32string) u32_empty = ""' % ns, + '(%s::u32string) u32_empty = U""' % ns, "(%s::string *) null_str = nullptr" % ns, ], ) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py index f8fc8ae66405b..3883395f23924 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py @@ -81,11 +81,11 @@ def cleanup(): summary='L"hello world!\\0てざ ル゜䋨ミ㠧槊 きゅへ狦穤襩 じゃ馩リョ 䤦監"', ) self.expect_var_path("u16_string", type="std::u16string_view", summary='u"ß水氶"') - self.expect_var_path("u16_empty", type="std::u16string_view", summary='""') + self.expect_var_path("u16_empty", type="std::u16string_view", summary='u""') self.expect_var_path( "u32_string", type="std::u32string_view", summary='U"🍄🍅🍆🍌"' ) - self.expect_var_path("u32_empty", type="std::u32string_view", summary='""') + self.expect_var_path("u32_empty", type="std::u32string_view", summary='U""') self.expect_var_path( "oops", type="std::string_view", summary='"Hellooo World\\n"' ) @@ -145,11 +145,11 @@ def cleanup(): summary='L"hello world!\\0てざ ル゜䋨ミ㠧槊 きゅへ狦穤襩 じゃ馩リョ 䤦監"', ) self.expect_var_path("u16_string", type="std::u16string_view", summary='u"ß水氶"') - self.expect_var_path("u16_empty", type="std::u16string_view", summary='""') + self.expect_var_path("u16_empty", type="std::u16string_view", summary='u""') self.expect_var_path( "u32_string", type="std::u32string_view", summary='U"🍄🍅🍆🍌"' ) - self.expect_var_path("u32_empty", type="std::u32string_view", summary='""') + self.expect_var_path("u32_empty", type="std::u32string_view", summary='U""') self.runCmd("cont") self.expect( _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
