[lldb-dap] Correct auto-completion based on ReplMode and escape char #110784
+226
−83
Conversation
This file contains 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
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.
|
@llvm/pr-subscribers-lldb Author: Adrian Vogelsgesang (vogelsgesang) ChangesThis 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:
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:
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();
|
vogelsgesang
requested review from
walter-erquinigo,
cmtice,
ashgti and
jeffreytan81
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
walter-erquinigo
approved these changes
Oct 2, 2024
ashgti
reviewed
Oct 2, 2024
lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
Outdated
Show resolved
Hide resolved
ashgti
approved these changes
Oct 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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:
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 typingla, 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.