Skip to content

Commit

Permalink
[lldb-vscode] Use auto summaries whenever variables don't have a summ…
Browse files Browse the repository at this point in the history
…ary (#66551)

Auto summaries were only being used when non-pointer/reference variables
didn't have values nor summaries. Greg pointed out that it should be
better to simply use auto summaries when the variable doesn't have a
summary of its own, regardless of other conditions.

This led to code simplification and correct visualization of auto
summaries for pointer/reference types, as seen in this screenshot.

<img width="310" alt="Screenshot 2023-09-19 at 7 04 55 PM"
src="https://github.com/llvm/llvm-project/assets/1613874/d356d579-13f2-487b-ae3a-f3443dce778f">
  • Loading branch information
walter-erquinigo authored Sep 20, 2023
1 parent feb5c57 commit 96b1784
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ def run_test_evaluate_expressions(
"struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x"
)
self.assertEvaluate(
"struct2", "{foo:16}" if enableAutoVariableSummaries else "0x.*"
"struct2", "0x.* {foo:16}" if enableAutoVariableSummaries else "0x.*"
)
self.assertEvaluate("struct3", "0x.*0")
self.assertEvaluate("struct1.foo", "15")
self.assertEvaluate("struct2->foo", "16")

Expand Down
3 changes: 2 additions & 1 deletion lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct my_struct {
int main(int argc, char const *argv[]) {
my_struct struct1 = {15};
my_struct *struct2 = new my_struct{16};
my_struct *struct3 = nullptr;
int var1 = 20;
int var2 = 21;
int var3 = static_int; // breakpoint 1
Expand All @@ -43,6 +44,6 @@ int main(int argc, char const *argv[]) {
my_bool_vec.push_back(true);
my_bool_vec.push_back(false); // breakpoint 6
my_bool_vec.push_back(true); // breakpoint 7

return 0;
}
80 changes: 27 additions & 53 deletions lldb/tools/lldb-vscode/JSONUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
/// 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) {
TryCreateAutoSummaryForContainer(lldb::SBValue &v) {
// We gate this feature because it performs GetNumChildren(), which can
// cause performance issues because LLDB needs to complete possibly huge
// types.
if (!g_vsc.enable_auto_variable_summaries)
return std::nullopt;

if (v.TypeIsPointerType() || !v.MightHaveChildren())
if (!v.MightHaveChildren())
return std::nullopt;
/// As this operation can be potentially slow, we limit the total time spent
/// fetching children to a few ms.
Expand Down Expand Up @@ -194,28 +194,18 @@ GetSyntheticSummaryForContainer(lldb::SBValue &v) {
return summary;
}

/// Return whether we should dereference an SBValue in order to generate a more
/// meaningful summary string.
static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
/// Try to create a summary string for the given value that doesn't have a
/// summary of its own.
static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue value) {
if (!g_vsc.enable_auto_variable_summaries)
return false;

if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
return false;

// If we are referencing a pointer, we don't dereference to avoid confusing
// the user with the addresses that could shown in the summary.
if (v.Dereference().GetType().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;
return std::nullopt;

// We use the dereferenced value for generating the summary.
if (value.GetType().IsPointerType() || value.GetType().IsReferenceType())
value = value.Dereference();

// We only support auto summaries for containers.
return TryCreateAutoSummaryForContainer(value);
}

void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
Expand All @@ -227,36 +217,20 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
if (!error.Success()) {
strm << "<error: " << error.GetCString() << ">";
} else {
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;
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) {
llvm::StringRef value = v.GetValue();
llvm::StringRef nonAutoSummary = v.GetSummary();
std::optional<std::string> summary = !nonAutoSummary.empty()
? nonAutoSummary.str()
: TryCreateAutoSummary(v);
if (!value.empty()) {
strm << value;
if (summary)
strm << ' ' << *summary;
} else if (summary) {
strm << *summary;

// As last resort, we print its type and address if available.
} else {
if (llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
!type_name.empty()) {
strm << type_name;
Expand Down

0 comments on commit 96b1784

Please sign in to comment.