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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
