llvm
[LLDB] Add type summaries for MSVC STL strings
This PR adds type summaries for std::{string,wstring,u8string,u16string,u32string} from the MSVC STL.
See https://github.com/llvm/llvm-project/issues/24834 for the MSVC STL issue.
The following changes were made:
-
dotest.pynow detects if the MSVC STL is available. It does so by looking at the target triple, which is an additional argument passed from Lit. It specifically checks forwindows-msvcto not match onwindows-gnu(i.e. MinGW/Cygwin). - (The main part): Added support for summarizing
std::(w)stringfrom MSVC's STL. Because the type names from the libstdc++ (pre C++ 11) string types are the same as on MSVC's STL,CXXCompositeSummaryFormatis used with two entries, one for MSVC's STL and one for libstdc++. With MSVC's STL,std::u{8,16,32}stringis also handled. These aren't handled for libstdc++, so I put them inLoadMsvcStlFormatters.
:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
I would love some general feedback on this PR. I updated the description to reflect the current state.
For some reason, the Windows CI doesn't run on this PR, but I ran the STL tests locally.
@llvm/pr-subscribers-lldb
Author: nerix (Nerixyz)
Changes
This PR adds type summaries for std::{string,wstring,u8string,u16string,u32string} from the MSVC STL.
See https://github.com/llvm/llvm-project/issues/24834 for the MSVC STL issue.
The following changes were made:
- Added a new type summary kind
CXXCompositeSummaryFormat. It holds a list of child summaries paired with a validator function that checks if aValueObjectcan be formatted by a child. This feels like it should be done in another PR, but there aren't any tests for it other than the STL formatters (since it's only in C++ it can't be used in Python).- As part of this, the formatter data (callback/format string) from
CXXFunctionSummaryFormatandStringSummaryFormatwas extracted toCXXFunctionSummaryDataandStringSummaryDatarespectively. This allows the composite format to use the same data andFormatObjectof said data.
- As part of this, the formatter data (callback/format string) from
-
dotest.pynow detects if the MSVC STL is available. It does so by looking at the target triple, which is an additional argument passed from Lit. It specifically checks forwindows-msvcto not match onwindows-gnu(i.e. MinGW/Cygwin). - (The main part): Added support for summarizing
std::(w)stringfrom MSVC's STL. Because the type names from the libstdc++ (pre C++ 11) string types are the same as on MSVC's STL,CXXCompositeSummaryFormatis used with two entries, one for MSVC's STL and one for libstdc++. With MSVC's STL,std::u{8,16,32}stringis also handled. These aren't handled for libstdc++, so I put them inLoadMsvcStlFormatters.
Patch is 53.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143177.diff
21 Files Affected:
- (modified) lldb/include/lldb/DataFormatters/StringPrinter.h (+15)
- (modified) lldb/include/lldb/DataFormatters/TypeSummary.h (+91-11)
- (modified) lldb/include/lldb/DataFormatters/ValueObjectPrinter.h (+1-1)
- (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+2)
- (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+23)
- (modified) lldb/packages/Python/lldbsuite/test/dotest_args.py (+6)
- (modified) lldb/packages/Python/lldbsuite/test/test_categories.py (+1)
- (modified) lldb/source/API/SBTypeSummary.cpp (+10-1)
- (modified) lldb/source/Commands/CommandObjectType.cpp (+4-3)
- (modified) lldb/source/DataFormatters/TypeSummary.cpp (+112-31)
- (modified) lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt (+1)
- (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (+115-31)
- (added) lldb/source/Plugins/Language/CPlusPlus/MsvcStl.cpp (+140)
- (added) lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h (+33)
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/string/Makefile (+4)
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/string/TestDataFormatterStdString.py (+118)
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/string/main.cpp (+40)
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/u8string/Makefile (+4)
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/u8string/TestDataFormatterStdU8String.py (+31)
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/u8string/main.cpp (+14)
- (modified) lldb/test/API/lit.cfg.py (+3)
diff --git a/lldb/include/lldb/DataFormatters/StringPrinter.h b/lldb/include/lldb/DataFormatters/StringPrinter.h
index 4169f53e63f38..4ebe712be60e1 100644
--- a/lldb/include/lldb/DataFormatters/StringPrinter.h
+++ b/lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -152,6 +152,21 @@ class StringPrinter {
template <StringElementType element_type>
static bool
ReadBufferAndDumpToStream(const ReadBufferAndDumpToStreamOptions &options);
+
+ template <StringElementType element_type>
+ static constexpr uint64_t ElementByteSize() {
+ switch (element_type) {
+ case StringElementType::ASCII:
+ case StringElementType::UTF8:
+ return 1;
+ case StringElementType::UTF16:
+ return 2;
+ case StringElementType::UTF32:
+ return 3;
+ default:
+ return 0;
+ }
+ }
};
} // namespace formatters
diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h
index 589f68c2ce314..ea32b31c0b2ae 100644
--- a/lldb/include/lldb/DataFormatters/TypeSummary.h
+++ b/lldb/include/lldb/DataFormatters/TypeSummary.h
@@ -48,7 +48,14 @@ class TypeSummaryOptions {
class TypeSummaryImpl {
public:
- enum class Kind { eSummaryString, eScript, eBytecode, eCallback, eInternal };
+ enum class Kind {
+ eSummaryString,
+ eScript,
+ eBytecode,
+ eCallback,
+ eComposite,
+ eInternal
+ };
virtual ~TypeSummaryImpl() = default;
@@ -292,18 +299,29 @@ class TypeSummaryImpl {
const TypeSummaryImpl &operator=(const TypeSummaryImpl &) = delete;
};
-// simple string-based summaries, using ${var to show data
-struct StringSummaryFormat : public TypeSummaryImpl {
+struct StringSummaryData {
std::string m_format_str;
FormatEntity::Entry m_format;
Status m_error;
+ StringSummaryData(const char *f);
+ void SetFormat(const char *f);
+
+ bool FormatObject(ValueObject *valobj, std::string &dest,
+ const TypeSummaryOptions &options,
+ const TypeSummaryImpl &summary);
+};
+
+// simple string-based summaries, using ${var to show data
+struct StringSummaryFormat : public TypeSummaryImpl {
+ StringSummaryData m_data;
+
StringSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *f,
uint32_t ptr_match_depth = 1);
~StringSummaryFormat() override = default;
- const char *GetSummaryString() const { return m_format_str.c_str(); }
+ const char *GetSummaryString() const { return m_data.m_format_str.c_str(); }
void SetSummaryString(const char *f);
@@ -323,15 +341,23 @@ struct StringSummaryFormat : public TypeSummaryImpl {
const StringSummaryFormat &operator=(const StringSummaryFormat &) = delete;
};
-// summaries implemented via a C++ function
-struct CXXFunctionSummaryFormat : public TypeSummaryImpl {
+struct CXXFunctionSummaryData {
// we should convert these to SBValue and SBStream if we ever cross the
// boundary towards the external world
- typedef std::function<bool(ValueObject &, Stream &,
- const TypeSummaryOptions &)>
- Callback;
+ using Callback =
+ std::function<bool(ValueObject &, Stream &, const TypeSummaryOptions &)>;
Callback m_impl;
+
+ bool FormatObject(ValueObject *valobj, std::string &dest,
+ const TypeSummaryOptions &options,
+ const TypeSummaryImpl &summary);
+};
+
+// summaries implemented via a C++ function
+struct CXXFunctionSummaryFormat : public TypeSummaryImpl {
+ using Callback = CXXFunctionSummaryData::Callback;
+ CXXFunctionSummaryData m_data;
std::string m_description;
CXXFunctionSummaryFormat(const TypeSummaryImpl::Flags &flags, Callback impl,
@@ -340,11 +366,13 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl {
~CXXFunctionSummaryFormat() override = default;
- Callback GetBackendFunction() const { return m_impl; }
+ Callback GetBackendFunction() const { return m_data.m_impl; }
const char *GetTextualInfo() const { return m_description.c_str(); }
- void SetBackendFunction(Callback cb_func) { m_impl = std::move(cb_func); }
+ void SetBackendFunction(Callback cb_func) {
+ m_data.m_impl = std::move(cb_func);
+ }
void SetTextualInfo(const char *descr) {
if (descr)
@@ -372,6 +400,58 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl {
operator=(const CXXFunctionSummaryFormat &) = delete;
};
+/// A summary that supports multiple type layouts for the same type name.
+///
+/// This summary maintains a list of child summaries, each paired with a
+/// validator function. An associated validator is used to determine which child
+/// is appropriate for formatting a given ValueObject.
+struct CXXCompositeSummaryFormat : public TypeSummaryImpl {
+ using Validator = bool(ValueObject &valobj);
+ using ChildData = std::variant<CXXFunctionSummaryData, StringSummaryData>;
+ using Child = std::pair<Validator *, ChildData>;
+ using Children = llvm::SmallVector<Child, 2>;
+
+ Children m_children;
+ std::string m_description;
+
+ CXXCompositeSummaryFormat(const TypeSummaryImpl::Flags &flags,
+ const char *description, Children impls = {},
+ uint32_t ptr_match_depth = 1);
+
+ ~CXXCompositeSummaryFormat() override = default;
+
+ const char *GetTextualInfo() const { return m_description.c_str(); }
+
+ CXXCompositeSummaryFormat *Append(Validator *validator, ChildData impl);
+
+ void SetTextualInfo(const char *descr) {
+ if (descr)
+ m_description.assign(descr);
+ else
+ m_description.clear();
+ }
+
+ Children CopyChildren() const;
+
+ bool FormatObject(ValueObject *valobj, std::string &dest,
+ const TypeSummaryOptions &options) override;
+
+ std::string GetDescription() override;
+
+ static bool classof(const TypeSummaryImpl *S) {
+ return S->GetKind() == Kind::eComposite;
+ }
+
+ std::string GetName() override;
+
+ using SharedPointer = std::shared_ptr<CXXCompositeSummaryFormat>;
+
+private:
+ CXXCompositeSummaryFormat(const CXXCompositeSummaryFormat &) = delete;
+ const CXXCompositeSummaryFormat &
+ operator=(const CXXCompositeSummaryFormat &) = delete;
+};
+
// Python-based summaries, running script code to show data
struct ScriptSummaryFormat : public TypeSummaryImpl {
std::string m_function_name;
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index f9deb6ebf8d6d..1f20c3180d37a 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -165,7 +165,7 @@ class ValueObjectPrinter {
std::string m_error;
bool m_val_summary_ok;
- friend struct StringSummaryFormat;
+ friend struct StringSummaryData;
ValueObjectPrinter(const ValueObjectPrinter &) = delete;
const ValueObjectPrinter &operator=(const ValueObjectPrinter &) = delete;
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py
index b2d91fd211477..4ec892bef69f9 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -134,6 +134,8 @@
libcxx_include_target_dir = None
libcxx_library_dir = None
+target_triple = None
+
# A plugin whose tests will be enabled, like intel-pt.
enabled_plugins = []
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py
index d7f274ac4f60e..dd6fbdf8daed4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -299,6 +299,8 @@ def parseOptionsAndInitTestdirs():
configuration.libcxx_library_dir = args.libcxx_library_dir
configuration.cmake_build_type = args.cmake_build_type.lower()
+ configuration.target_triple = args.target_triple
+
if args.channels:
lldbtest_config.channels = args.channels
@@ -831,6 +833,26 @@ def checkLibstdcxxSupport():
configuration.skip_categories.append("libstdcxx")
+def canRunMsvcStlTests():
+ if "windows-msvc" in configuration.target_triple:
+ return True, "MSVC STL is present on *windows-msvc*"
+ return (
+ False,
+ f"Don't know how to build with MSVC's STL on {configuration.target_triple}",
+ )
+
+
+def checkMsvcStlSupport():
+ result, reason = canRunMsvcStlTests()
+ if result:
+ return # msvcstl supported
+ if "msvcstl" in configuration.categories_list:
+ return # msvcstl category explicitly requested, let it run.
+ if configuration.verbose:
+ print(f"msvcstl tests will not be run because: {reason}")
+ configuration.skip_categories.append("msvcstl")
+
+
def canRunWatchpointTests():
from lldbsuite.test import lldbplatformutil
@@ -1044,6 +1066,7 @@ def run_suite():
checkLibcxxSupport()
checkLibstdcxxSupport()
+ checkMsvcStlSupport()
checkWatchpointSupport()
checkDebugInfoSupport()
checkDebugServerSupport()
diff --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index e9c21388bc213..f47cfc7db357e 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -116,6 +116,12 @@ def create_parser():
"The location of llvm tools used for testing (yaml2obj, FileCheck, etc.)."
),
)
+ group.add_argument(
+ "--target-triple",
+ help=textwrap.dedent(
+ "The target triple the tests will run on (e.g. x86_64-pc-windows-msvc)."
+ ),
+ )
# Test filtering options
group = parser.add_argument_group("Test filtering options")
diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py b/lldb/packages/Python/lldbsuite/test/test_categories.py
index b585f695adeab..1f6e8a78e0c0d 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -33,6 +33,7 @@
"lldb-server": "Tests related to lldb-server",
"lldb-dap": "Tests for the Debug Adapter Protocol with lldb-dap",
"llgs": "Tests for the gdb-server functionality of lldb-server",
+ "msvcstl": "Test for MSVC STL data formatters",
"pexpect": "Tests requiring the pexpect library to be available",
"objc": "Tests related to the Objective-C programming language support",
"pyapi": "Tests related to the Python API",
diff --git a/lldb/source/API/SBTypeSummary.cpp b/lldb/source/API/SBTypeSummary.cpp
index 58ec068ab9600..b62e786f522d4 100644
--- a/lldb/source/API/SBTypeSummary.cpp
+++ b/lldb/source/API/SBTypeSummary.cpp
@@ -370,6 +370,9 @@ bool SBTypeSummary::IsEqualTo(lldb::SBTypeSummary &rhs) {
if (IsSummaryString() != rhs.IsSummaryString())
return false;
return GetOptions() == rhs.GetOptions();
+ case TypeSummaryImpl::Kind::eComposite:
+ return llvm::dyn_cast<CXXCompositeSummaryFormat>(m_opaque_sp.get()) ==
+ llvm::dyn_cast<CXXCompositeSummaryFormat>(rhs.m_opaque_sp.get());
case TypeSummaryImpl::Kind::eInternal:
return (m_opaque_sp.get() == rhs.m_opaque_sp.get());
}
@@ -406,7 +409,7 @@ bool SBTypeSummary::CopyOnWrite_Impl() {
if (CXXFunctionSummaryFormat *current_summary_ptr =
llvm::dyn_cast<CXXFunctionSummaryFormat>(m_opaque_sp.get())) {
new_sp = TypeSummaryImplSP(new CXXFunctionSummaryFormat(
- GetOptions(), current_summary_ptr->m_impl,
+ GetOptions(), current_summary_ptr->m_data.m_impl,
current_summary_ptr->m_description.c_str()));
} else if (ScriptSummaryFormat *current_summary_ptr =
llvm::dyn_cast<ScriptSummaryFormat>(m_opaque_sp.get())) {
@@ -417,6 +420,12 @@ bool SBTypeSummary::CopyOnWrite_Impl() {
llvm::dyn_cast<StringSummaryFormat>(m_opaque_sp.get())) {
new_sp = TypeSummaryImplSP(new StringSummaryFormat(
GetOptions(), current_summary_ptr->GetSummaryString()));
+ } else if (CXXCompositeSummaryFormat *current_summary_ptr =
+ llvm::dyn_cast<CXXCompositeSummaryFormat>(m_opaque_sp.get())) {
+ new_sp = TypeSummaryImplSP(new CXXCompositeSummaryFormat(
+ GetOptions(), current_summary_ptr->GetTextualInfo(),
+ current_summary_ptr->CopyChildren(),
+ current_summary_ptr->GetPtrMatchDepth()));
}
SetSP(new_sp);
diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index 19cd3ff2972e9..bd6138a860caa 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -1397,9 +1397,10 @@ bool CommandObjectTypeSummaryAdd::Execute_StringSummary(
result.AppendError("summary creation failed");
return false;
}
- if (string_format->m_error.Fail()) {
- result.AppendErrorWithFormat("syntax error: %s",
- string_format->m_error.AsCString("<unknown>"));
+ if (string_format->m_data.m_error.Fail()) {
+ result.AppendErrorWithFormat(
+ "syntax error: %s",
+ string_format->m_data.m_error.AsCString("<unknown>"));
return false;
}
lldb::TypeSummaryImplSP entry(string_format.release());
diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp
index 6aa290698cd12..4830b2c28901a 100644
--- a/lldb/source/DataFormatters/TypeSummary.cpp
+++ b/lldb/source/DataFormatters/TypeSummary.cpp
@@ -57,21 +57,17 @@ std::string TypeSummaryImpl::GetSummaryKindName() {
return "python";
case Kind::eInternal:
return "c++";
+ case Kind::eComposite:
+ return "composite";
case Kind::eBytecode:
return "bytecode";
}
llvm_unreachable("Unknown type kind name");
}
-StringSummaryFormat::StringSummaryFormat(const TypeSummaryImpl::Flags &flags,
- const char *format_cstr,
- uint32_t ptr_match_depth)
- : TypeSummaryImpl(Kind::eSummaryString, flags, ptr_match_depth),
- m_format_str() {
- SetSummaryString(format_cstr);
-}
+StringSummaryData::StringSummaryData(const char *f) { SetFormat(f); }
-void StringSummaryFormat::SetSummaryString(const char *format_cstr) {
+void StringSummaryData::SetFormat(const char *format_cstr) {
m_format.Clear();
if (format_cstr && format_cstr[0]) {
m_format_str = format_cstr;
@@ -82,8 +78,19 @@ void StringSummaryFormat::SetSummaryString(const char *format_cstr) {
}
}
-bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval,
- const TypeSummaryOptions &options) {
+StringSummaryFormat::StringSummaryFormat(const TypeSummaryImpl::Flags &flags,
+ const char *format_cstr,
+ uint32_t ptr_match_depth)
+ : TypeSummaryImpl(Kind::eSummaryString, flags, ptr_match_depth),
+ m_data(format_cstr) {}
+
+void StringSummaryFormat::SetSummaryString(const char *format_cstr) {
+ m_data.SetFormat(format_cstr);
+}
+
+bool StringSummaryData::FormatObject(ValueObject *valobj, std::string &retval,
+ const TypeSummaryOptions &options,
+ const TypeSummaryImpl &summary) {
if (!valobj) {
retval.assign("NULL ValueObject");
return false;
@@ -96,12 +103,12 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval,
if (frame)
sc = frame->GetSymbolContext(lldb::eSymbolContextEverything);
- if (IsOneLiner()) {
+ if (summary.IsOneLiner()) {
// We've already checked the case of a NULL valobj above. Let's put in an
// assert here to make sure someone doesn't take that out:
assert(valobj && "Must have a valid ValueObject to summarize");
ValueObjectPrinter printer(*valobj, &s, DumpValueObjectOptions());
- printer.PrintChildrenOneLiner(HideNames(valobj));
+ printer.PrintChildrenOneLiner(summary.HideNames(valobj));
retval = std::string(s.GetString());
return true;
} else {
@@ -117,40 +124,52 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval,
}
}
+bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval,
+ const TypeSummaryOptions &options) {
+ return m_data.FormatObject(valobj, retval, options, *this);
+}
+
std::string StringSummaryFormat::GetDescription() {
StreamString sstr;
- sstr.Printf("`%s`%s%s%s%s%s%s%s%s%s ptr-match-depth=%u", m_format_str.c_str(),
- m_error.Fail() ? " error: " : "",
- m_error.Fail() ? m_error.AsCString() : "",
- Cascades() ? "" : " (not cascading)",
- !DoesPrintChildren(nullptr) ? "" : " (show children)",
- !DoesPrintValue(nullptr) ? " (hide value)" : "",
- IsOneLiner() ? " (one-line printout)" : "",
- SkipsPointers() ? " (skip pointers)" : "",
- SkipsReferences() ? " (skip references)" : "",
- HideNames(nullptr) ? " (hide member names)" : "",
- GetPtrMatchDepth());
+ sstr.Printf(
+ "`%s`%s%s%s%s%s%s%s%s%s ptr-match-depth=%u", m_data.m_format_str.c_str(),
+ m_data.m_error.Fail() ? " error: " : "",
+ m_data.m_error.Fail() ? m_data.m_error.AsCString() : "",
+ Cascades() ? "" : " (not cascading)",
+ !DoesPrintChildren(nullptr) ? "" : " (show children)",
+ !DoesPrintValue(nullptr) ? " (hide value)" : "",
+ IsOneLiner() ? " (one-line printout)" : "",
+ SkipsPointers() ? " (skip pointers)" : "",
+ SkipsReferences() ? " (skip references)" : "",
+ HideNames(nullptr) ? " (hide member names)" : "", GetPtrMatchDepth());
return std::string(sstr.GetString());
}
-std::string StringSummaryFormat::GetName() { return m_format_str; }
+std::string StringSummaryFormat::GetName() { return m_data.m_format_str; }
+
+bool CXXFunctionSummaryData::FormatObject(ValueObject *valobj,
+ std::string &dest,
+ const TypeSummaryOptions &options,
+ const TypeSummaryImpl &summary) {
+ dest.clear();
+ StreamString stream;
+ if (!m_impl || !m_impl(*valobj, stream, options))
+ return false;
+ dest = std::string(stream.GetString());
+ return true;
+}
CXXFunctionSummaryFormat::CXXFunctionSummaryFormat(
const TypeSummaryImpl::Flags &flags, Callback impl, const char *description,
uint32_t ptr_match_depth)
- : TypeSummaryImpl(Kind::eCallback, flags, ptr_match_depth), m_impl(impl),
+ : TypeSummaryImpl(Kind::eCallback, flags, ptr_match_depth), m_data{impl},
m_description(description ? description : "") {}
bool CXXFunctionSummaryFormat::FormatObject(ValueObject *valobj,
std::string &dest,
const TypeSummaryOptions &options) {
- dest.clear();
- StreamString stream;
- if (!m_impl || !m_impl(*valobj, stream, options))
- return false;
- dest = std::string(stream.GetString());
- return true;
+ return m_data.FormatObject(valobj, dest, options, *this);
}
std::string CXXFunctionSummaryFormat::GetDescription() {
@@ -169,6 +188,68 @@ std::string CXXFunctionSummaryFormat::GetDescription() {
std::string CXXFunctionSummaryFormat::GetName() { return m_description; }
+CXXCompositeSummaryFormat::CXXCompositeSummaryFormat(
+ const TypeSummaryImpl::Flags &flags, const char *description,
+ Children children, uint32_t ptr_match_depth)
+ : TypeSummaryImpl(Kind::eComposite, flags, ptr_match_depth),
+ m_children(std::move(children)) {}
+
+CXXCompositeSummaryFormat *
+CXXCompositeSummaryFormat::Append(Validator *validator, ChildData child) {
+ m_children.emplace_back(validator, std::move(child));
+ return this;
+}
+
+CXXCompositeSummaryFormat::Children
+CXXCompositeSummaryFormat::CopyChildren() const {
+ auto copy = [](const Child &entry) -> Child {
+ return {entry.first,
+ std::visit(llvm::makeVisitor(
+ [](const CXXFunctionSummaryData &fn) -> ChildData {
+ return fn;
+ },
+ [](const StringSummaryData &string) -> ChildData {
+ return StringS...
[truncated]
Added a new type summary kind CXXCompositeSummaryFormat. It holds a list of child summaries paired with a validator function that checks if a ValueObject can be formatted by a child. This feels like it should be done in another PR, but there aren't any tests for it other than the STL formatters (since it's only in C++ it can't be used in Python).
I had a similar problem with another formatting option, so I added https://github.com/llvm/llvm-project/blob/main/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp.
Maybe you could extend or take inspiration from that.
The composite formats are not an unreasonable idea, but I can't help but wonder if we even need to touch the generic data formatter infrastructure. I mean we already have the ability to do callback-based summaries, so what if we just did the stl switch inside the callback? Being able to do this declaratively is sort of nice, but this already isn't completely declarative since you have callbacks for type validation.
Maybe this means that we can't specify the summary for std::string via summary strings (or maybe we can -- I haven't checked how the code is structured), but I don't think that's such a disaster. Implementing ${var._M_dataplus._M_p} in c++ is not that hard, and this is probably one of our more complex summaries (containers just say size=#children), which doesn't even need to be customized for different STLs.
The composite formats are not an _un_reasonable idea, but I can't help but wonder if we even need to touch the generic data formatter infrastructure. I mean we already have the ability to do callback-based summaries, so what if we just did the stl switch inside the callback? Being able to do this declaratively is sort of nice, but this already isn't completely declarative since you have callbacks for type validation.
That's a good point. I removed the CXXCompositeSummaryFormat and implemented the selection in a lambda.
Implementing
${var._M_dataplus._M_p}in c++ is not that hard, and this is probably one of our more complex summaries (containers just saysize=#children), which doesn't even need to be customized for different STLs.
Right, I did that at the beginning, but then saw more types using string summaries without looking at what they're doing, so I wanted to support both.
Without the new summary type this PR is much more manageable.
Another aside: Does anyone know why the libstdc++ std::__cxx11::string summaries weren't using the string summary? They were initially added in D13964, but it doesn't provide a reason to do so. Note that the summary for a std::string v = "foo" was the format string one (${var._M_dataplus._M_p}). Only when printing a reference or pointer to a string, LibStdcppStringSummaryProvider was called. I tried to use this function for all libstdc++ strings, but this broke when using -D_GLIBCXX_USE_CXX11_ABI=0 (on MinGW). On the other hand, ${var._M_dataplus._M_p} works with both ABIs.
Another aside: Does anyone know why the libstdc++
std::__cxx11::stringsummaries weren't using the string summary? They were initially added in D13964, but it doesn't provide a reason to do so. Note that the summary for astd::string v = "foo"was the format string one (${var._M_dataplus._M_p}). Only when printing a reference or pointer to a string,LibStdcppStringSummaryProviderwas called. I tried to use this function for all libstdc++ strings, but this broke when using-D_GLIBCXX_USE_CXX11_ABI=0(on MinGW). On the other hand,${var._M_dataplus._M_p}works with both ABIs.
Well, the commit says (I haven't verified this) that the std::string formatter at the time was written in python, so this could have been a way to avoid the python dependency. As for the implementation, a possible reason (again, I don't know if that's the case) could be to make the formatter work in the case where you don't have debug info for std::string -- it looks like it avoids looking at any type information, which means it could work even with a forward declaration -- but that it would break when the ABI changes, as you've noticed.
I don't think we need to concern ourselves with that though. Your new implementation is fine.
:white_check_mark: With the latest revision this PR passed the Python code formatter.
LLVM Buildbot has detected a new failure on builder lldb-x86_64-win running on as-builder-10 while building lldb at step 8 "test-check-lldb-unit".
Full details are available at: https://lab.llvm.org/buildbot/#/builders/211/builds/321
Here is the relevant piece of the build log for the reference
Step 8 (test-check-lldb-unit) failure: Test just built components: check-lldb-unit completed (failure)
******************** TEST 'lldb-unit :: Host/./HostTests.exe/31/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\unittests\Host\.\HostTests.exe-lldb-unit-4664-31-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=31 C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\unittests\Host\.\HostTests.exe
--
Script:
--
C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\unittests\Host\.\HostTests.exe --gtest_filter=MainLoopTest.NoSpuriousPipeReads
--
C:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\unittests\Host\MainLoopTest.cpp(141): error: Expected equality of these values:
1u
Which is: 1
callback_count
Which is: 2
C:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\unittests\Host\MainLoopTest.cpp:141
Expected equality of these values:
1u
Which is: 1
callback_count
Which is: 2
********************
