https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/66551
enableAutoVariableSummaries is a setting that enhances the summaries of variables in the IDE. A shortcoming of this feature is that it wasn't showing the addresses of pointers, which is valuable information. This fixes it by adding it whenever applicable. >From 776f589a3fdfd160ce449461d0d2936e52849bd2 Mon Sep 17 00:00:00 2001 From: walter erquinigo <wal...@modular.com> Date: Fri, 15 Sep 2023 16:50:35 -0400 Subject: [PATCH] [lldb-vscode] Show addresses next to dereferenced summaries when using enableAutoVariableSummaries enableAutoVariableSummaries is a setting that enhances the summaries of variables in the IDE. A shortcoming of this feature is that it wasn't showing the addresses of pointers, which is valuable information. This fixes it by adding it whenever applicable. --- .../evaluate/TestVSCode_evaluate.py | 5 +- .../API/tools/lldb-vscode/evaluate/main.cpp | 3 +- lldb/tools/lldb-vscode/JSONUtils.cpp | 51 +++++++++++-------- 3 files changed, 36 insertions(+), 23 deletions(-) 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 3cfe02ef6aa1576..bf3b16067fed2fa 100644 --- a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py +++ b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py @@ -63,7 +63,10 @@ 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") diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp index 3a541b21b220828..f09d00e6444bb79 100644 --- a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp +++ b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp @@ -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 @@ -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; } diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp index c6b422e4d7a02e6..d475f45bec459bb 100644 --- a/lldb/tools/lldb-vscode/JSONUtils.cpp +++ b/lldb/tools/lldb-vscode/JSONUtils.cpp @@ -203,10 +203,12 @@ static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) { 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; + // We don't want to dereference invalid data. + if (!v.IsSynthetic()) { + lldb::addr_t address = v.GetValueAsUnsigned(0); + if (address == 0 && address == LLDB_INVALID_ADDRESS) + return false; + } // If it's synthetic or a pointer to a basic type that provides a summary, we // don't dereference. @@ -227,33 +229,40 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object, if (!error.Success()) { strm << "<error: " << error.GetCString() << ">"; } else { - auto tryDumpSummaryAndValue = [&strm](lldb::SBValue value) { + auto tryDumpSummaryAndValue = + [](lldb::SBValue value) -> std::optional<std::string> { llvm::StringRef val = value.GetValue(); llvm::StringRef summary = value.GetSummary(); if (!val.empty()) { - strm << val; + std::string dump; + llvm::raw_string_ostream os(dump); + os << val; if (!summary.empty()) - strm << ' ' << summary; - return true; + os << ' ' << summary; + return dump; } - if (!summary.empty()) { - strm << ' ' << summary; - return true; - } - if (auto container_summary = GetSyntheticSummaryForContainer(value)) { - strm << *container_summary; - return true; - } - return false; + if (!summary.empty()) + return summary.str(); + return GetSyntheticSummaryForContainer(value); }; + bool done = false; // We first try to get the summary from its dereferenced value. - bool done = ShouldBeDereferencedForSummary(v) && - tryDumpSummaryAndValue(v.Dereference()); + if (ShouldBeDereferencedForSummary(v)) { + if (std::optional<std::string> text = + tryDumpSummaryAndValue(v.Dereference())) { + strm << v.GetValue() << " → " << *text; + done = true; + } + } // We then try to get it from the current value itself. - if (!done) - done = tryDumpSummaryAndValue(v); + if (!done) { + if (std::optional<std::string> text = tryDumpSummaryAndValue(v)) { + strm << *text; + done = true; + } + } // As last resort, we print its type and address if available. if (!done) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits