[Lldb-commits] [lldb] a5876be - [lldb-dap] Correct auto-completion based on ReplMode and escape char (#110784)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 2 17:50:49 PDT 2024
Author: Adrian Vogelsgesang
Date: 2024-10-03T02:50:46+02:00
New Revision: a5876bef61e44453b915e1f79366ca1bbfdba371
URL: https://github.com/llvm/llvm-project/commit/a5876bef61e44453b915e1f79366ca1bbfdba371
DIFF: https://github.com/llvm/llvm-project/commit/a5876bef61e44453b915e1f79366ca1bbfdba371.diff
LOG: [lldb-dap] Correct auto-completion based on ReplMode and escape char (#110784)
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.
Added:
Modified:
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py
lldb/tools/lldb-dap/DAP.cpp
lldb/tools/lldb-dap/DAP.h
lldb/tools/lldb-dap/README.md
lldb/tools/lldb-dap/lldb-dap.cpp
Removed:
################################################################################
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..8ad12b3fc2f6b1 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,28 @@
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 +40,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
diff erent breakpoints
- """
+
+ def setup_debugee(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
@@ -32,90 +50,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=variable"
+ """
+ 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 +222,45 @@ 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..5e75d84cf8243e 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -483,20 +483,20 @@ 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 +509,12 @@ 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..ba6d3d80410e3d 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 modified 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..edbbd9bc5f9539 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1395,9 +1395,20 @@ 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 +1420,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;
@@ -1571,10 +1582,10 @@ void request_evaluate(const llvm::json::Object &request) {
bool repeat_last_command =
expression.empty() && g_dap.last_nonempty_var_expression.empty();
- if (context == "repl" && (repeat_last_command ||
- (!expression.empty() &&
- g_dap.DetectExpressionContext(frame, expression) ==
- ExpressionContext::Command))) {
+ if (context == "repl" &&
+ (repeat_last_command ||
+ (!expression.empty() &&
+ 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();
More information about the lldb-commits
mailing list