https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/77026
>From 7656af47e058aa7101504cb31aaa067178110351 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Thu, 4 Jan 2024 15:42:35 -0800 Subject: [PATCH 1/5] [lldb-dap] Updating VariableDescription to use GetDescription() as a fallback. When generating a `display_value` for a variable the current approach calls `SBValue::GetValue()` and `SBValue::GetSummary()` to generate a `display_value` for the `SBValue`. However, there are cases where both of these return an empty string and the fallback is to print a pointer and type name instead (e.g. "FooBarType @ 0x00321"). For swift types, lldb includes a langauge runtime plugin that can generate a user description of the object but this is only used with `SBValue::GetDescription()`. For example: ``` $ lldb swift-binary ... stop at breakpoint ... lldb> script >>> event = lldb.frame.GetValueForVariablePath("event") >>> print("Value", event.GetValue()) Value None >>> print("Summary", event.GetSummary()) Summary None >>> print("Description", event) Description (main.Event) event = (name = "Greetings", time = 2024-01-04 23:38:06 UTC) ``` With this change, if GetValue and GetSummary return empty then we try `SBValue::GetDescription()` as a fallback before using the previous logic of printing "<type> @ <addr>". --- lldb/tools/lldb-dap/JSONUtils.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index df17ac9d849176..f8ac53ef809e6e 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -1042,10 +1042,14 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex, os_display_value << " " << *effective_summary; } else if (effective_summary) { os_display_value << *effective_summary; - - // As last resort, we print its type and address if available. } else { - if (!raw_display_type_name.empty()) { + lldb::SBStream description; + // Try letting lldb generate a description. + if (v.GetDescription(description) && description.GetSize()) { + os_display_value << description.GetData(); + + // As last resort, we print its type and address if available. + } else if (!raw_display_type_name.empty()) { os_display_value << raw_display_type_name; lldb::addr_t address = v.GetLoadAddress(); if (address != LLDB_INVALID_ADDRESS) >From 4c3ad4b1c9363dc9fbaebe7aa005889e87ca5039 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Fri, 5 Jan 2024 16:48:27 -0800 Subject: [PATCH 2/5] Tweaking the format to remove trailing newlines and updating tests to cover the adjusted printing. --- .../lldb-dap/evaluate/TestDAP_evaluate.py | 17 +++- .../lldb-dap/variables/TestDAP_variables.py | 93 ++++++++++++++++--- .../API/tools/lldb-dap/variables/main.cpp | 2 +- lldb/tools/lldb-dap/JSONUtils.cpp | 47 ++++++---- 4 files changed, 127 insertions(+), 32 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index de9d2c93a1109d..d9e96b8219af34 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -2,6 +2,7 @@ Test lldb-dap completions request """ +import re import lldbdap_testcase import dap_server @@ -10,7 +11,7 @@ from lldbsuite.test.lldbtest import * -class TestDAP_variables(lldbdap_testcase.DAPTestCaseBase): +class TestDAP_evaluate(lldbdap_testcase.DAPTestCaseBase): def assertEvaluate(self, expression, regex): self.assertRegexpMatches( self.dap_server.request_evaluate(expression, context=self.context)["body"][ @@ -60,7 +61,12 @@ def run_test_evaluate_expressions( self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") self.assertEvaluate( - "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x" + "struct1", + re.escape( + "{foo:15}" + if enableAutoVariableSummaries + else "(my_struct) struct1 = (foo = 15)" + ), ) self.assertEvaluate( "struct2", "0x.* {foo:16}" if enableAutoVariableSummaries else "0x.*" @@ -96,7 +102,12 @@ def run_test_evaluate_expressions( "non_static_int", "10" ) # different variable with the same name self.assertEvaluate( - "struct1", "{foo:15}" if enableAutoVariableSummaries else "my_struct @ 0x" + "struct1", + re.escape( + "{foo:15}" + if enableAutoVariableSummaries + else "(my_struct) struct1 = (foo = 15)" + ), ) self.assertEvaluate("struct1.foo", "15") self.assertEvaluate("struct2->foo", "16") diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index 58d6b941ac81a7..16824827d14d91 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -3,6 +3,7 @@ """ import os +import re import dap_server import lldbdap_testcase @@ -39,7 +40,18 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None): startswith = actual_value.startswith(verify_value) self.assertTrue( startswith, - ('"%s" value "%s" doesn\'t start with' ' "%s")') + ('"%s" value "%s" doesn\'t start with "%s")') + % (key, actual_value, verify_value), + ) + if "matches" in verify_dict: + verify = verify_dict["matches"] + for key in verify: + verify_value = verify[key] + actual_value = actual[key] + self.assertRegex( + actual_value, + verify_value, + ('"%s" value "%s" doesn\'t match pattern "%s")') % (key, actual_value, verify_value), ) if "contains" in verify_dict: @@ -150,7 +162,7 @@ def do_test_scopes_variables_setVariable_evaluate( self.continue_to_breakpoints(breakpoint_ids) locals = self.dap_server.get_local_variables() globals = self.dap_server.get_global_variables() - buffer_children = make_buffer_verify_dict(0, 32) + buffer_children = make_buffer_verify_dict(0, 16) verify_locals = { "argc": { "equals": { @@ -242,19 +254,57 @@ def do_test_scopes_variables_setVariable_evaluate( }, "pt": { "equals": {"type": "PointType"}, - "startswith": { - "result": "{x:11, y:22}" + "equals": { + "result": "{x:11, y:22, buffer:{...}}" if enableAutoVariableSummaries - else "PointType @ 0x" + else """(PointType) pt = { + x = 11 + y = 22 + buffer = { + [0] = 0 + [1] = 1 + [2] = 2 + [3] = 3 + [4] = 4 + [5] = 5 + [6] = 6 + [7] = 7 + [8] = 8 + [9] = 9 + [10] = 10 + [11] = 11 + [12] = 12 + [13] = 13 + [14] = 14 + [15] = 15 + } +}""" }, "hasVariablesReference": True, }, "pt.buffer": { "equals": {"type": "int[32]"}, - "startswith": { + "equals": { "result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}" if enableAutoVariableSummaries - else "int[32] @ 0x" + else """(int[16]) buffer = { + [0] = 0 + [1] = 1 + [2] = 2 + [3] = 3 + [4] = 4 + [5] = 5 + [6] = 6 + [7] = 7 + [8] = 8 + [9] = 9 + [10] = 10 + [11] = 11 + [12] = 12 + [13] = 13 + [14] = 14 + [15] = 15 +}""" }, "hasVariablesReference": True, }, @@ -440,7 +490,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo }, "buffer": { "children": buffer_children, - "equals": {"indexedVariables": 32}, + "equals": {"indexedVariables": 16}, }, }, }, @@ -457,10 +507,31 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo "name": "pt", "response": { "equals": {"type": "PointType"}, - "startswith": { - "result": "{x:11, y:22}" + "matches": { + "result": re.escape("{x:11, y:22, buffer:{...}}") if enableAutoVariableSummaries - else "PointType @ 0x" + else r"""\(PointType\) (\$0|pt) = \{ + x = 11 + y = 22 + buffer = \{ + \[0\] = 0 + \[1\] = 1 + \[2\] = 2 + \[3\] = 3 + \[4\] = 4 + \[5\] = 5 + \[6\] = 6 + \[7\] = 7 + \[8\] = 8 + \[9\] = 9 + \[10\] = 10 + \[11\] = 11 + \[12\] = 12 + \[13\] = 13 + \[14\] = 14 + \[15\] = 15 + \} +\}""" }, "missing": ["indexedVariables"], "hasVariablesReference": True, diff --git a/lldb/test/API/tools/lldb-dap/variables/main.cpp b/lldb/test/API/tools/lldb-dap/variables/main.cpp index da09ecb474f3b2..3557067ed9ce13 100644 --- a/lldb/test/API/tools/lldb-dap/variables/main.cpp +++ b/lldb/test/API/tools/lldb-dap/variables/main.cpp @@ -1,5 +1,5 @@ -#define BUFFER_SIZE 32 +#define BUFFER_SIZE 16 struct PointType { int x; int y; diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index f8ac53ef809e6e..88f9df20b40409 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -135,6 +135,18 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj, return strs; } +static std::string GetDescriptionTrimmed(lldb::SBValue &value) { + lldb::SBStream stream; + value.GetDescription(stream); + const char *description = stream.GetData(); + size_t length = stream.GetSize(); + if (length > 0 && + (description[length - 1] == '\n' || description[length - 1] == '\r')) { + --length; + } + return std::string(description, length); +} + /// 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. @@ -173,21 +185,21 @@ TryCreateAutoSummaryForContainer(lldb::SBValue &v) { lldb::SBValue child = v.GetChildAtIndex(i); if (llvm::StringRef name = child.GetName(); !name.empty()) { - llvm::StringRef value; + llvm::StringRef desc; if (llvm::StringRef summary = child.GetSummary(); !summary.empty()) - value = summary; + desc = summary; + else if (llvm::StringRef value = child.GetValue(); !value.empty()) + desc = value; 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 = ", "; - } + desc = "{...}"; // Fallback for nested types. + + // If the child is an indexed entry, we don't show its index to save + // characters. + if (name.starts_with("[")) + os << separator << desc; + else + os << separator << name << ":" << desc; + separator = ", "; } } os << "}"; @@ -1043,10 +1055,11 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex, } else if (effective_summary) { os_display_value << *effective_summary; } else { - lldb::SBStream description; - // Try letting lldb generate a description. - if (v.GetDescription(description) && description.GetSize()) { - os_display_value << description.GetData(); + // Try the value description, which may call into language runtime + // specific formatters, see ValueObjectPrinter. + std::string description = GetDescriptionTrimmed(v); + if (!description.empty()) { + os_display_value << description; // As last resort, we print its type and address if available. } else if (!raw_display_type_name.empty()) { >From 86b8fd1493c7a0df9f34613cfcf078609074313c Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Tue, 9 Jan 2024 11:40:28 -0800 Subject: [PATCH 3/5] Ensuring the SBValue::GetDescription returns the correct value to inidicate if the description was actually detected. --- lldb/source/API/SBValue.cpp | 13 +++++++------ lldb/tools/lldb-dap/JSONUtils.cpp | 5 ++++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 89d26a1fbe2824..b3e896dedabc0a 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -1210,15 +1210,16 @@ bool SBValue::GetDescription(SBStream &description) { ValueLocker locker; lldb::ValueObjectSP value_sp(GetSP(locker)); - if (value_sp) { - DumpValueObjectOptions options; - options.SetUseDynamicType(m_opaque_sp->GetUseDynamic()); - options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic()); - value_sp->Dump(strm, options); - } else { + if (!value_sp) { strm.PutCString("No value"); + return false; } + DumpValueObjectOptions options; + options.SetUseDynamicType(m_opaque_sp->GetUseDynamic()); + options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic()); + value_sp->Dump(strm, options); + return true; } diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 88f9df20b40409..b45d737cceb58a 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -137,7 +137,10 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj, static std::string GetDescriptionTrimmed(lldb::SBValue &value) { lldb::SBStream stream; - value.GetDescription(stream); + if (!value.GetDescription(stream)) { + return ""; + } + const char *description = stream.GetData(); size_t length = stream.GetSize(); if (length > 0 && >From f3d1412fe1f4b6641d100a628844db1a13996baf Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Tue, 9 Jan 2024 13:30:11 -0800 Subject: [PATCH 4/5] Rename GetDescriptionTrimmed>TryCreateDescription and simplify the impl. --- lldb/tools/lldb-dap/JSONUtils.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index b45d737cceb58a..1d0790ff6cc113 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -135,19 +135,16 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj, return strs; } -static std::string GetDescriptionTrimmed(lldb::SBValue &value) { +/// Creates a description of the specified value. The description may be +/// generated by the language runtime of the value, if one is detected +/// and configured. +static std::optional<std::string> TryCreateDescription(lldb::SBValue &value) { lldb::SBStream stream; - if (!value.GetDescription(stream)) { - return ""; - } + if (!value.IsValid() || !value.GetDescription(stream)) + return std::nullopt; - const char *description = stream.GetData(); - size_t length = stream.GetSize(); - if (length > 0 && - (description[length - 1] == '\n' || description[length - 1] == '\r')) { - --length; - } - return std::string(description, length); + llvm::StringRef description = stream.GetData(); + return description.trim().str(); } /// Create a short summary for a container that contains the summary of its @@ -1060,9 +1057,9 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex, } else { // Try the value description, which may call into language runtime // specific formatters, see ValueObjectPrinter. - std::string description = GetDescriptionTrimmed(v); - if (!description.empty()) { - os_display_value << description; + std::optional<std::string> description = TryCreateDescription(v); + if (description) { + os_display_value << *description; // As last resort, we print its type and address if available. } else if (!raw_display_type_name.empty()) { >From d8eb78e220b2127b78cee2868daddbe57ae2c5c8 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Wed, 10 Jan 2024 13:16:51 -0800 Subject: [PATCH 5/5] Adjusting how variables are printed in the DAP evalulation 'repl' and 'hover' context to have more detail object descriptions. --- .../lldb-dap/evaluate/TestDAP_evaluate.py | 66 ++++-- .../lldb-dap/variables/TestDAP_variables.py | 189 +++++++++--------- lldb/tools/lldb-dap/JSONUtils.cpp | 40 ++-- lldb/tools/lldb-dap/JSONUtils.h | 3 + lldb/tools/lldb-dap/lldb-dap.cpp | 2 +- 5 files changed, 163 insertions(+), 137 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index d9e96b8219af34..4016316a509130 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -26,6 +26,9 @@ def assertEvaluateFailure(self, expression): self.dap_server.request_evaluate(expression, context=self.context)["body"], ) + def isResultExpandedDescription(self): + return self.context == "repl" or self.context == "hover" + def isExpressionParsedExpected(self): return self.context != "hover" @@ -60,21 +63,30 @@ def run_test_evaluate_expressions( self.assertEvaluate("var2", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") - self.assertEvaluate( - "struct1", - re.escape( - "{foo:15}" - if enableAutoVariableSummaries - else "(my_struct) struct1 = (foo = 15)" - ), - ) - self.assertEvaluate( - "struct2", "0x.* {foo:16}" if enableAutoVariableSummaries else "0x.*" - ) - self.assertEvaluate("struct3", "0x.*0") self.assertEvaluate("struct1.foo", "15") self.assertEvaluate("struct2->foo", "16") + if self.isResultExpandedDescription(): + self.assertEvaluate( + "struct1", + r"\(my_struct\) (struct1|\$\d+) = \(foo = 15\)", + ) + self.assertEvaluate( + "struct2", r"\(my_struct \*\) (struct2|\$\d+) = 0x.*" + ) + self.assertEvaluate("struct3", r"\(my_struct \*\) (struct3|\$\d+) = nullptr") + else: + self.assertEvaluate( + "struct1", + re.escape("{foo:15}") + if enableAutoVariableSummaries + else "my_struct @ 0x", + ) + self.assertEvaluate( + "struct2", "0x.* {foo:16}" if enableAutoVariableSummaries else "0x.*" + ) + self.assertEvaluate("struct3", "0x.*0") + self.assertEvaluateFailure("var") # local variable of a_function self.assertEvaluateFailure("my_struct") # type name self.assertEvaluateFailure("int") # type name @@ -101,14 +113,18 @@ def run_test_evaluate_expressions( self.assertEvaluate( "non_static_int", "10" ) # different variable with the same name - self.assertEvaluate( - "struct1", - re.escape( - "{foo:15}" + if self.isResultExpandedDescription(): + self.assertEvaluate( + "struct1", + r"\(my_struct\) (struct1|\$\d+) = \(foo = 15\)", + ) + else: + self.assertEvaluate( + "struct1", + re.escape("{foo:15}") if enableAutoVariableSummaries - else "(my_struct) struct1 = (foo = 15)" - ), - ) + else "my_struct @ 0x", + ) self.assertEvaluate("struct1.foo", "15") self.assertEvaluate("struct2->foo", "16") @@ -175,16 +191,22 @@ def test_generic_evaluate_expressions(self): @skipIfRemote def test_repl_evaluate_expressions(self): # Tests expression evaluations that are triggered from the Debug Console - self.run_test_evaluate_expressions("repl", enableAutoVariableSummaries=True) + self.run_test_evaluate_expressions("repl", enableAutoVariableSummaries=False) @skipIfWindows @skipIfRemote def test_watch_evaluate_expressions(self): # Tests expression evaluations that are triggered from a watch expression - self.run_test_evaluate_expressions("watch", enableAutoVariableSummaries=False) + self.run_test_evaluate_expressions("watch", enableAutoVariableSummaries=True) @skipIfWindows @skipIfRemote def test_hover_evaluate_expressions(self): # Tests expression evaluations that are triggered when hovering on the editor - self.run_test_evaluate_expressions("hover", enableAutoVariableSummaries=True) + self.run_test_evaluate_expressions("hover", enableAutoVariableSummaries=False) + + @skipIfWindows + @skipIfRemote + def test_variable_evaluate_expressions(self): + # Tests expression evaluations that are triggered in the variable explorer + self.run_test_evaluate_expressions("variable", enableAutoVariableSummaries=True) diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index 16824827d14d91..2cee170d8dafc9 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -3,7 +3,6 @@ """ import os -import re import dap_server import lldbdap_testcase @@ -254,57 +253,19 @@ def do_test_scopes_variables_setVariable_evaluate( }, "pt": { "equals": {"type": "PointType"}, - "equals": { + "startswith": { "result": "{x:11, y:22, buffer:{...}}" if enableAutoVariableSummaries - else """(PointType) pt = { - x = 11 - y = 22 - buffer = { - [0] = 0 - [1] = 1 - [2] = 2 - [3] = 3 - [4] = 4 - [5] = 5 - [6] = 6 - [7] = 7 - [8] = 8 - [9] = 9 - [10] = 10 - [11] = 11 - [12] = 12 - [13] = 13 - [14] = 14 - [15] = 15 - } -}""" + else "PointType @ 0x" }, "hasVariablesReference": True, }, "pt.buffer": { - "equals": {"type": "int[32]"}, - "equals": { + "equals": {"type": "int[16]"}, + "startswith": { "result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}" if enableAutoVariableSummaries - else """(int[16]) buffer = { - [0] = 0 - [1] = 1 - [2] = 2 - [3] = 3 - [4] = 4 - [5] = 5 - [6] = 6 - [7] = 7 - [8] = 8 - [9] = 9 - [10] = 10 - [11] = 11 - [12] = 12 - [13] = 13 - [14] = 14 - [15] = 15 -}""" + else "int[16] @ 0x" }, "hasVariablesReference": True, }, @@ -505,36 +466,85 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo # the other temporary (from other UI). expandable_expression = { "name": "pt", - "response": { - "equals": {"type": "PointType"}, - "matches": { - "result": re.escape("{x:11, y:22, buffer:{...}}") - if enableAutoVariableSummaries - else r"""\(PointType\) (\$0|pt) = \{ + "context": { + "repl": { + "equals": {"type": "PointType"}, + "equals": { + "result": """(PointType) $0 = { x = 11 y = 22 - buffer = \{ - \[0\] = 0 - \[1\] = 1 - \[2\] = 2 - \[3\] = 3 - \[4\] = 4 - \[5\] = 5 - \[6\] = 6 - \[7\] = 7 - \[8\] = 8 - \[9\] = 9 - \[10\] = 10 - \[11\] = 11 - \[12\] = 12 - \[13\] = 13 - \[14\] = 14 - \[15\] = 15 - \} -\}""" + buffer = { + [0] = 0 + [1] = 1 + [2] = 2 + [3] = 3 + [4] = 4 + [5] = 5 + [6] = 6 + [7] = 7 + [8] = 8 + [9] = 9 + [10] = 10 + [11] = 11 + [12] = 12 + [13] = 13 + [14] = 14 + [15] = 15 + } +}""" + }, + "missing": ["indexedVariables"], + "hasVariablesReference": True, + }, + "hover": { + "equals": {"type": "PointType"}, + "equals": { + "result": """(PointType) pt = { + x = 11 + y = 22 + buffer = { + [0] = 0 + [1] = 1 + [2] = 2 + [3] = 3 + [4] = 4 + [5] = 5 + [6] = 6 + [7] = 7 + [8] = 8 + [9] = 9 + [10] = 10 + [11] = 11 + [12] = 12 + [13] = 13 + [14] = 14 + [15] = 15 + } +}""" + }, + "missing": ["indexedVariables"], + "hasVariablesReference": True, + }, + "watch": { + "equals": {"type": "PointType"}, + "startswith": { + "result": "{x:11, y:22, buffer:{...}}" + if enableAutoVariableSummaries + else "PointType @ 0x" + }, + "missing": ["indexedVariables"], + "hasVariablesReference": True, + }, + "variables": { + "equals": {"type": "PointType"}, + "startswith": { + "result": "{x:11, y:22, buffer:{...}}" + if enableAutoVariableSummaries + else "PointType @ 0x" + }, + "missing": ["indexedVariables"], + "hasVariablesReference": True, }, - "missing": ["indexedVariables"], - "hasVariablesReference": True, }, "children": { "x": {"equals": {"type": "int", "value": "11"}}, @@ -543,27 +553,18 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo }, } - # Evaluate from permanent UI. - permanent_expr_varref_dict = {} - response = self.dap_server.request_evaluate( - expandable_expression["name"], frameIndex=0, threadId=None, context="repl" - ) - self.verify_values( - expandable_expression["response"], - response["body"], - permanent_expr_varref_dict, - expandable_expression["name"], - ) - - # Evaluate from temporary UI. - temporary_expr_varref_dict = {} - response = self.dap_server.request_evaluate(expandable_expression["name"]) - self.verify_values( - expandable_expression["response"], - response["body"], - temporary_expr_varref_dict, - expandable_expression["name"], - ) + # Evaluate from known contexts. + expr_varref_dict = {} + for context, verify_dict in expandable_expression["context"].items(): + response = self.dap_server.request_evaluate( + expandable_expression["name"], frameIndex=0, threadId=None, context=context + ) + self.verify_values( + verify_dict, + response["body"], + expr_varref_dict, + expandable_expression["name"], + ) # Evaluate locals again. locals = self.dap_server.get_local_variables() @@ -571,7 +572,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo # Verify the evaluated expressions before second locals evaluation # can be expanded. - var_ref = temporary_expr_varref_dict[expandable_expression["name"]] + var_ref = expr_varref_dict[expandable_expression["name"]] response = self.dap_server.request_variables(var_ref) self.verify_variables( expandable_expression["children"], response["body"]["variables"] @@ -587,7 +588,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo ) self.continue_to_breakpoints(breakpoint_ids) - var_ref = permanent_expr_varref_dict[expandable_expression["name"]] + var_ref = expr_varref_dict[expandable_expression["name"]] response = self.dap_server.request_variables(var_ref) self.verify_variables( expandable_expression["children"], response["body"]["variables"] diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 1d0790ff6cc113..793246e4551170 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -135,18 +135,6 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj, return strs; } -/// Creates a description of the specified value. The description may be -/// generated by the language runtime of the value, if one is detected -/// and configured. -static std::optional<std::string> TryCreateDescription(lldb::SBValue &value) { - lldb::SBStream stream; - if (!value.IsValid() || !value.GetDescription(stream)) - return std::nullopt; - - llvm::StringRef description = stream.GetData(); - return description.trim().str(); -} - /// 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. @@ -1054,15 +1042,10 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex, os_display_value << " " << *effective_summary; } else if (effective_summary) { os_display_value << *effective_summary; + + // As last resort, we print its type and address if available. } else { - // Try the value description, which may call into language runtime - // specific formatters, see ValueObjectPrinter. - std::optional<std::string> description = TryCreateDescription(v); - if (description) { - os_display_value << *description; - - // As last resort, we print its type and address if available. - } else if (!raw_display_type_name.empty()) { + if (!raw_display_type_name.empty()) { os_display_value << raw_display_type_name; lldb::addr_t address = v.GetLoadAddress(); if (address != LLDB_INVALID_ADDRESS) @@ -1108,6 +1091,23 @@ llvm::json::Object VariableDescription::GetVariableExtensionsJSON() { return extensions; } +std::string VariableDescription::GetResult(llvm::StringRef context) { + // In repl and hover context, the results can be displayed as multiple lines + // so more detailed descriptions can be returned. + if (context != "repl" && context != "hover") + return display_value; + + lldb::SBStream stream; + // Try the SBValue::GetDescription(), which may call into language runtime + // specific formatters (see ValueObjectPrinter), otherwise return the + // display_value. + if (!v.IsValid() || !v.GetDescription(stream)) + return display_value; + + llvm::StringRef description = stream.GetData(); + return description.trim().str(); +} + // "Variable": { // "type": "object", // "description": "A Variable is a name/value pair. Optionally a variable diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 7f9fc7037c63d6..056d04945f03d4 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -405,6 +405,9 @@ struct VariableDescription { /// Create a JSON object that represents these extensions to the DAP variable /// response. llvm::json::Object GetVariableExtensionsJSON(); + + /// Returns a description of the value appropraite for the specified context. + std::string GetResult(llvm::StringRef context); }; /// Create a "Variable" object for a LLDB thread object. diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 0d45b0379c8526..e91b4115156b73 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1317,7 +1317,7 @@ void request_evaluate(const llvm::json::Object &request) { EmplaceSafeString(response, "message", "evaluate failed"); } else { VariableDescription desc(value); - EmplaceSafeString(body, "result", desc.display_value); + EmplaceSafeString(body, "result", desc.GetResult(context)); EmplaceSafeString(body, "type", desc.display_type_name); if (value.MightHaveChildren()) { auto variableReference = g_dap.variables.InsertExpandableVariable( _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits