[Lldb-commits] [lldb] 34885bf - [lldb-vscode] Handle request_evaluate's context attribute
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 15 15:09:48 PDT 2021
Author: Walter Erquinigo
Date: 2021-03-15T15:09:23-07:00
New Revision: 34885bffdf43920c0f011e17a65fd678100240dd
URL: https://github.com/llvm/llvm-project/commit/34885bffdf43920c0f011e17a65fd678100240dd
DIFF: https://github.com/llvm/llvm-project/commit/34885bffdf43920c0f011e17a65fd678100240dd.diff
LOG: [lldb-vscode] Handle request_evaluate's context attribute
Summary:
The request "evaluate" supports a "context" attribute, which is sent by VSCode. The attribute is defined here https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Evaluate
The "clipboard" context is not yet supported by lldb-vscode, so we can forget about it for now. The 'repl' (i.e. Debug Console) and 'watch' (i.e. Watch Expression) contexts must use the expression parser in case the frame's variable path is not enough, as the user expects these expressions to never fail. On the other hand, the 'hover' expression is invoked whenever the user hovers on any keyword on the UI and the user is fine with the expression not being fully resolved, as they know that the 'repl' case is the fallback they can rely on.
Given that the 'hover' expression is invoked many many times without the user noticing it due to it being triggered by the mouse, I'm making it use only the frame's variable path functionality and not the expression parser. This should speed up tremendously the responsiveness of a debug session when the user only sets source breakpoints and inspect local variables, as the entire debug info is not needed to be parsed.
Regarding tests, I've tried to be as comprehensive as possible considering a multi-file project. Fortunately, the results from the "hover" case are enough most of the times.
Differential Revision: https://reviews.llvm.org/D98656
Added:
lldb/test/API/tools/lldb-vscode/evaluate/Makefile
lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
lldb/test/API/tools/lldb-vscode/evaluate/foo.cpp
lldb/test/API/tools/lldb-vscode/evaluate/foo.h
lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
Modified:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/lldb-vscode.cpp
Removed:
################################################################################
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 1582115ee40b..5a433f2baac2 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -139,7 +139,7 @@ def get_modules(self):
for module in module_list:
modules[module['name']] = module
return modules
-
+
def get_output(self, category, timeout=0.0, clear=True):
self.output_condition.acquire()
output = None
@@ -304,7 +304,7 @@ def send_recv(self, command):
return response_or_request
else:
if response_or_request['command'] == 'runInTerminal':
- subprocess.Popen(response_or_request['arguments']['args'],
+ subprocess.Popen(response_or_request['arguments']['args'],
env=response_or_request['arguments']['env'])
self.send_packet({
"type": "response",
@@ -317,7 +317,7 @@ def send_recv(self, command):
else:
desc = 'unkonwn reverse request "%s"' % (response_or_request['command'])
raise ValueError(desc)
-
+
return None
def wait_for_event(self, filter=None, timeout=None):
@@ -567,13 +567,14 @@ def request_disconnect(self, terminateDebuggee=None):
}
return self.send_recv(command_dict)
- def request_evaluate(self, expression, frameIndex=0, threadId=None):
+ def request_evaluate(self, expression, frameIndex=0, threadId=None, context=None):
stackFrame = self.get_stackFrame(frameIndex=frameIndex,
threadId=threadId)
if stackFrame is None:
return []
args_dict = {
'expression': expression,
+ 'context': context,
'frameId': stackFrame['id'],
}
command_dict = {
@@ -787,7 +788,7 @@ def request_compileUnits(self, moduleId):
}
response = self.send_recv(command_dict)
return response
-
+
def request_completions(self, text):
args_dict = {
'text': text,
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/Makefile b/lldb/test/API/tools/lldb-vscode/evaluate/Makefile
new file mode 100644
index 000000000000..7df22699c57d
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp foo.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
new file mode 100644
index 000000000000..f496073d1b6d
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
@@ -0,0 +1,157 @@
+"""
+Test lldb-vscode completions request
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def assertEvaluate(self, expression, regex):
+ self.assertRegexpMatches(
+ self.vscode.request_evaluate(expression, context=self.context)['body']['result'],
+ regex)
+
+ def assertEvaluateFailure(self, expression):
+ self.assertNotIn('result',
+ self.vscode.request_evaluate(expression, context=self.context)['body'])
+
+ def isExpressionParsedExpected(self):
+ return self.context != "hover"
+
+ def run_test_evaluate_expressions(self, context=None):
+ """
+ Tests the evaluate expression request at
diff erent breakpoints
+ """
+ self.context = context
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ self.set_source_breakpoints(
+ source,
+ [
+ line_number(source, "// breakpoint 1"),
+ line_number(source, "// breakpoint 2"),
+ line_number(source, "// breakpoint 3")
+ ]
+ )
+ self.continue_to_next_stop()
+
+ # Expressions at breakpoint 1, which is in main
+ self.assertEvaluate("var1", "20")
+ self.assertEvaluate("var2", "21")
+ self.assertEvaluate("static_int", "42")
+ self.assertEvaluate("non_static_int", "43")
+ self.assertEvaluate("struct1", "my_struct @ 0x.*")
+ self.assertEvaluate("struct1.foo", "15")
+ self.assertEvaluate("struct2->foo", "16")
+
+ self.assertEvaluateFailure("var") # local variable of a_function
+ self.assertEvaluateFailure("my_struct") # type name
+ self.assertEvaluateFailure("int") # type name
+ self.assertEvaluateFailure("foo") # member of my_struct
+
+ if self.isExpressionParsedExpected():
+ self.assertEvaluate(
+ "a_function",
+ "0x.* \(a.out`a_function\(int\) at main.cpp:7\)")
+ self.assertEvaluate("a_function(1)", "1")
+ self.assertEvaluate("var2 + struct1.foo", "36")
+ self.assertEvaluate(
+ "foo_func",
+ "0x.* \(a.out`foo_func\(\) at foo.cpp:3\)")
+ self.assertEvaluate("foo_var", "44")
+ else:
+ self.assertEvaluateFailure("a_function")
+ self.assertEvaluateFailure("a_function(1)")
+ self.assertEvaluateFailure("var2 + struct1.foo")
+ self.assertEvaluateFailure("foo_func")
+ self.assertEvaluateFailure("foo_var")
+
+ # Expressions at breakpoint 2, which is an anonymous block
+ self.continue_to_next_stop()
+ self.assertEvaluate("var1", "20")
+ self.assertEvaluate("var2", "2") #
diff erent variable with the same name
+ self.assertEvaluate("static_int", "42")
+ self.assertEvaluate("non_static_int", "10") #
diff erent variable with the same name
+ self.assertEvaluate("struct1", "my_struct @ 0x.*")
+ self.assertEvaluate("struct1.foo", "15")
+ self.assertEvaluate("struct2->foo", "16")
+
+ if self.isExpressionParsedExpected():
+ self.assertEvaluate(
+ "a_function",
+ "0x.* \(a.out`a_function\(int\) at main.cpp:7\)")
+ self.assertEvaluate("a_function(1)", "1")
+ self.assertEvaluate("var2 + struct1.foo", "17")
+ self.assertEvaluate(
+ "foo_func",
+ "0x.* \(a.out`foo_func\(\) at foo.cpp:3\)")
+ self.assertEvaluate("foo_var", "44")
+ else:
+ self.assertEvaluateFailure("a_function")
+ self.assertEvaluateFailure("a_function(1)")
+ self.assertEvaluateFailure("var2 + struct1.foo")
+ self.assertEvaluateFailure("foo_func")
+ self.assertEvaluateFailure("foo_var")
+
+ # Expressions at breakpoint 3, which is inside a_function
+ self.continue_to_next_stop()
+ self.assertEvaluate("var", "42")
+ self.assertEvaluate("static_int", "42")
+ self.assertEvaluate("non_static_int", "43")
+
+ self.assertEvaluateFailure("var1")
+ self.assertEvaluateFailure("var2")
+ self.assertEvaluateFailure("struct1")
+ self.assertEvaluateFailure("struct1.foo")
+ self.assertEvaluateFailure("struct2->foo")
+ self.assertEvaluateFailure("var2 + struct1.foo")
+
+ if self.isExpressionParsedExpected():
+ self.assertEvaluate(
+ "a_function",
+ "0x.* \(a.out`a_function\(int\) at main.cpp:7\)")
+ self.assertEvaluate("a_function(1)", "1")
+ self.assertEvaluate("var + 1", "43")
+ self.assertEvaluate(
+ "foo_func",
+ "0x.* \(a.out`foo_func\(\) at foo.cpp:3\)")
+ self.assertEvaluate("foo_var", "44")
+ else:
+ self.assertEvaluateFailure("a_function")
+ self.assertEvaluateFailure("a_function(1)")
+ self.assertEvaluateFailure("var + 1")
+ self.assertEvaluateFailure("foo_func")
+ self.assertEvaluateFailure("foo_var")
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_generic_evaluate_expressions(self):
+ # Tests context-less expression evaluations
+ self.run_test_evaluate_expressions()
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_repl_evaluate_expressions(self):
+ # Tests expression evaluations that are triggered from the Debug Console
+ self.run_test_evaluate_expressions("repl")
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_watch_evaluate_expressions(self):
+ # Tests expression evaluations that are triggered from a watch expression
+ self.run_test_evaluate_expressions("watch")
+
+ @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")
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/foo.cpp b/lldb/test/API/tools/lldb-vscode/evaluate/foo.cpp
new file mode 100644
index 000000000000..b608d71f9884
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/foo.cpp
@@ -0,0 +1,5 @@
+#include "foo.h"
+
+int foo_func() { return 43; }
+
+int foo_var = 44;
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/foo.h b/lldb/test/API/tools/lldb-vscode/evaluate/foo.h
new file mode 100644
index 000000000000..7c1925d115ab
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/foo.h
@@ -0,0 +1,3 @@
+int foo_func();
+
+extern int foo_var;
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
new file mode 100644
index 000000000000..564660da7119
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
@@ -0,0 +1,29 @@
+#include "foo.h"
+
+static int static_int = 42;
+
+int non_static_int = 43;
+
+int a_function(int var) {
+ return var; // breakpoint 3
+}
+
+struct my_struct {
+ int foo;
+};
+
+int main(int argc, char const *argv[]) {
+ my_struct struct1 = {15};
+ my_struct *struct2 = new my_struct{16};
+ int var1 = 20;
+ int var2 = 21;
+ int var3 = static_int; // breakpoint 1
+ {
+ int non_static_int = 10;
+ int var2 = 2;
+ int var3 = non_static_int; // breakpoint 2
+ }
+ a_function(var3);
+ foo_func();
+ return 0;
+}
diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 9469690cd7db..1097decbf556 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1125,6 +1125,7 @@ void request_evaluate(const llvm::json::Object &request) {
auto arguments = request.getObject("arguments");
lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
const auto expression = GetString(arguments, "expression");
+ llvm::StringRef context = GetString(arguments, "context");
if (!expression.empty() && expression[0] == '`') {
auto result =
@@ -1133,13 +1134,17 @@ void request_evaluate(const llvm::json::Object &request) {
body.try_emplace("variablesReference", (int64_t)0);
} else {
// Always try to get the answer from the local variables if possible. If
- // this fails, then actually evaluate an expression using the expression
- // parser. "frame variable" is more reliable than the expression parser in
+ // 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);
- if (value.GetError().Fail())
+
+ 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
More information about the lldb-commits
mailing list