[Lldb-commits] [lldb] [lldb-vscode] Display a more descriptive summary for containers and pointers (PR #65514)
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 6 11:43:13 PDT 2023
https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/65514:
We've been displaying types and addresses for containers, but that's not very useful information. A better approach is to compose the summary of containers with the summary of a few of its children.
Not only that, we can dereference simple pointers and references to get the summary of the pointer variable, which is also better than just showing an anddress.
And in the rare case where the user wants to inspect the raw address, they can always use the debug console for that.
>From e7ed33687a44992889df60eae5af03fb439aacc0 Mon Sep 17 00:00:00 2001
From: walter erquinigo <walter at modular.com>
Date: Wed, 6 Sep 2023 10:38:41 -0400
Subject: [PATCH] [lldb-vscode] Display a more descriptive summary for
containers and pointers
We've been displaying types and addresses for containers, but that's not very useful information. A better approach is to compose the summary of containers with the summary of a few of its children.
Not only that, we can dereference simple pointers and references to get the summary of the pointer variable, which is also better than just showing an anddress.
And in the rare case where the user wants to inspect the raw address, they can always use the debug console for that.
---
.../tools/lldb-vscode/lldbvscode_testcase.py | 7 +-
.../evaluate/TestVSCode_evaluate.py | 4 +-
.../variables/TestVSCode_variables.py | 13 +-
lldb/tools/lldb-vscode/JSONUtils.cpp | 128 ++++++++++++++++--
4 files changed, 127 insertions(+), 25 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index a6c370ccd2036d..00534752eb2719 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -1,8 +1,9 @@
-from lldbsuite.test.lldbtest import *
import os
-import vscode
import time
+import vscode
+from lldbsuite.test.lldbtest import *
+
class VSCodeTestCaseBase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
@@ -267,7 +268,7 @@ def disassemble(self, threadId=None, frameIndex=None):
if memoryReference not in self.vscode.disassembled_instructions:
self.vscode.request_disassemble(memoryReference=memoryReference)
-
+
return self.vscode.disassembled_instructions[memoryReference]
def attach(
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
index fee685435c1952..85bd23d9abfdef 100644
--- a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
@@ -55,7 +55,7 @@ def run_test_evaluate_expressions(self, context=None):
self.assertEvaluate("var2", "21")
self.assertEvaluate("static_int", "42")
self.assertEvaluate("non_static_int", "43")
- self.assertEvaluate("struct1", "my_struct @ 0x.*")
+ self.assertEvaluate("struct1", "{foo:15}")
self.assertEvaluate("struct1.foo", "15")
self.assertEvaluate("struct2->foo", "16")
@@ -85,7 +85,7 @@ def run_test_evaluate_expressions(self, context=None):
self.assertEvaluate(
"non_static_int", "10"
) # different variable with the same name
- self.assertEvaluate("struct1", "my_struct @ 0x.*")
+ self.assertEvaluate("struct1", "{foo:15}")
self.assertEvaluate("struct1.foo", "15")
self.assertEvaluate("struct2->foo", "16")
diff --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index 88bf58e9155ad2..6eb8e87190d007 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -2,12 +2,13 @@
Test lldb-vscode setBreakpoints request
"""
+import os
+
+import lldbvscode_testcase
import vscode
+from lldbsuite.test import lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-import lldbvscode_testcase
-import os
def make_buffer_verify_dict(start_idx, count, offset=0):
@@ -218,12 +219,12 @@ def test_scopes_variables_setVariable_evaluate(self):
},
"pt": {
"equals": {"type": "PointType"},
- "startswith": {"result": "PointType @ 0x"},
+ "startswith": {"result": "{x:11, y:22}"},
"hasVariablesReference": True,
},
"pt.buffer": {
"equals": {"type": "int[32]"},
- "startswith": {"result": "int[32] @ 0x"},
+ "startswith": {"result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"},
"hasVariablesReference": True,
},
"argv": {
@@ -409,7 +410,7 @@ def test_scopes_and_evaluate_expansion(self):
"name": "pt",
"response": {
"equals": {"type": "PointType"},
- "startswith": {"result": "PointType @ 0x"},
+ "startswith": {"result": "{x:11, y:22}"},
"missing": ["indexedVariables"],
"hasVariablesReference": True,
},
diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 03826f8936ae32..8640547734e8f8 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -132,6 +132,84 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
return strs;
}
+/// Create a short summary for a container that contains the summary of its
+/// first children, so that the user can get a glimpse of its contents at a
+/// glance.
+static std::optional<std::string>
+GetSyntheticSummaryForContainer(lldb::SBValue &v) {
+ if (v.TypeIsPointerType() || !v.MightHaveChildren())
+ return std::nullopt;
+ /// As this operation can be potentially slow, we limit the total time spent
+ /// fetching children to a few ms.
+ const auto max_evaluation_time = std::chrono::milliseconds(10);
+ /// We don't want to generate a extremely long summary string, so we limit its
+ /// length.
+ const size_t max_length = 32;
+
+ auto start = std::chrono::steady_clock::now();
+ std::string summary;
+ llvm::raw_string_ostream os(summary);
+ os << "{";
+
+ llvm::StringRef separator = "";
+
+ for (size_t i = 0, e = v.GetNumChildren(); i < e; ++i) {
+ // If we reached the time limit or exceeded the number of characters, we
+ // dump `...` to signal that there are more elements in the collection.
+ if (summary.size() > max_length ||
+ (std::chrono::steady_clock::now() - start) > max_evaluation_time) {
+ os << separator << "...";
+ break;
+ }
+ lldb::SBValue child = v.GetChildAtIndex(i);
+
+ if (llvm::StringRef name = child.GetName(); !name.empty()) {
+ llvm::StringRef value;
+ if (llvm::StringRef summary = child.GetSummary(); !summary.empty())
+ value = summary;
+ else
+ value = child.GetValue();
+
+ if (!value.empty()) {
+ // If the child is an indexed entry, we don't show its index to save
+ // characters.
+ if (name.starts_with("["))
+ os << separator << value;
+ else
+ os << separator << name << ":" << value;
+ separator = ", ";
+ }
+ }
+ }
+ os << "}";
+
+ if (summary == "{...}" || summary == "{}")
+ return std::nullopt;
+ return summary;
+}
+
+/// Return whether we should dereference an SBValue in order to generate a more
+/// meaningful summary string.
+static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
+ if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
+ return false;
+
+ // If it's a pointer to a pointer, we don't dereference to avoid confusing
+ // the user with the addresses that could shown in the summary.
+ if (v.GetType().IsPointerType() &&
+ v.GetType().GetDereferencedType().IsPointerType())
+ return false;
+
+ // If it's synthetic or a pointer to a basic type that provides a summary, we
+ // don't dereference.
+ if ((v.IsSynthetic() || v.GetType().GetPointeeType().GetBasicType() !=
+ lldb::eBasicTypeInvalid) &&
+ !llvm::StringRef(v.GetSummary()).empty()) {
+ return false;
+ }
+ return true;
+}
+
void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
llvm::StringRef key) {
std::string result;
@@ -141,23 +219,45 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
if (!error.Success()) {
strm << "<error: " << error.GetCString() << ">";
} else {
- llvm::StringRef value = v.GetValue();
- llvm::StringRef summary = v.GetSummary();
- llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
- if (!value.empty()) {
- strm << value;
- if (!summary.empty())
+ auto tryDumpSummaryAndValue = [&strm](lldb::SBValue value) {
+ llvm::StringRef val = value.GetValue();
+ llvm::StringRef summary = value.GetSummary();
+ if (!val.empty()) {
+ strm << val;
+ if (!summary.empty())
+ strm << ' ' << summary;
+ return true;
+ }
+ if (!summary.empty()) {
strm << ' ' << summary;
- } else if (!summary.empty()) {
- strm << ' ' << summary;
- } else if (!type_name.empty()) {
- strm << type_name;
- lldb::addr_t address = v.GetLoadAddress();
- if (address != LLDB_INVALID_ADDRESS)
- strm << " @ " << llvm::format_hex(address, 0);
+ return true;
+ }
+ if (auto container_summary = GetSyntheticSummaryForContainer(value)) {
+ strm << *container_summary;
+ return true;
+ }
+ return false;
+ };
+
+ // We first try to get the summary from its dereferenced value.
+ bool done = ShouldBeDereferencedForSummary(v) &&
+ tryDumpSummaryAndValue(v.Dereference());
+
+ // We then try to get it from the current value itself.
+ if (!done)
+ done = tryDumpSummaryAndValue(v);
+
+ // As last resort, we print its type and address if available.
+ if (!done) {
+ if (llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+ !type_name.empty()) {
+ strm << type_name;
+ lldb::addr_t address = v.GetLoadAddress();
+ if (address != LLDB_INVALID_ADDRESS)
+ strm << " @ " << llvm::format_hex(address, 0);
+ }
}
}
- strm.flush();
EmplaceSafeString(object, key, result);
}
More information about the lldb-commits
mailing list