[lldb-dap] Migrating 'evaluate' to structured types. #167720
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adding structured types for the evaluate request handler.
This should be mostly a non-functional change. I did catch some spelling mistakes in our tests ('variable' vs 'variables').
Member
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesAdding structured types for the evaluate request handler. This should be mostly a non-functional change. I did catch some spelling mistakes in our tests ('variable' vs 'variables'). Patch is 28.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167720.diff 10 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index ac550962cfb85..a4ca090021f3f 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -978,9 +978,10 @@ def request_evaluate(self, expression, frameIndex=0, threadId=None, context=None
return []
args_dict = {
"expression": expression,
- "context": context,
"frameId": stackFrame["id"],
}
+ if context:
+ args_dict["context"] = context
command_dict = {
"command": "evaluate",
"type": "request",
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 20a75f4076e42..d620cddbebcc1 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -1,5 +1,5 @@
"""
-Test lldb-dap completions request
+Test lldb-dap evaluate request
"""
import re
@@ -245,4 +245,6 @@ def test_hover_evaluate_expressions(self):
@skipIfWindows
def test_variable_evaluate_expressions(self):
# Tests expression evaluations that are triggered in the variable explorer
- self.run_test_evaluate_expressions("variable", enableAutoVariableSummaries=True)
+ self.run_test_evaluate_expressions(
+ "variables", enableAutoVariableSummaries=True
+ )
diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
index e1556846dff19..57b2f621865f1 100644
--- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
@@ -10,148 +10,36 @@
#include "EventHelper.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "Protocol/ProtocolTypes.h"
#include "RequestHandler.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include <future>
+#include <optional>
+#include <unistd.h>
+
+using namespace llvm;
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
namespace lldb_dap {
-// "EvaluateRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Evaluate request; value of command field is 'evaluate'.
-// Evaluates the given expression in the context of the
-// top most stack frame. The expression has access to any
-// variables and arguments that are in scope.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "evaluate" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/EvaluateArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "EvaluateArguments": {
-// "type": "object",
-// "description": "Arguments for 'evaluate' request.",
-// "properties": {
-// "expression": {
-// "type": "string",
-// "description": "The expression to evaluate."
-// },
-// "frameId": {
-// "type": "integer",
-// "description": "Evaluate the expression in the scope of this stack
-// frame. If not specified, the expression is evaluated
-// in the global scope."
-// },
-// "context": {
-// "type": "string",
-// "_enum": [ "watch", "repl", "hover" ],
-// "enumDescriptions": [
-// "evaluate is run in a watch.",
-// "evaluate is run from REPL console.",
-// "evaluate is run from a data hover."
-// ],
-// "description": "The context in which the evaluate request is run."
-// },
-// "format": {
-// "$ref": "#/definitions/ValueFormat",
-// "description": "Specifies details on how to format the Evaluate
-// result."
-// }
-// },
-// "required": [ "expression" ]
-// },
-// "EvaluateResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'evaluate' request.",
-// "properties": {
-// "body": {
-// "type": "object",
-// "properties": {
-// "result": {
-// "type": "string",
-// "description": "The result of the evaluate request."
-// },
-// "type": {
-// "type": "string",
-// "description": "The optional type of the evaluate result."
-// },
-// "presentationHint": {
-// "$ref": "#/definitions/VariablePresentationHint",
-// "description": "Properties of a evaluate result that can be
-// used to determine how to render the result in
-// the UI."
-// },
-// "variablesReference": {
-// "type": "number",
-// "description": "If variablesReference is > 0, the evaluate
-// result is structured and its children can be
-// retrieved by passing variablesReference to the
-// VariablesRequest."
-// },
-// "namedVariables": {
-// "type": "number",
-// "description": "The number of named child variables. The
-// client can use this optional information to
-// present the variables in a paged UI and fetch
-// them in chunks."
-// },
-// "indexedVariables": {
-// "type": "number",
-// "description": "The number of indexed child variables. The
-// client can use this optional information to
-// present the variables in a paged UI and fetch
-// them in chunks."
-// },
-// "valueLocationReference": {
-// "type": "integer",
-// "description": "A reference that allows the client to request
-// the location where the returned value is
-// declared. For example, if a function pointer is
-// returned, the adapter may be able to look up the
-// function's location. This should be present only
-// if the adapter is likely to be able to resolve
-// the location.\n\nThis reference shares the same
-// lifetime as the `variablesReference`. See
-// 'Lifetime of Object References' in the
-// Overview section for details."
-// }
-// "memoryReference": {
-// "type": "string",
-// "description": "A memory reference to a location appropriate
-// for this result. For pointer type eval
-// results, this is generally a reference to the
-// memory address contained in the pointer. This
-// attribute may be returned by a debug adapter
-// if corresponding capability
-// `supportsMemoryReferences` is true."
-// },
-// },
-// "required": [ "result", "variablesReference" ]
-// }
-// },
-// "required": [ "body" ]
-// }]
-// }
-void EvaluateRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- llvm::json::Object body;
- const auto *arguments = request.getObject("arguments");
- lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
- std::string expression =
- GetString(arguments, "expression").value_or("").str();
- const llvm::StringRef context = GetString(arguments, "context").value_or("");
+/// Evaluates the given expression in the context of a stack frame.
+///
+/// The expression has access to any variables and arguments that are in scope.
+Expected<EvaluateResponseBody>
+EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const {
+ EvaluateResponseBody body;
+ lldb::SBFrame frame = dap.GetLLDBFrame(arguments.frameId);
+ std::string expression = arguments.expression;
bool repeat_last_command =
expression.empty() && dap.last_nonempty_var_expression.empty();
- if (context == "repl" &&
+ if (arguments.context == protocol::eEvaluateContextRepl &&
(repeat_last_command ||
(!expression.empty() &&
dap.DetectReplMode(frame, expression, false) == ReplMode::Command))) {
@@ -169,66 +57,61 @@ void EvaluateRequestHandler::operator()(
dap.debugger, llvm::StringRef(), {expression}, required_command_failed,
/*parse_command_directives=*/false, /*echo_commands=*/false);
- EmplaceSafeString(body, "result", result);
- body.try_emplace("variablesReference", (int64_t)0);
- } else {
- if (context == "repl") {
- // If the expression is empty and the last expression was for a
- // variable, set the expression to the previous expression (repeat the
- // evaluation); otherwise save the current non-empty expression for the
- // next (possibly empty) variable expression.
- if (expression.empty())
- expression = dap.last_nonempty_var_expression;
- else
- dap.last_nonempty_var_expression = expression;
- }
- // Always try to get the answer from the local variables if possible. If
- // this fails, then if the context is not "hover", actually evaluate an
- // expression using the expression parser.
- //
- // "frame variable" is more reliable than the expression parser in
- // many cases and it is faster.
- lldb::SBValue value = frame.GetValueForVariablePath(
- expression.data(), lldb::eDynamicDontRunTarget);
-
- // Freeze dry the value in case users expand it later in the debug console
- if (value.GetError().Success() && context == "repl")
- value = value.Persist();
-
- if (value.GetError().Fail() && context != "hover")
- value = frame.EvaluateExpression(expression.data());
-
- if (value.GetError().Fail()) {
- response["success"] = llvm::json::Value(false);
- // This error object must live until we're done with the pointer returned
- // by GetCString().
- lldb::SBError error = value.GetError();
- const char *error_cstr = error.GetCString();
- if (error_cstr && error_cstr[0])
- EmplaceSafeString(response, "message", error_cstr);
- else
- EmplaceSafeString(response, "message", "evaluate failed");
- } else {
- VariableDescription desc(value,
- dap.configuration.enableAutoVariableSummaries);
- EmplaceSafeString(body, "result", desc.GetResult(context));
- EmplaceSafeString(body, "type", desc.display_type_name);
- int64_t var_ref = 0;
- if (value.MightHaveChildren() || ValuePointsToCode(value))
- var_ref = dap.variables.InsertVariable(
- value, /*is_permanent=*/context == "repl");
- if (value.MightHaveChildren())
- body.try_emplace("variablesReference", var_ref);
- else
- body.try_emplace("variablesReference", (int64_t)0);
- if (lldb::addr_t addr = value.GetLoadAddress();
- addr != LLDB_INVALID_ADDRESS)
- body.try_emplace("memoryReference", EncodeMemoryReference(addr));
- if (ValuePointsToCode(value))
- body.try_emplace("valueLocationReference", var_ref);
- }
+ body.result = result;
+ return body;
+ }
+
+ if (arguments.context == eEvaluateContextRepl) {
+ // If the expression is empty and the last expression was for a
+ // variable, set the expression to the previous expression (repeat the
+ // evaluation); otherwise save the current non-empty expression for the
+ // next (possibly empty) variable expression.
+ if (expression.empty())
+ expression = dap.last_nonempty_var_expression;
+ else
+ dap.last_nonempty_var_expression = expression;
+ }
+
+ // Always try to get the answer from the local variables if possible. If
+ // this fails, then if the context is not "hover", actually evaluate an
+ // expression using the expression parser.
+ //
+ // "frame variable" is more reliable than the expression parser in
+ // many cases and it is faster.
+ lldb::SBValue value = frame.GetValueForVariablePath(
+ expression.data(), lldb::eDynamicDontRunTarget);
+
+ // Freeze dry the value in case users expand it later in the debug console
+ if (value.GetError().Success() && arguments.context == eEvaluateContextRepl)
+ value = value.Persist();
+
+ if (value.GetError().Fail() && arguments.context != eEvaluateContextHover)
+ value = frame.EvaluateExpression(expression.data());
+
+ if (value.GetError().Fail()) {
+ // This error object must live until we're done with the pointer returned
+ // by GetCString().
+ lldb::SBError error = value.GetError();
+ const char *error_cstr = error.GetCString();
+ return make_error<DAPError>(
+ error_cstr && error_cstr[0] ? error_cstr : "evaluate failed");
}
- response.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(response)));
+
+ VariableDescription desc(value,
+ dap.configuration.enableAutoVariableSummaries);
+ body.result = desc.GetResult(arguments.context);
+ body.type = desc.display_type_name;
+ if (value.MightHaveChildren() || ValuePointsToCode(value))
+ body.variablesReference = dap.variables.InsertVariable(
+ value, /*is_permanent=*/arguments.context == eEvaluateContextRepl);
+ if (lldb::addr_t addr = value.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
+ body.memoryReference = EncodeMemoryReference(addr);
+
+ if (ValuePointsToCode(value) &&
+ body.variablesReference != LLDB_DAP_INVALID_VARRERF)
+ body.valueLocationReference = body.variablesReference;
+
+ return body;
}
+
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index bc22133d92453..65a52075ebd79 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -292,11 +292,14 @@ class DisconnectRequestHandler
Run(const std::optional<protocol::DisconnectArguments> &args) const override;
};
-class EvaluateRequestHandler : public LegacyRequestHandler {
+class EvaluateRequestHandler
+ : public RequestHandler<protocol::EvaluateArguments,
+ llvm::Expected<protocol::EvaluateResponseBody>> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "evaluate"; }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Expected<protocol::EvaluateResponseBody>
+ Run(const protocol::EvaluateArguments &) const override;
FeatureSet GetSupportedFeatures() const override {
return {protocol::eAdapterFeatureEvaluateForHovers};
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 1a3a6701b194d..81eadae03bb48 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -11,6 +11,7 @@
#include "ExceptionBreakpoint.h"
#include "LLDBUtils.h"
#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolRequests.h"
#include "ProtocolUtils.h"
#include "lldb/API/SBAddress.h"
#include "lldb/API/SBCompileUnit.h"
@@ -817,10 +818,10 @@ VariableDescription::VariableDescription(lldb::SBValue v,
evaluate_name = llvm::StringRef(evaluateStream.GetData()).str();
}
-std::string VariableDescription::GetResult(llvm::StringRef context) {
+std::string VariableDescription::GetResult(protocol::EvaluateContext context) {
// In repl context, the results can be displayed as multiple lines so more
// detailed descriptions can be returned.
- if (context != "repl")
+ if (context != protocol::eEvaluateContextRepl)
return display_value;
if (!v.IsValid())
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 0c865a33a6ce4..329dc8ab02f99 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -10,7 +10,7 @@
#define LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
#include "DAPForward.h"
-#include "Protocol/ProtocolTypes.h"
+#include "Protocol/ProtocolRequests.h"
#include "lldb/API/SBCompileUnit.h"
#include "lldb/API/SBFormat.h"
#include "lldb/API/SBType.h"
@@ -28,7 +28,7 @@
namespace lldb_dap {
-/// Emplace a StringRef in a json::Object after enusring that the
+/// Emplace a StringRef in a json::Object after ensuring that the
/// string is valid UTF8. If not, first call llvm::json::fixUTF8
/// before emplacing.
///
@@ -351,7 +351,7 @@ struct VariableDescription {
std::optional<std::string> custom_name = {});
/// Returns a description of the value appropriate for the specified context.
- std::string GetResult(llvm::StringRef context);
+ std::string GetResult(protocol::EvaluateContext context);
};
/// Does the given variable have an associated value location?
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 44ae79f8b9f43..82a461dad4451 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -8,6 +8,7 @@
#include "Protocol/ProtocolRequests.h"
#include "JSONUtils.h"
+#include "Protocol/ProtocolTypes.h"
#include "lldb/lldb-defines.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringMap.h"
@@ -639,6 +640,56 @@ json::Value toJSON(const ExceptionInfoResponseBody &ERB) {
result.insert({"description", ERB.description});
if (ERB.details.has_value())
result.insert({"details", *ERB.details});
+ return result;
+}
+
+static bool fromJSON(const llvm::json::Value &Params, EvaluateContext &C,
+ llvm::json::Path P) {
+ auto rawContext = Params.getAsString();
+ if (!rawContext) {
+ P.report("expected a string");
+ return false;
+ }
+ C = StringSwitch<EvaluateContext>(*rawContext)
+ .Case("watch", EvaluateContext::eEvaluateContextWatch)
+ .Case("repl", EvaluateContext::eEvaluateContextRepl)
+ .Case("hover", EvaluateContext::eEvaluateContextHover)
+ .Case("clipboard", EvaluateContext::eEvaluateContextClipboard)
+ .Case("variables", EvaluateContext::eEvaluateContextVariables)
+ .Default(eEvaluateContextUnknown);
+ return true;
+}
+
+bool fromJSON(const llvm::json::Value &Params, EvaluateArguments &Args,
+ llvm::json::Path P) {
+ json::ObjectMapper O(Params, P);
+ return O && O.map("expression", Args.expression) &&
+ O.mapOptional("frameId", Args.frameId) &&
+ O.mapOptional("line", Args.line) &&
+ O.mapOptional("column", Args.column) &&
+ O.mapOptional("source", Args.source) &&
+ O.mapOptional("context", Args.context) &&
+ O.mapOptional("format", Args.format);
+}
+
+llvm::json::Value toJSON(const EvaluateResponseBody &Body) {
+ json::Object result{{"result", Body.result}};
+
+ if (!Body.type.empty())
+ result.insert({"type", Body.type});
+ if (Body.presentationHint)
+ result.insert({"presentationHint", Body.presentationHint});
+ if (Body.variablesReference != 0 &&
+ Body.variablesReference != LLDB_DAP_INVALID_VARRERF)
+ result.insert({"variablesReference", Body.variablesReference});
+ if (Body.namedVariables)
+ result.insert({"namedVariables", Body.namedVariables});
+ if (Body.indexedVariables)
+ result.insert({"indexedVariables", Body.indexedVariables});
+ if (!Body.memoryReference.empty())
+ result.insert({"memoryReference", Body.memoryReference});
+ if (Body.valueLocationReference ...
[truncated]
|
da-viper
approved these changes
Nov 13, 2025
da-viper
reviewed
Nov 13, 2025
🐧 Linux x64 Test Results
|
ashgti
added a commit
to ashgti/llvm-project
that referenced
this pull request
Nov 17, 2025
Updated the evaluate handler to check for DAP ErrorResponse bodies, which are used to display user errors if a request fails. This was updated in PR llvm#167720
ashgti
added a commit
that referenced
this pull request
Nov 18, 2025
…68430) Updated the evaluate handler to check for DAP ErrorResponse bodies, which are used to display user errors if a request fails. This was updated in PR #167720 This should fix https://lab.llvm.org/buildbot/#/builders/163
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Nov 18, 2025
…quests. (#168430) Updated the evaluate handler to check for DAP ErrorResponse bodies, which are used to display user errors if a request fails. This was updated in PR llvm/llvm-project#167720 This should fix https://lab.llvm.org/buildbot/#/builders/163
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding structured types for the evaluate request handler.
This should be mostly a non-functional change. I did catch some spelling mistakes in our tests ('variable' vs 'variables').