[lldb-vscode] disable completions

[wallace]

Summary

Completion requests are causing some problems in the debugger, which is explained in the comment in the code.
I'm disabling it for now until we have time to do a good implementation of it.

[clayborg]

These completion requests are also expected to be asynchronous right? Do we have any support for this right now? Currently we will deadlock in lldb-vscode handling the current completion until it completes right? To properly handle completion, we will need to be able to cancel a current completion and only remember the last completion that was sent and throw away any intermediate completions. Did we add support for that already, or do we read the first completion command, and then block until it completes and return the response, then handle any others.

IIUC if we get a completion for: "hello", we will go off and start trying to complete this, we might, while trying to complete "hello" get new requests in for "hello w" and "hello wo" and "hello wor" and "hello worl" queued up. If we have no async support, we will need to mark such packets as asynchronous, add the ability to cancel a completion if we don't already have that in the SBCommandInterpreter::HandleCompletion(...) functions, and then only remember the last unhandled completion "hello worl" in this case and respond with error to the first "hello" (canceled due to new completion that came in), "hello w", "hello wo" and "hello wor" since we won't try to complete those because new completions came in after these, and then issue the "hello worl" completion and respond as needed.

We might also think about putting a timeout in the completion code, so we don't even try to complete for a specific amount of time (1 second) so we could watch for new completions to come up. Do we know if the completion was just because someone was typing or if they requested it with CTRL-space?

[wallace]

These completion requests are also expected to be asynchronous right? Do we have any support for this right now? Currently we will deadlock in lldb-vscode handling the current completion until it completes right? To properly handle completion, we will need to be able to cancel a current completion and only remember the last completion that was sent and throw away any intermediate completions. Did we add support for that already, or do we read the first completion command, and then block until it completes and return the response, then handle any others.

Currently we are blocking in lldb-vscode until each completion completes. And as you say, there's no cancellation mechanism at the moment.

IIUC if we get a completion for: "hello", we will go off and start trying to complete this, we might, while trying to complete "hello" get new requests in for "hello w" and "hello wo" and "hello wor" and "hello worl" queued up. If we have no async support, we will need to mark such packets as asynchronous, add the ability to cancel a completion if we don't already have that in the SBCommandInterpreter::HandleCompletion(...) functions, and then only remember the last unhandled completion "hello worl" in this case and respond with error to the first "hello" (canceled due to new completion that came in), "hello w", "hello wo" and "hello wor" since we won't try to complete those because new completions came in after these, and then issue the "hello worl" completion and respond as needed.

I agree with the async support needed for this

We might also think about putting a timeout in the completion code, so we don't even try to complete for a specific amount of time (1 second) so we could watch for new completions to come up. Do we know if the completion was just because someone was typing or if they requested it with CTRL-space?

Timeouts make sense. Besides, we still don't know if the user triggered this manually or not. Hopefully at some point VSCode implements that

lldb/tools/lldb-vscode/lldb-vscode.cpp

void request_initialize(const llvm::json::Object &request) {
   // The debug adapter supports setting a variable to a value.
   body.try_emplace("supportsSetVariable", true);
   // The debug adapter supports restarting a frame.
   body.try_emplace("supportsRestartFrame", false);
   // The debug adapter supports the gotoTargetsRequest.
   body.try_emplace("supportsGotoTargetsRequest", false);
   // The debug adapter supports the stepInTargetsRequest.
   body.try_emplace("supportsStepInTargetsRequest", false);
-  // The debug adapter supports the completionsRequest.
-  body.try_emplace("supportsCompletionsRequest", true);
+  // We need to improve the current implementation of completions in order to
+  // enable it again. For some context, this is how VSCode works: 
+  // - VSCode sends a completion request whenever chars are added, the user
+  //   triggers completion manually via CTRL-space or similar mechanisms, but
+  //   not when there's a deletion. Besides, VSCode doesn't let us know which
+  //   of these events we are handling. What is more, the use can paste or cut
+  //   sections of the text arbitrarily.
+  //   https://github.com/microsoft/vscode/issues/89531 tracks part of the
+  //   issue just mentioned.
+  // This behavior causes many problems with the current way completion is
+  // implemented in lldb-vscode, as these requests could be really expensive,
+  // blocking the debugger, and there could be many concurrent requests unless
+  // the user types very slowly... We need to address this specific issue, or
+  // at least trigger completion only when the user explicitly wants it, which
+  // is the behavior of LLDB CLI, that expects a TAB.
+  body.try_emplace("supportsCompletionsRequest", false);
   // The debug adapter supports the modules request.
   body.try_emplace("supportsModulesRequest", false);
   // The set of additional module information exposed by the debug adapter.
   //   body.try_emplace("additionalModuleColumns"] = ColumnDescriptor
   // Checksum algorithms supported by the debug adapter.
   //   body.try_emplace("supportedChecksumAlgorithms"] = ChecksumAlgorithm
   // The debug adapter supports the RestartRequest. In this case a client
   // should not implement 'restart' by terminating and relaunching the adapter