Narrow the CompletionRequest API to being append-only.

Summary:
We currently allow any completion handler to read and manipulate the list of matches we
calculated so far. This leads to a few problems:

Firstly, a completion handler's logic can now depend on previously calculated results
by another handlers. No completion handler should have such an implicit dependency,
but the current API makes it likely that this could happen (or already happens). Especially
the fact that some completion handler deleted all previously calculated results can mess
things up right now.

Secondly, all completion handlers have knowledge about our internal data structures with
this API. This makes refactoring this internal data structure much harder than it should be.
Especially planned changes like the support of descriptions for completions are currently
giant patches because we have to refactor every single completion handler.

This patch narrows the contract the CompletionRequest has with the different handlers to:

1. A handler can suggest a completion.
2. A handler can ask how many suggestions we already have.

Point 2 obviously means we still have a  dependency left between the different handlers, but
getting rid of this is too large to just append it to this patch.

Otherwise this patch just completely hides the internal StringList to the different handlers.

The CompletionRequest API now also ensures that the list of completions is unique and we
don't suggest the same value multiple times to the user. This property has been so far only
been ensured by the `Option` handler, but is now applied globally. This is part of this patch
as the OptionHandler is no longer able to implement this functionality itself.

Reviewers: jingham, davide, labath

Reviewed By: davide

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D49322

git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@338151 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 14 insertions, 12 deletions

diff --git a/source/Commands/CommandObjectMultiword.cpp b/source/Commands/CommandObjectMultiword.cpp
index ade1a2d01..19fcf60e5 100644
--- a/source/Commands/CommandObjectMultiword.cpp
+++ b/source/Commands/CommandObjectMultiword.cpp
@@ -143,7 +143,7 @@ bool CommandObjectMultiword::Execute(const char *args_string,
 
   if (num_subcmd_matches > 0) {
     error_msg.append(" Possible completions:");
-    for (size_t i = 0; i < num_subcmd_matches; i++) {
+    for (size_t i = 0; i < matches.GetSize(); i++) {
       error_msg.append("\n\t");
       error_msg.append(matches.GetStringAtIndex(i));
     }
@@ -190,21 +190,22 @@ int CommandObjectMultiword::HandleCompletion(CompletionRequest &request) {
   // Any of the command matches will provide a complete word, otherwise the
   // individual completers will override this.
   request.SetWordComplete(true);
-  auto &matches = request.GetMatches();
 
   auto arg0 = request.GetParsedLine()[0].ref;
   if (request.GetCursorIndex() == 0) {
-    AddNamesMatchingPartialString(m_subcommand_dict, arg0, matches);
+    StringList new_matches;
+    AddNamesMatchingPartialString(m_subcommand_dict, arg0, new_matches);
+    request.AddCompletions(new_matches);
 
-    if (matches.GetSize() == 1 && matches.GetStringAtIndex(0) != nullptr &&
-        (arg0 == matches.GetStringAtIndex(0))) {
+    if (new_matches.GetSize() == 1 &&
+        new_matches.GetStringAtIndex(0) != nullptr &&
+        (arg0 == new_matches.GetStringAtIndex(0))) {
       StringList temp_matches;
       CommandObject *cmd_obj = GetSubcommandObject(arg0, &temp_matches);
       if (cmd_obj != nullptr) {
         if (request.GetParsedLine().GetArgumentCount() == 1) {
           request.SetWordComplete(true);
         } else {
-          matches.DeleteStringAtIndex(0);
           request.GetParsedLine().Shift();
           request.SetCursorCharPosition(0);
           request.GetParsedLine().AppendArgument(llvm::StringRef());
@@ -212,14 +213,17 @@ int CommandObjectMultiword::HandleCompletion(CompletionRequest &request) {
         }
       }
     }
-    return matches.GetSize();
+    return new_matches.GetSize();
   } else {
-    CommandObject *sub_command_object = GetSubcommandObject(arg0, &matches);
+    StringList new_matches;
+    CommandObject *sub_command_object = GetSubcommandObject(arg0, &new_matches);
     if (sub_command_object == nullptr) {
-      return matches.GetSize();
+      request.AddCompletions(new_matches);
+      return request.GetNumberOfMatches();
     } else {
       // Remove the one match that we got from calling GetSubcommandObject.
-      matches.DeleteStringAtIndex(0);
+      new_matches.DeleteStringAtIndex(0);
+      request.AddCompletions(new_matches);
       request.GetParsedLine().Shift();
       request.SetCursorIndex(request.GetCursorIndex() - 1);
       return sub_command_object->HandleCompletion(request);
@@ -366,7 +370,6 @@ int CommandObjectProxy::HandleCompletion(CompletionRequest &request) {
   CommandObject *proxy_command = GetProxyCommandObject();
   if (proxy_command)
     return proxy_command->HandleCompletion(request);
-  request.GetMatches().Clear();
   return 0;
 }
 
@@ -375,7 +378,6 @@ int CommandObjectProxy::HandleArgumentCompletion(
   CommandObject *proxy_command = GetProxyCommandObject();
   if (proxy_command)
     return proxy_command->HandleArgumentCompletion(request, opt_element_vector);
-  request.GetMatches().Clear();
   return 0;
 }