[Lldb-commits] [lldb] [lldb-dap] Correct auto-completion based on ReplMode and escape char (PR #110784)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 1 19:42:42 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Adrian Vogelsgesang (vogelsgesang)
<details>
<summary>Changes</summary>
This commit improves the auto-completion in the Debug Console provided by VS-Code.
So far, we were always suggesting completions for both LLDB commands and for variables / expressions, even if the heuristic already determined how the given string will be executed, e.g., because the user explicitly typed the escape prefix. Furthermore, auto-completion after the escape character was broken, since the offsets were not adjusted correctly. With this commit we now correctly take this into account.
Even with this commit, auto-completion does not always work reliably:
* VS Code only requests auto-completion after typing the first alphabetic character, but not after punctuation characters. This means that no completions are provided after typing "`"
* LLDB does not provide autocompletions if a string is an exact match. This means if a user types `l` (which is a valid command), LLDB will not provide "language" and "log" as potential completions. Even worse, VS Code caches the completion and does client-side filtering. Hence, even after typing `la`, no auto-completion for "language" is shown in the UI.
Those issues might be fixed in follow-up commits. Also with those known issues, the experience is already much better with this commit.
Furthermore, I updated the README since I noticed that it was slightly inaccurate.
---
Full diff: https://github.com/llvm/llvm-project/pull/110784.diff
6 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+1-1)
- (modified) lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py (+161-51)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+15-9)
- (modified) lldb/tools/lldb-dap/DAP.h (+18-12)
- (modified) lldb/tools/lldb-dap/README.md (+7-3)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+15-6)
``````````diff
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 449af1b67d8022..1d5e6e0d75c7cb 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
@@ -1006,7 +1006,7 @@ def request_compileUnits(self, moduleId):
return response
def request_completions(self, text, frameId=None):
- args_dict = {"text": text, "column": len(text)}
+ args_dict = {"text": text, "column": len(text) + 1}
if frameId:
args_dict["frameId"] = frameId
command_dict = {
diff --git a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
index 2b3ec656c107a5..ae0fe00fec67e8 100644
--- a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
+++ b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
@@ -9,6 +9,25 @@
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
+session_completion = {
+ "text": "session",
+ "label": "session -- Commands controlling LLDB session.",
+}
+settings_completion = {
+ "text": "settings",
+ "label": "settings -- Commands for managing LLDB settings.",
+}
+memory_completion = {
+ "text": "memory",
+ "label": "memory -- Commands for operating on memory in the current target process."
+}
+command_var_completion = {
+ "text": "var",
+ "label": "var -- Show variables for the current stack frame. Defaults to all arguments and local variables in scope. Names of argument, local, file static and file global variables can be specified.",
+}
+variable_var_completion = { "text": "var", "label": "var -- vector<baz> &", }
+variable_var1_completion = {"text": "var1", "label": "var1 -- int &"}
+variable_var2_completion = {"text": "var2", "label": "var2 -- int &"}
class TestDAP_completions(lldbdap_testcase.DAPTestCaseBase):
def verify_completions(self, actual_list, expected_list, not_expected_list=[]):
@@ -18,12 +37,8 @@ def verify_completions(self, actual_list, expected_list, not_expected_list=[]):
for not_expected_item in not_expected_list:
self.assertNotIn(not_expected_item, actual_list)
- @skipIfWindows
- @skipIf(compiler="clang", compiler_version=["<", "17.0"])
- def test_completions(self):
- """
- Tests the completion request at different breakpoints
- """
+
+ def setup_debugee(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
@@ -32,90 +47,146 @@ def test_completions(self):
breakpoint2_line = line_number(source, "// breakpoint 2")
self.set_source_breakpoints(source, [breakpoint1_line, breakpoint2_line])
+
+
+ def test_command_completions(self):
+ """
+ Tests completion requests for lldb commands, within "repl-mode=command"
+ """
+ self.setup_debugee()
self.continue_to_next_stop()
- # shouldn't see variables inside main
+ res = self.dap_server.request_evaluate("`lldb-dap repl-mode command", context="repl")
+ self.assertTrue(res["success"])
+
+ # Provides completion for top-level commands
self.verify_completions(
- self.dap_server.get_completions("var"),
+ self.dap_server.get_completions("se"),
+ [session_completion, settings_completion]
+ )
+
+ # Provides completions for sub-commands
+ self.verify_completions(
+ self.dap_server.get_completions("memory "),
[
{
- "text": "var",
- "label": "var -- vector<baz> &",
+ "text": "read",
+ "label": "read -- Read from the memory of the current target process."
},
{
- "text": "var",
- "label": "var -- Show variables for the current stack frame. Defaults to all arguments and local variables in scope. Names of argument, local, file static and file global variables can be specified.",
+ "text": "region",
+ "label": "region -- Get information on the memory region containing an address in the current target process.",
},
- ],
- [
- {"text": "var1", "label": "var1 -- int &"},
- ],
+ ]
)
- # should see global keywords but not variables inside main
+ # Provides completions for parameter values of commands
self.verify_completions(
- self.dap_server.get_completions("str"),
- [{"text": "struct", "label": "struct"}],
- [{"text": "str1", "label": "str1 -- std::string &"}],
+ self.dap_server.get_completions("`log enable "),
+ [{"text": "gdb-remote", "label": "gdb-remote"}],
)
- self.continue_to_next_stop()
+ # Also works if the escape prefix is used
+ self.verify_completions(
+ self.dap_server.get_completions("`mem"),
+ [memory_completion]
+ )
- # should see variables from main but not from the other function
self.verify_completions(
- self.dap_server.get_completions("var"),
- [
- {"text": "var1", "label": "var1 -- int &"},
- {"text": "var2", "label": "var2 -- int &"},
- ],
+ self.dap_server.get_completions("`"),
+ [session_completion, settings_completion, memory_completion]
+ )
+
+
+ # Completes an incomplete quoted token
+ self.verify_completions(
+ self.dap_server.get_completions('setting "se'),
[
{
- "text": "var",
- "label": "var -- vector<baz> &",
+ "text": "set",
+ "label": "set -- Set the value of the specified debugger setting.",
}
],
)
+ # Completes an incomplete quoted token
self.verify_completions(
- self.dap_server.get_completions("str"),
- [
- {"text": "struct", "label": "struct"},
- {"text": "str1", "label": "str1 -- string &"},
- ],
+ self.dap_server.get_completions("'mem"),
+ [memory_completion],
)
- # should complete arbitrary commands including word starts
+ # Completes expressions with quotes inside
self.verify_completions(
- self.dap_server.get_completions("`log enable "),
- [{"text": "gdb-remote", "label": "gdb-remote"}],
+ self.dap_server.get_completions('expr " "; typed'),
+ [{"text": "typedef", "label": "typedef"}],
)
- # should complete expressions with quotes inside
+ # Provides completions for commands, but not variables
self.verify_completions(
- self.dap_server.get_completions('`expr " "; typed'),
- [{"text": "typedef", "label": "typedef"}],
+ self.dap_server.get_completions("var"),
+ [command_var_completion],
+ [variable_var_completion],
+ )
+
+
+ def test_variable_completions(self):
+ """
+ Tests completion requests in "repl-mode=command"
+ """
+ self.setup_debugee()
+ self.continue_to_next_stop()
+
+ res = self.dap_server.request_evaluate("`lldb-dap repl-mode variable", context="repl")
+ self.assertTrue(res["success"])
+
+ # Provides completions for varibles, but not command
+ self.verify_completions(
+ self.dap_server.get_completions("var"),
+ [variable_var_completion],
+ [command_var_completion],
)
- # should complete an incomplete quoted token
+ # We stopped inside `fun`, so we shouldn't see variables from main
self.verify_completions(
- self.dap_server.get_completions('`setting "se'),
+ self.dap_server.get_completions("var"),
+ [variable_var_completion],
[
- {
- "text": "set",
- "label": "set -- Set the value of the specified debugger setting.",
- }
+ variable_var1_completion,
+ variable_var2_completion,
],
)
+
+ # We should see global keywords but not variables inside main
self.verify_completions(
- self.dap_server.get_completions("`'comm"),
+ self.dap_server.get_completions("str"),
+ [{"text": "struct", "label": "struct"}],
+ [{"text": "str1", "label": "str1 -- std::string &"}],
+ )
+
+ self.continue_to_next_stop()
+
+ # We stopped in `main`, so we should see variables from main but
+ # not from the other function
+ self.verify_completions(
+ self.dap_server.get_completions("var"),
[
- {
- "text": "command",
- "label": "command -- Commands for managing custom LLDB commands.",
- }
+ variable_var1_completion,
+ variable_var2_completion,
+ ],
+ [
+ variable_var_completion,
],
)
+ self.verify_completions(
+ self.dap_server.get_completions("str"),
+ [
+ {"text": "struct", "label": "struct"},
+ {"text": "str1", "label": "str1 -- string &"},
+ ],
+ )
+
+ # Completion also works for more complex expressions
self.verify_completions(
self.dap_server.get_completions("foo1.v"),
[{"text": "var1", "label": "foo1.var1 -- int"}],
@@ -148,3 +219,42 @@ def test_completions(self):
[{"text": "var1", "label": "var1 -- int"}],
[{"text": "var2", "label": "var2 -- int"}],
)
+
+ # Even in variable mode, we can still use the escape prefix
+ self.verify_completions(
+ self.dap_server.get_completions("`mem"),
+ [memory_completion]
+ )
+
+ def test_auto_completions(self):
+ """
+ Tests completion requests in "repl-mode=auto"
+ """
+ self.setup_debugee()
+
+ res = self.dap_server.request_evaluate("`lldb-dap repl-mode auto", context="repl")
+ self.assertTrue(res["success"])
+
+ self.continue_to_next_stop()
+ self.continue_to_next_stop()
+
+ # We are stopped inside `main`. Variables `var1` and `var2` are in scope.
+ # Make sure, we offer all completions
+ self.verify_completions(
+ self.dap_server.get_completions("va"),
+ [
+ command_var_completion,
+ variable_var1_completion,
+ variable_var2_completion,
+ ],
+ )
+
+ # If we are using the escape prefix, only commands are suggested, but no variables
+ self.verify_completions(
+ self.dap_server.get_completions("`va"),
+ [ command_var_completion, ],
+ [
+ variable_var1_completion,
+ variable_var2_completion,
+ ],
+ )
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 884a71ff6693f8..75fe97802cfa67 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -483,20 +483,19 @@ llvm::json::Value DAP::CreateTopLevelScopes() {
return llvm::json::Value(std::move(scopes));
}
-ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame frame,
- std::string &expression) {
- // Include the escape hatch prefix.
+ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression, bool partial_expression) {
+ // Check for the escape hatch prefix.
if (!expression.empty() &&
llvm::StringRef(expression).starts_with(g_dap.command_escape_prefix)) {
expression = expression.substr(g_dap.command_escape_prefix.size());
- return ExpressionContext::Command;
+ return ReplMode::Command;
}
switch (repl_mode) {
case ReplMode::Variable:
- return ExpressionContext::Variable;
+ return ReplMode::Variable;
case ReplMode::Command:
- return ExpressionContext::Command;
+ return ReplMode::Command;
case ReplMode::Auto:
// To determine if the expression is a command or not, check if the first
// term is a variable or command. If it's a variable in scope we will prefer
@@ -509,6 +508,13 @@ ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame frame,
// int var and expression "va" > command
std::pair<llvm::StringRef, llvm::StringRef> token =
llvm::getToken(expression);
+
+ // If the first token is not fully finished yet, we can't
+ // determine whether this will be a variable or a lldb command.
+ if (partial_expression && token.second.empty()) {
+ return ReplMode::Auto;
+ }
+
std::string term = token.first.str();
lldb::SBCommandInterpreter interpreter = debugger.GetCommandInterpreter();
bool term_is_command = interpreter.CommandExists(term.c_str()) ||
@@ -527,9 +533,9 @@ ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame frame,
// Variables take preference to commands in auto, since commands can always
// be called using the command_escape_prefix
- return term_is_variable ? ExpressionContext::Variable
- : term_is_command ? ExpressionContext::Command
- : ExpressionContext::Variable;
+ return term_is_variable ? ReplMode::Variable
+ : term_is_command ? ReplMode::Command
+ : ReplMode::Variable;
}
llvm_unreachable("enum cases exhausted.");
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57719f98d2aa17..fa24138149cc3c 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -94,12 +94,6 @@ enum class PacketStatus {
enum class ReplMode { Variable = 0, Command, Auto };
-/// The detected context of an expression based off the current repl mode.
-enum class ExpressionContext {
- Variable = 0,
- Command,
-};
-
struct Variables {
/// Variable_reference start index of permanent expandable variable.
static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);
@@ -245,12 +239,24 @@ struct DAP {
void PopulateExceptionBreakpoints();
- /// \return
- /// Attempt to determine if an expression is a variable expression or
- /// lldb command using a hueristic based on the first term of the
- /// expression.
- ExpressionContext DetectExpressionContext(lldb::SBFrame frame,
- std::string &expression);
+ /// Attempt to determine if an expression is a variable expression or
+ /// lldb command using a heuristic based on the first term of the
+ /// expression.
+ ///
+ /// \param[in] frame
+ /// The frame, used as context to detect local variable names
+ /// \param[inout] expression
+ /// The expression string. Might be modifuied by this function to
+ /// remove the leading escape character.
+ /// \param[in] partial_expression
+ /// Whether the provided `expression` is only a prefix of the
+ /// final expression. If `true`, this function might return
+ /// `ReplMode::Auto` to indicate that the expression could be
+ /// either an expression or a statement, depending on the rest of
+ /// the expression.
+ /// \return the expression mode
+ ReplMode DetectReplMode(lldb::SBFrame frame, std::string &expression,
+ bool partial_expression);
/// \return
/// \b false if a fatal error was found while executing these commands,
diff --git a/lldb/tools/lldb-dap/README.md b/lldb/tools/lldb-dap/README.md
index c3bed593154e02..0c1a5a5ef344ac 100644
--- a/lldb/tools/lldb-dap/README.md
+++ b/lldb/tools/lldb-dap/README.md
@@ -228,9 +228,13 @@ the following `lldb-dap` specific key/value pairs:
## Debug Console
The debug console allows printing variables / expressions and executing lldb commands.
-By default, all provided commands are interpreteted as variable names / expressions whose values will be printed to the Debug Console.
-To execute regular LLDB commands, prefix them with the `\`` character.
-The escape character can be changed via the `commandEscapePrefix` configuration option.
+By default, lldb-dap tries to auto-detect whether a provided commands is a variable
+name / expressions whose values will be printed to the Debug Console or a LLDB command.
+To side-step this auto-dection and execute a LLDB command, prefix it with the `\``
+character.
+
+The auto-detection can be disabled using the `lldb-dap repl-mode` command.
+The escape character can be adjusted via the `commandEscapePrefix` configuration option.
### lldb-dap specific commands
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index db4dbbd6f6200a..93811056a74c69 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1395,9 +1395,18 @@ void request_completions(const llvm::json::Object &request) {
}
llvm::json::Array targets;
- if (!text.empty() &&
- llvm::StringRef(text).starts_with(g_dap.command_escape_prefix)) {
- text = text.substr(g_dap.command_escape_prefix.size());
+ bool had_escape_prefix = llvm::StringRef(text).starts_with(g_dap.command_escape_prefix);
+ ReplMode completion_mode = g_dap.DetectReplMode(frame, text, true);
+
+ // Handle the offset change introduced by stripping out the `command_escape_prefix`.
+ if (had_escape_prefix) {
+ if (offset < g_dap.command_escape_prefix.size()) {
+ body.try_emplace("targets", std::move(targets));
+ response.try_emplace("body", std::move(body));
+ g_dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+ offset -= g_dap.command_escape_prefix.size();
}
// While the user is typing then we likely have an incomplete input and cannot
@@ -1409,7 +1418,7 @@ void request_completions(const llvm::json::Object &request) {
std::make_tuple(ReplMode::Variable, expr_prefix + text,
offset + expr_prefix.size())}};
for (const auto &[mode, line, cursor] : exprs) {
- if (g_dap.repl_mode != ReplMode::Auto && g_dap.repl_mode != mode)
+ if (completion_mode != ReplMode::Auto && completion_mode != mode)
continue;
lldb::SBStringList matches;
@@ -1573,8 +1582,8 @@ void request_evaluate(const llvm::json::Object &request) {
if (context == "repl" && (repeat_last_command ||
(!expression.empty() &&
- g_dap.DetectExpressionContext(frame, expression) ==
- ExpressionContext::Command))) {
+ g_dap.DetectReplMode(frame, expression, false) ==
+ ReplMode::Command))) {
// Since the current expression is not for a variable, clear the
// last_nonempty_var_expression field.
g_dap.last_nonempty_var_expression.clear();
``````````
</details>
https://github.com/llvm/llvm-project/pull/110784
More information about the lldb-commits
mailing list