llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb <details> <summary>Changes</summary> 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. An example of the proposal: <img width="260" alt="Screenshot 2023-09-15 at 5 15 31 PM" src="https://github.com/llvm/llvm-project/assets/1613874/b55c8dbc-9b6b-40f3-8cba-3bee4a1237a9"> --- Full diff: https://github.com/llvm/llvm-project/pull/66551.diff 3 Files Affected: - (modified) lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py (+4-1) - (modified) lldb/test/API/tools/lldb-vscode/evaluate/main.cpp (+2-1) - (modified) lldb/tools/lldb-vscode/JSONUtils.cpp (+30-21) ``````````diff 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) { `````````` </details> https://github.com/llvm/llvm-project/pull/66551 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
