[Lldb-commits] [lldb] [lldb-dap] Emit more structured info along with variables (PR #75244)
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 12 12:45:21 PST 2023
https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/75244
In order to allow smarter vscode extensions, it's useful to send additional structured information of SBValues to the client. Specifically, I'm now sending error, summary, autoSummary and inMemoryValue in addition to the existing properties being sent. This is cheap because these properties have to be calculated anyway to generate the display value of the variable, but they are now available for extensions to better analyze variables. For example, if the error field is not present, the extension might be able to provide cool features, and the current way to do that is to look for the "<error: " prefix, which is error-prone.
>From 3dbe79ffdbff7283c8d462dcb7f681f38e451a18 Mon Sep 17 00:00:00 2001
From: walter erquinigo <walter at modular.com>
Date: Fri, 8 Dec 2023 12:58:30 -0500
Subject: [PATCH] [lldb-dap] Emit more structured info along with variables
In order to allow smarter vscode extensions, it's useful to send additional structured information of SBValues to the client. Specifically, I'm now sending error, summary, autoSummary and inMemoryValue in addition to the existing properties being sent. This is cheap because these properties have to be calculated anyway to generate the display value of the variable, but they are now available for extensions to better analyze variables. For example, if the error field is not present, the extension might be able to provide cool features, and the current way to do that is to look for the "<error: " prefix, which is error-prone.
---
.../lldb-dap/optimized/TestDAP_optimized.py | 7 +-
.../lldb-dap/variables/TestDAP_variables.py | 15 +-
lldb/tools/lldb-dap/BreakpointBase.cpp | 2 +-
lldb/tools/lldb-dap/JSONUtils.cpp | 194 +++++++++++-------
lldb/tools/lldb-dap/JSONUtils.h | 45 ++--
lldb/tools/lldb-dap/lldb-dap.cpp | 12 +-
6 files changed, 159 insertions(+), 116 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py b/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py
index cc544919b6a823..1443726c7ddaca 100644
--- a/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py
+++ b/lldb/test/API/tools/lldb-dap/optimized/TestDAP_optimized.py
@@ -3,10 +3,10 @@
"""
import dap_server
+import lldbdap_testcase
+from lldbsuite.test import lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-import lldbdap_testcase
class TestDAP_optimized(lldbdap_testcase.DAPTestCaseBase):
@@ -47,3 +47,6 @@ def test_optimized_variable(self):
optimized_variable = self.dap_server.get_local_variable("argc")
self.assertTrue(optimized_variable["value"].startswith("<error:"))
+ self.assertEqual(
+ optimized_variable["error"], "Could not evaluate DW_OP_entry_value."
+ )
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 9b0755eea7d3ec..bdaa0430f7fc6a 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -153,7 +153,11 @@ def do_test_scopes_variables_setVariable_evaluate(
buffer_children = make_buffer_verify_dict(0, 32)
verify_locals = {
"argc": {
- "equals": {"type": "int", "value": "1"},
+ "equals": {
+ "type": "int",
+ "value": "1",
+ "inMemoryValue": "1",
+ },
"declaration": {
"equals": {"line": 12, "column": 14},
"contains": {"path": ["lldb-dap", "variables", "main.cpp"]},
@@ -165,7 +169,14 @@ def do_test_scopes_variables_setVariable_evaluate(
"hasVariablesReference": True,
},
"pt": {
- "equals": {"type": "PointType"},
+ "equals": {
+ "type": "PointType",
+ **(
+ {"autoSummary": "{x:11, y:22}"}
+ if enableAutoVariableSummaries
+ else {}
+ ),
+ },
"hasVariablesReference": True,
"children": {
"x": {"equals": {"type": "int", "value": "11"}},
diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp
index bc9bde97678adb..fb4b27fbe315fc 100644
--- a/lldb/tools/lldb-dap/BreakpointBase.cpp
+++ b/lldb/tools/lldb-dap/BreakpointBase.cpp
@@ -296,7 +296,7 @@ bool BreakpointBase::BreakpointHitCallback(
frame.GetValueForVariablePath(expr, lldb::eDynamicDontRunTarget);
if (value.GetError().Fail())
value = frame.EvaluateExpression(expr);
- output += ValueToString(value);
+ output += VariableDescription(value).display_value;
} else {
output += messagePart.text;
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index c8e5304ecec81a..05f249488a3e15 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -211,46 +211,6 @@ static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue value) {
return TryCreateAutoSummaryForContainer(value);
}
-std::string ValueToString(lldb::SBValue v) {
- std::string result;
- llvm::raw_string_ostream strm(result);
-
- lldb::SBError error = v.GetError();
- if (!error.Success()) {
- strm << "<error: " << error.GetCString() << ">";
- } else {
- 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;
- lldb::addr_t address = v.GetLoadAddress();
- if (address != LLDB_INVALID_ADDRESS)
- strm << " @ " << llvm::format_hex(address, 0);
- }
- }
- }
- return result;
-}
-
-void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
- llvm::StringRef key) {
- std::string result = ValueToString(v);
- EmplaceSafeString(object, key, result);
-}
-
void FillResponse(const llvm::json::Object &request,
llvm::json::Object &response) {
// Fill in all of the needed response fields to a "request" and set "success"
@@ -1044,6 +1004,59 @@ std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
return name_builder.GetData();
}
+VariableDescription::VariableDescription(
+ lldb::SBValue v, bool format_hex, bool is_name_duplicated,
+ std::optional<std::string> custom_name) {
+ name = custom_name
+ ? *custom_name
+ : CreateUniqueVariableNameForDisplay(v, is_name_duplicated);
+
+ type_obj = v.GetType();
+ std::string raw_display_type_name =
+ llvm::StringRef(type_obj.GetDisplayTypeName()).str();
+ display_type_name =
+ !raw_display_type_name.empty() ? raw_display_type_name : NO_TYPENAME;
+
+ if (format_hex)
+ v.SetFormat(lldb::eFormatHex);
+
+ llvm::raw_string_ostream os_display_value(display_value);
+
+ if (lldb::SBError sb_error = v.GetError(); sb_error.Fail()) {
+ error = sb_error.GetCString();
+ os_display_value << "<error: " << error << ">";
+ } else {
+ in_memory_value = llvm::StringRef(v.GetValue()).str();
+ summary = llvm::StringRef(v.GetSummary()).str();
+ if (summary.empty())
+ auto_summary = TryCreateAutoSummary(v);
+
+ std::optional<std::string> effective_summary =
+ !summary.empty() ? summary : auto_summary;
+
+ if (!in_memory_value.empty()) {
+ os_display_value << in_memory_value;
+ if (effective_summary)
+ 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()) {
+ os_display_value << raw_display_type_name;
+ lldb::addr_t address = v.GetLoadAddress();
+ if (address != LLDB_INVALID_ADDRESS)
+ os_display_value << " @ " << llvm::format_hex(address, 0);
+ }
+ }
+ }
+
+ lldb::SBStream evaluateStream;
+ v.GetExpressionPath(evaluateStream);
+ evaluate_name = llvm::StringRef(evaluateStream.GetData()).str();
+}
+
// "Variable": {
// "type": "object",
// "description": "A Variable is a name/value pair. Optionally a variable
@@ -1103,28 +1116,52 @@ std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
// can use this optional information to present the
// children in a paged UI and fetch them in chunks."
// }
+//
+//
+// // Unofficial extensions to the protocol:
+//
// "declaration": {
-// "type": "object | undefined",
+// "type": "object",
// "description": "Extension to the protocol that indicates the source
// location where the variable was declared. This value
// might not be present if no declaration is available.",
// "properties": {
// "path": {
-// "type": "string | undefined",
+// "type": "string",
// "description": "The source file path where the variable was
// declared."
// },
// "line": {
-// "type": "number | undefined",
+// "type": "number",
// "description": "The 1-indexed source line where the variable was
// declared."
// },
// "column": {
-// "type": "number | undefined",
+// "type": "number",
// "description": "The 1-indexed source column where the variable was
// declared."
// }
// }
+// },
+// "inMemoryValue":
+// "type": "string",
+// "description": "The internal value of the variable as it's in memory.
+// This is effectively SBValue.GetValue()."
+// },
+// "summary":
+// "type": "string",
+// "description": "The summary string of the variable. This is effectively
+// SBValue.GetSummary()."
+// },
+// "autoSummary":
+// "type": "string",
+// "description": "The auto generated summary if using
+// enableAutoVariableSummaries."
+// },
+// "error":
+// "type": "string",
+// "description": "An error message generated if LLDB couldn't inspect the
+// variable."
// }
// },
// "required": [ "name", "value", "variablesReference" ]
@@ -1133,63 +1170,63 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
int64_t varID, bool format_hex,
bool is_name_duplicated,
std::optional<std::string> custom_name) {
+ VariableDescription desc(v, format_hex, is_name_duplicated, custom_name);
llvm::json::Object object;
- EmplaceSafeString(
- object, "name",
- custom_name ? *custom_name
- : CreateUniqueVariableNameForDisplay(v, is_name_duplicated));
+ EmplaceSafeString(object, "name", desc.name);
+ EmplaceSafeString(object, "value", desc.display_value);
+
+ if (desc.error)
+ EmplaceSafeString(object, "error", *desc.error);
+ if (!desc.in_memory_value.empty())
+ EmplaceSafeString(object, "inMemoryValue", desc.in_memory_value);
+ if (!desc.summary.empty())
+ EmplaceSafeString(object, "summary", desc.summary);
+ if (desc.auto_summary)
+ EmplaceSafeString(object, "autoSummary", *desc.auto_summary);
+ if (!desc.evaluate_name.empty())
+ EmplaceSafeString(object, "evaluateName", desc.evaluate_name);
- if (format_hex)
- v.SetFormat(lldb::eFormatHex);
- SetValueForKey(v, object, "value");
- auto type_obj = v.GetType();
- auto type_cstr = type_obj.GetDisplayTypeName();
// If we have a type with many children, we would like to be able to
// give a hint to the IDE that the type has indexed children so that the
- // request can be broken up in grabbing only a few children at a time. We want
- // to be careful and only call "v.GetNumChildren()" if we have an array type
- // or if we have a synthetic child provider. We don't want to call
- // "v.GetNumChildren()" on all objects as class, struct and union types don't
- // need to be completed if they are never expanded. So we want to avoid
- // calling this to only cases where we it makes sense to keep performance high
- // during normal debugging.
-
- // If we have an array type, say that it is indexed and provide the number of
- // children in case we have a huge array. If we don't do this, then we might
- // take a while to produce all children at onces which can delay your debug
- // session.
- const bool is_array = type_obj.IsArrayType();
+ // request can be broken up in grabbing only a few children at a time. We
+ // want to be careful and only call "v.GetNumChildren()" if we have an array
+ // type or if we have a synthetic child provider. We don't want to call
+ // "v.GetNumChildren()" on all objects as class, struct and union types
+ // don't need to be completed if they are never expanded. So we want to
+ // avoid calling this to only cases where we it makes sense to keep
+ // performance high during normal debugging.
+
+ // If we have an array type, say that it is indexed and provide the number
+ // of children in case we have a huge array. If we don't do this, then we
+ // might take a while to produce all children at onces which can delay your
+ // debug session.
+ const bool is_array = desc.type_obj.IsArrayType();
const bool is_synthetic = v.IsSynthetic();
if (is_array || is_synthetic) {
const auto num_children = v.GetNumChildren();
// We create a "[raw]" fake child for each synthetic type, so we have to
- // account for it when returning indexed variables. We don't need to do this
- // for non-indexed ones.
+ // account for it when returning indexed variables. We don't need to do
+ // this for non-indexed ones.
bool has_raw_child = is_synthetic && g_dap.enable_synthetic_child_debugging;
int actual_num_children = num_children + (has_raw_child ? 1 : 0);
if (is_array) {
object.try_emplace("indexedVariables", actual_num_children);
} else if (num_children > 0) {
- // If a type has a synthetic child provider, then the SBType of "v" won't
- // tell us anything about what might be displayed. So we can check if the
- // first child's name is "[0]" and then we can say it is indexed.
+ // If a type has a synthetic child provider, then the SBType of "v"
+ // won't tell us anything about what might be displayed. So we can check
+ // if the first child's name is "[0]" and then we can say it is indexed.
const char *first_child_name = v.GetChildAtIndex(0).GetName();
if (first_child_name && strcmp(first_child_name, "[0]") == 0)
object.try_emplace("indexedVariables", actual_num_children);
}
}
- EmplaceSafeString(object, "type", type_cstr ? type_cstr : NO_TYPENAME);
+ EmplaceSafeString(object, "type", desc.display_type_name);
if (varID != INT64_MAX)
object.try_emplace("id", varID);
if (v.MightHaveChildren())
object.try_emplace("variablesReference", variablesReference);
else
object.try_emplace("variablesReference", (int64_t)0);
- lldb::SBStream evaluateStream;
- v.GetExpressionPath(evaluateStream);
- const char *evaluateName = evaluateStream.GetData();
- if (evaluateName && evaluateName[0])
- EmplaceSafeString(object, "evaluateName", std::string(evaluateName));
if (lldb::SBDeclaration decl = v.GetDeclaration(); decl.IsValid()) {
llvm::json::Object decl_obj;
@@ -1206,7 +1243,8 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
if (int column = decl.GetColumn())
decl_obj.try_emplace("column", column);
- object.try_emplace("declaration", std::move(decl_obj));
+ if (!decl_obj.empty())
+ object.try_emplace("declaration", std::move(decl_obj));
}
return llvm::json::Value(std::move(object));
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 02ee499445fa19..59f207069d3183 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -167,33 +167,6 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
void FillResponse(const llvm::json::Object &request,
llvm::json::Object &response);
-/// Utility function to convert SBValue \v into a string.
-std::string ValueToString(lldb::SBValue v);
-
-/// Emplace the string value from an SBValue into the supplied object
-/// using \a key as the key that will contain the value.
-///
-/// The value is what we will display in VS Code. Some SBValue objects
-/// can have a value and/or a summary. If a value has both, we
-/// combine the value and the summary into one string. If we only have a
-/// value or summary, then that is considered the value. If there is
-/// no value and no summary then the value is the type name followed by
-/// the address of the type if it has an address.
-///
-///
-/// \param[in] v
-/// A lldb::SBValue object to extract the string value from
-///
-///
-/// \param[in] object
-/// The object to place the value object into
-///
-///
-/// \param[in] key
-/// The key name to use when inserting the value object we create
-void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
- llvm::StringRef key);
-
/// Converts \a bp to a JSON value and appends the first valid location to the
/// \a breakpoints array.
///
@@ -401,6 +374,24 @@ const char *GetNonNullVariableName(lldb::SBValue value);
std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
bool is_name_duplicated);
+/// Helper struct that parses the metadata of an \a lldb::SBValue and produces
+/// a canonical set of properties that can be sent to DAP clients.
+struct VariableDescription {
+ std::optional<std::string> error;
+ std::string display_value;
+ std::string name;
+ std::string evaluate_name;
+ std::string in_memory_value;
+ std::string summary;
+ std::optional<std::string> auto_summary;
+ lldb::SBType type_obj;
+ std::string display_type_name;
+
+ VariableDescription(lldb::SBValue v, bool format_hex = false,
+ bool is_name_duplicated = false,
+ std::optional<std::string> custom_name = {});
+};
+
/// Create a "Variable" object for a LLDB thread object.
///
/// This function will fill in the following keys in the returned
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d6b593eba93eca..c7b9b16b26a8d4 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1301,10 +1301,9 @@ void request_evaluate(const llvm::json::Object &request) {
else
EmplaceSafeString(response, "message", "evaluate failed");
} else {
- SetValueForKey(value, body, "result");
- auto value_typename = value.GetType().GetDisplayTypeName();
- EmplaceSafeString(body, "type",
- value_typename ? value_typename : NO_TYPENAME);
+ VariableDescription desc(value);
+ EmplaceSafeString(body, "result", desc.display_value);
+ EmplaceSafeString(body, "type", desc.display_type_name);
if (value.MightHaveChildren()) {
auto variableReference = g_dap.variables.InsertExpandableVariable(
value, /*is_permanent=*/context == "repl");
@@ -3084,8 +3083,9 @@ void request_setVariable(const llvm::json::Object &request) {
lldb::SBError error;
bool success = variable.SetValueFromCString(value.data(), error);
if (success) {
- SetValueForKey(variable, body, "value");
- EmplaceSafeString(body, "type", variable.GetType().GetDisplayTypeName());
+ VariableDescription desc(variable);
+ EmplaceSafeString(body, "result", desc.display_value);
+ EmplaceSafeString(body, "type", desc.display_type_name);
// We don't know the index of the variable in our g_dap.variables
// so always insert a new one to get its variablesReference.
More information about the lldb-commits
mailing list