[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.

Author: vogelsgesang

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 = {

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 different 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,
+            ],
+        )

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.");

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,