teemperor created this revision.
teemperor added a reviewer: jingham.

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.


https://reviews.llvm.org/D49322

Files:
  include/lldb/Utility/CompletionRequest.h
  source/Commands/CommandCompletions.cpp
  source/Commands/CommandObjectCommands.cpp
  source/Commands/CommandObjectFrame.cpp
  source/Commands/CommandObjectMultiword.cpp
  source/Commands/CommandObjectPlatform.cpp
  source/Commands/CommandObjectPlugin.cpp
  source/Commands/CommandObjectProcess.cpp
  source/Commands/CommandObjectSettings.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/FormatEntity.cpp
  source/Core/IOHandler.cpp
  source/Interpreter/CommandInterpreter.cpp
  source/Interpreter/CommandObject.cpp
  source/Interpreter/CommandObjectRegexCommand.cpp
  source/Interpreter/OptionValue.cpp
  source/Interpreter/OptionValueArch.cpp
  source/Interpreter/OptionValueBoolean.cpp
  source/Interpreter/OptionValueEnumeration.cpp
  source/Interpreter/OptionValueFileSpec.cpp
  source/Interpreter/OptionValueUUID.cpp
  source/Interpreter/Options.cpp
  source/Symbol/Variable.cpp
  source/Utility/ArchSpec.cpp
  source/Utility/CompletionRequest.cpp
  unittests/Utility/CompletionRequestTest.cpp

Index: unittests/Utility/CompletionRequestTest.cpp
===================================================================
--- unittests/Utility/CompletionRequestTest.cpp
+++ unittests/Utility/CompletionRequestTest.cpp
@@ -34,7 +34,70 @@
 
   EXPECT_EQ(request.GetPartialParsedLine().GetArgumentCount(), 2u);
   EXPECT_STREQ(request.GetPartialParsedLine().GetArgumentAtIndex(1), "b");
+}
+
+TEST(CompletionRequest, DuplicateFiltering) {
+  std::string command = "a bad c";
+  const unsigned cursor_pos = 3;
+  StringList matches;
+
+  CompletionRequest request(command, cursor_pos, 0, 0, matches);
+
+  EXPECT_EQ(0U, request.GetNumberOfMatches());
+
+  // Add foo twice
+  request.AddCompletion("foo");
+  EXPECT_EQ(1U, request.GetNumberOfMatches());
+  EXPECT_EQ(1U, matches.GetSize());
+  EXPECT_STREQ("foo", matches.GetStringAtIndex(0));
+
+  request.AddCompletion("foo");
+  EXPECT_EQ(1U, request.GetNumberOfMatches());
+  EXPECT_EQ(1U, matches.GetSize());
+  EXPECT_STREQ("foo", matches.GetStringAtIndex(0));
+
+  // Add bar twice
+  request.AddCompletion("bar");
+  EXPECT_EQ(2U, request.GetNumberOfMatches());
+  EXPECT_EQ(2U, matches.GetSize());
+  EXPECT_STREQ("foo", matches.GetStringAtIndex(0));
+  EXPECT_STREQ("bar", matches.GetStringAtIndex(1));
+
+  request.AddCompletion("bar");
+  EXPECT_EQ(2U, request.GetNumberOfMatches());
+  EXPECT_EQ(2U, matches.GetSize());
+  EXPECT_STREQ("foo", matches.GetStringAtIndex(0));
+  EXPECT_STREQ("bar", matches.GetStringAtIndex(1));
+
+  // Add foo again.
+  request.AddCompletion("foo");
+  EXPECT_EQ(2U, request.GetNumberOfMatches());
+  EXPECT_EQ(2U, matches.GetSize());
+  EXPECT_STREQ("foo", matches.GetStringAtIndex(0));
+  EXPECT_STREQ("bar", matches.GetStringAtIndex(1));
+
+  // Add something with an existing prefix
+  request.AddCompletion("foobar");
+  EXPECT_EQ(3U, request.GetNumberOfMatches());
+  EXPECT_EQ(3U, matches.GetSize());
+  EXPECT_STREQ("foo", matches.GetStringAtIndex(0));
+  EXPECT_STREQ("bar", matches.GetStringAtIndex(1));
+  EXPECT_STREQ("foobar", matches.GetStringAtIndex(2));
+}
+
+TEST(CompletionRequest, TestCompletionOwnership) {
+  std::string command = "a bad c";
+  const unsigned cursor_pos = 3;
+  StringList matches;
+
+  CompletionRequest request(command, cursor_pos, 0, 0, matches);
+
+  std::string Temporary = "bar";
+  request.AddCompletion(Temporary);
+  // Manipulate our completion. The request should have taken a copy, so that
+  // shouldn't influence anything.
+  Temporary[0] = 'f';
 
-  // This is the generated matches should be equal to our passed string list.
-  EXPECT_EQ(&request.GetMatches(), &matches);
+  EXPECT_EQ(1U, request.GetNumberOfMatches());
+  EXPECT_STREQ("bar", matches.GetStringAtIndex(0));
 }
Index: source/Utility/CompletionRequest.cpp
===================================================================
--- source/Utility/CompletionRequest.cpp
+++ source/Utility/CompletionRequest.cpp
@@ -20,6 +20,7 @@
     : m_command(command_line), m_raw_cursor_pos(raw_cursor_pos),
       m_match_start_point(match_start_point),
       m_max_return_elements(max_return_elements), m_matches(&matches) {
+  matches.Clear();
 
   // We parse the argument up to the cursor, so the last argument in
   // parsed_line is the one containing the cursor, and the cursor is after the
Index: source/Utility/ArchSpec.cpp
===================================================================
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -255,12 +255,14 @@
     for (uint32_t i = 0; i < llvm::array_lengthof(g_core_definitions); ++i) {
       if (NameMatches(g_core_definitions[i].name, NameMatch::StartsWith,
                       request.GetCursorArgumentPrefix()))
-        request.GetMatches().AppendString(g_core_definitions[i].name);
+        request.AddCompletion(g_core_definitions[i].name);
     }
   } else {
-    ListSupportedArchNames(request.GetMatches());
+    StringList matches;
+    ListSupportedArchNames(matches);
+    request.AddCompletions(matches);
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
 
 #define CPU_ANY (UINT32_MAX)
Index: source/Symbol/Variable.cpp
===================================================================
--- source/Symbol/Variable.cpp
+++ source/Symbol/Variable.cpp
@@ -760,9 +760,11 @@
   CompilerType compiler_type;
 
   bool word_complete = false;
+  StringList matches;
   PrivateAutoComplete(exe_ctx.GetFramePtr(), request.GetCursorArgumentPrefix(),
-                      "", compiler_type, request.GetMatches(), word_complete);
+                      "", compiler_type, matches, word_complete);
   request.SetWordComplete(word_complete);
+  request.AddCompletions(matches);
 
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
Index: source/Interpreter/Options.cpp
===================================================================
--- source/Interpreter/Options.cpp
+++ source/Interpreter/Options.cpp
@@ -680,7 +680,7 @@
           if (!def.short_option)
             continue;
           opt_str[1] = def.short_option;
-          request.GetMatches().AppendString(opt_str);
+          request.AddCompletion(opt_str);
         }
 
         return true;
@@ -692,7 +692,7 @@
 
           full_name.erase(full_name.begin() + 2, full_name.end());
           full_name.append(def.long_option);
-          request.GetMatches().AppendString(full_name.c_str());
+          request.AddCompletion(full_name.c_str());
         }
         return true;
       } else if (opt_defs_index != OptionArgElement::eUnrecognizedArg) {
@@ -705,10 +705,10 @@
             strcmp(opt_defs[opt_defs_index].long_option, cur_opt_str) != 0) {
           std::string full_name("--");
           full_name.append(opt_defs[opt_defs_index].long_option);
-          request.GetMatches().AppendString(full_name.c_str());
+          request.AddCompletion(full_name.c_str());
           return true;
         } else {
-          request.GetMatches().AppendString(request.GetCursorArgument());
+          request.AddCompletion(request.GetCursorArgument());
           return true;
         }
       } else {
@@ -728,17 +728,7 @@
             if (strstr(def.long_option, cur_opt_str + 2) == def.long_option) {
               std::string full_name("--");
               full_name.append(def.long_option);
-              // The options definitions table has duplicates because of the
-              // way the grouping information is stored, so only add once.
-              bool duplicate = false;
-              for (size_t k = 0; k < request.GetMatches().GetSize(); k++) {
-                if (request.GetMatches().GetStringAtIndex(k) == full_name) {
-                  duplicate = true;
-                  break;
-                }
-              }
-              if (!duplicate)
-                request.GetMatches().AppendString(full_name.c_str());
+              request.AddCompletion(full_name.c_str());
             }
           }
         }
@@ -790,7 +780,7 @@
     for (int i = 0; enum_values[i].string_value != nullptr; i++) {
       if (strstr(enum_values[i].string_value, match_string.c_str()) ==
           enum_values[i].string_value) {
-        request.GetMatches().AppendString(enum_values[i].string_value);
+        request.AddCompletion(enum_values[i].string_value);
         return_value = true;
       }
     }
Index: source/Interpreter/OptionValueUUID.cpp
===================================================================
--- source/Interpreter/OptionValueUUID.cpp
+++ source/Interpreter/OptionValueUUID.cpp
@@ -70,7 +70,6 @@
 size_t OptionValueUUID::AutoComplete(CommandInterpreter &interpreter,
                                      CompletionRequest &request) {
   request.SetWordComplete(false);
-  request.GetMatches().Clear();
   ExecutionContext exe_ctx(interpreter.GetExecutionContext());
   Target *target = exe_ctx.GetTargetPtr();
   if (target) {
@@ -86,12 +85,12 @@
             llvm::ArrayRef<uint8_t> module_bytes = module_uuid.GetBytes();
             if (module_bytes.size() >= uuid_bytes.size() &&
                 module_bytes.take_front(uuid_bytes.size()).equals(uuid_bytes)) {
-              request.GetMatches().AppendString(module_uuid.GetAsString());
+              request.AddCompletion(module_uuid.GetAsString());
             }
           }
         }
       }
     }
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
Index: source/Interpreter/OptionValueFileSpec.cpp
===================================================================
--- source/Interpreter/OptionValueFileSpec.cpp
+++ source/Interpreter/OptionValueFileSpec.cpp
@@ -102,10 +102,9 @@
 size_t OptionValueFileSpec::AutoComplete(CommandInterpreter &interpreter,
                                          CompletionRequest &request) {
   request.SetWordComplete(false);
-  request.GetMatches().Clear();
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, m_completion_mask, request, nullptr);
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
 
 const lldb::DataBufferSP &OptionValueFileSpec::GetFileContents() {
Index: source/Interpreter/OptionValueEnumeration.cpp
===================================================================
--- source/Interpreter/OptionValueEnumeration.cpp
+++ source/Interpreter/OptionValueEnumeration.cpp
@@ -112,20 +112,18 @@
 size_t OptionValueEnumeration::AutoComplete(CommandInterpreter &interpreter,
                                             CompletionRequest &request) {
   request.SetWordComplete(false);
-  request.GetMatches().Clear();
 
   const uint32_t num_enumerators = m_enumerations.GetSize();
   if (!request.GetCursorArgumentPrefix().empty()) {
     for (size_t i = 0; i < num_enumerators; ++i) {
       llvm::StringRef name = m_enumerations.GetCStringAtIndex(i).GetStringRef();
       if (name.startswith(request.GetCursorArgumentPrefix()))
-        request.GetMatches().AppendString(name);
+        request.AddCompletion(name);
     }
   } else {
     // only suggest "true" or "false" by default
     for (size_t i = 0; i < num_enumerators; ++i)
-      request.GetMatches().AppendString(
-          m_enumerations.GetCStringAtIndex(i).GetStringRef());
+      request.AddCompletion(m_enumerations.GetCStringAtIndex(i).GetStringRef());
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
Index: source/Interpreter/OptionValueBoolean.cpp
===================================================================
--- source/Interpreter/OptionValueBoolean.cpp
+++ source/Interpreter/OptionValueBoolean.cpp
@@ -79,7 +79,6 @@
 size_t OptionValueBoolean::AutoComplete(CommandInterpreter &interpreter,
                                         CompletionRequest &request) {
   request.SetWordComplete(false);
-  request.GetMatches().Clear();
   static const llvm::StringRef g_autocomplete_entries[] = {
       "true", "false", "on", "off", "yes", "no", "1", "0"};
 
@@ -91,7 +90,7 @@
 
   for (auto entry : entries) {
     if (entry.startswith_lower(request.GetCursorArgumentPrefix()))
-      request.GetMatches().AppendString(entry);
+      request.AddCompletion(entry);
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
Index: source/Interpreter/OptionValueArch.cpp
===================================================================
--- source/Interpreter/OptionValueArch.cpp
+++ source/Interpreter/OptionValueArch.cpp
@@ -76,9 +76,8 @@
 size_t OptionValueArch::AutoComplete(CommandInterpreter &interpreter,
                                      CompletionRequest &request) {
   request.SetWordComplete(false);
-  request.GetMatches().Clear();
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, CommandCompletions::eArchitectureCompletion, request,
       nullptr);
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
Index: source/Interpreter/OptionValue.cpp
===================================================================
--- source/Interpreter/OptionValue.cpp
+++ source/Interpreter/OptionValue.cpp
@@ -575,8 +575,7 @@
 size_t OptionValue::AutoComplete(CommandInterpreter &interpreter,
                                  CompletionRequest &request) {
   request.SetWordComplete(false);
-  request.GetMatches().Clear();
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
 
 Status OptionValue::SetValueFromString(llvm::StringRef value,
Index: source/Interpreter/CommandObjectRegexCommand.cpp
===================================================================
--- source/Interpreter/CommandObjectRegexCommand.cpp
+++ source/Interpreter/CommandObjectRegexCommand.cpp
@@ -97,9 +97,8 @@
   if (m_completion_type_mask) {
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), m_completion_type_mask, request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   } else {
-    request.GetMatches().Clear();
     request.SetWordComplete(false);
   }
   return 0;
Index: source/Interpreter/CommandObject.cpp
===================================================================
--- source/Interpreter/CommandObject.cpp
+++ source/Interpreter/CommandObject.cpp
@@ -267,7 +267,6 @@
   if (WantsRawCommandString() && !WantsCompletion()) {
     // FIXME: Abstract telling the completion to insert the completion
     // character.
-    request.GetMatches().Clear();
     return -1;
   } else {
     // Can we do anything generic with the options?
@@ -282,7 +281,7 @@
       bool handled_by_options = cur_options->HandleOptionCompletion(
           request, opt_element_vector, GetCommandInterpreter());
       if (handled_by_options)
-        return request.GetMatches().GetSize();
+        return request.GetNumberOfMatches();
     }
 
     // If we got here, the last word is not an option or an option argument.
Index: source/Interpreter/CommandInterpreter.cpp
===================================================================
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -1709,7 +1709,7 @@
       error_msg.append("'.");
 
       error_msg.append(" Possible completions:");
-      for (int i = 0; i < num_matches; i++) {
+      for (std::size_t i = 0; i < matches.GetSize(); i++) {
         error_msg.append("\n\t");
         error_msg.append(matches.GetStringAtIndex(i));
       }
@@ -1730,7 +1730,6 @@
 }
 
 int CommandInterpreter::HandleCompletionMatches(CompletionRequest &request) {
-  auto &matches = request.GetMatches();
   int num_command_matches = 0;
   bool look_for_subcommand = false;
 
@@ -1740,30 +1739,34 @@
   if (request.GetCursorIndex() == -1) {
     // We got nothing on the command line, so return the list of commands
     bool include_aliases = true;
+    StringList new_matches;
     num_command_matches =
-        GetCommandNamesMatchingPartialString("", include_aliases, matches);
+        GetCommandNamesMatchingPartialString("", include_aliases, new_matches);
+    request.AddCompletions(new_matches);
   } else if (request.GetCursorIndex() == 0) {
     // The cursor is in the first argument, so just do a lookup in the
     // dictionary.
+    StringList new_matches;
     CommandObject *cmd_obj = GetCommandObject(
-        request.GetParsedLine().GetArgumentAtIndex(0), &matches);
-    num_command_matches = matches.GetSize();
+        request.GetParsedLine().GetArgumentAtIndex(0), &new_matches);
 
     if (num_command_matches == 1 && cmd_obj && cmd_obj->IsMultiwordObject() &&
-        matches.GetStringAtIndex(0) != nullptr &&
+        new_matches.GetStringAtIndex(0) != nullptr &&
         strcmp(request.GetParsedLine().GetArgumentAtIndex(0),
-               matches.GetStringAtIndex(0)) == 0) {
+               new_matches.GetStringAtIndex(0)) == 0) {
       if (request.GetParsedLine().GetArgumentCount() == 1) {
         request.SetWordComplete(true);
       } else {
         look_for_subcommand = true;
         num_command_matches = 0;
-        matches.DeleteStringAtIndex(0);
+        new_matches.DeleteStringAtIndex(0);
         request.GetParsedLine().AppendArgument(llvm::StringRef());
         request.SetCursorIndex(request.GetCursorIndex() + 1);
         request.SetCursorCharPosition(0);
       }
     }
+    request.AddCompletions(new_matches);
+    num_command_matches = request.GetNumberOfMatches();
   }
 
   if (request.GetCursorIndex() > 0 || look_for_subcommand) {
@@ -1800,8 +1803,7 @@
       return 0;
     else if (first_arg[0] == CommandHistory::g_repeat_char) {
       if (auto hist_str = m_command_history.FindString(first_arg)) {
-        request.GetMatches().Clear();
-        request.GetMatches().InsertStringAtIndex(0, *hist_str);
+        matches.InsertStringAtIndex(0, *hist_str);
         return -2;
       } else
         return 0;
@@ -1839,7 +1841,7 @@
         common_prefix.push_back(quote_char);
       common_prefix.push_back(' ');
     }
-    request.GetMatches().InsertStringAtIndex(0, common_prefix.c_str());
+    matches.InsertStringAtIndex(0, common_prefix.c_str());
   }
   return num_command_matches;
 }
Index: source/Core/IOHandler.cpp
===================================================================
--- source/Core/IOHandler.cpp
+++ source/Core/IOHandler.cpp
@@ -245,10 +245,10 @@
         io_handler.GetDebugger().GetCommandInterpreter(),
         CommandCompletions::eVariablePathCompletion, request, nullptr);
 
-    size_t num_matches = request.GetMatches().GetSize();
+    size_t num_matches = request.GetNumberOfMatches();
     if (num_matches > 0) {
       std::string common_prefix;
-      request.GetMatches().LongestCommonPrefix(common_prefix);
+      matches.LongestCommonPrefix(common_prefix);
       const size_t partial_name_len = request.GetCursorArgumentPrefix().size();
 
       // If we matched a unique single command, add a space... Only do this if
Index: source/Core/FormatEntity.cpp
===================================================================
--- source/Core/FormatEntity.cpp
+++ source/Core/FormatEntity.cpp
@@ -2349,7 +2349,6 @@
 
   request.SetWordComplete(false);
   str = str.drop_front(request.GetMatchStartPoint());
-  request.GetMatches().Clear();
 
   const size_t dollar_pos = str.rfind('$');
   if (dollar_pos == llvm::StringRef::npos)
@@ -2359,7 +2358,7 @@
   if (dollar_pos == str.size() - 1) {
     std::string match = str.str();
     match.append("{");
-    request.GetMatches().AppendString(match);
+    request.AddCompletion(match);
     return 1;
   }
 
@@ -2377,8 +2376,10 @@
   llvm::StringRef partial_variable(str.substr(dollar_pos + 2));
   if (partial_variable.empty()) {
     // Suggest all top level entites as we are just past "${"
-    AddMatches(&g_root, str, llvm::StringRef(), request.GetMatches());
-    return request.GetMatches().GetSize();
+    StringList new_matches;
+    AddMatches(&g_root, str, llvm::StringRef(), new_matches);
+    request.AddCompletions(new_matches);
+    return request.GetNumberOfMatches();
   }
 
   // We have a partially specified variable, find it
@@ -2394,19 +2395,23 @@
     // Exact match
     if (n > 0) {
       // "${thread.info" <TAB>
-      request.GetMatches().AppendString(MakeMatch(str, "."));
+      request.AddCompletion(MakeMatch(str, "."));
     } else {
       // "${thread.id" <TAB>
-      request.GetMatches().AppendString(MakeMatch(str, "}"));
+      request.AddCompletion(MakeMatch(str, "}"));
       request.SetWordComplete(true);
     }
   } else if (remainder.equals(".")) {
     // "${thread." <TAB>
-    AddMatches(entry_def, str, llvm::StringRef(), request.GetMatches());
+    StringList new_matches;
+    AddMatches(entry_def, str, llvm::StringRef(), new_matches);
+    request.AddCompletions(new_matches);
   } else {
     // We have a partial match
     // "${thre" <TAB>
-    AddMatches(entry_def, str, remainder, request.GetMatches());
+    StringList new_matches;
+    AddMatches(entry_def, str, remainder, new_matches);
+    request.AddCompletions(new_matches);
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -201,7 +201,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -1810,7 +1810,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eModuleCompletion, request,
         nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 };
 
@@ -1851,7 +1851,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eSourceFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 };
 
@@ -2393,7 +2393,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -3987,7 +3987,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
   Options *GetOptions() override { return &m_option_group; }
Index: source/Commands/CommandObjectSettings.cpp
===================================================================
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -172,7 +172,7 @@
         }
       }
     }
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -272,7 +272,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -338,7 +338,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -427,7 +427,7 @@
       CommandCompletions::InvokeCommonCompletionCallbacks(
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -544,7 +544,7 @@
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
 
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -644,7 +644,7 @@
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
 
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -749,7 +749,7 @@
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
 
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -843,7 +843,7 @@
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
 
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
@@ -924,7 +924,7 @@
           GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
           request, nullptr);
 
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
Index: source/Commands/CommandObjectProcess.cpp
===================================================================
--- source/Commands/CommandObjectProcess.cpp
+++ source/Commands/CommandObjectProcess.cpp
@@ -141,7 +141,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
   Options *GetOptions() override { return &m_options; }
@@ -410,9 +410,9 @@
           const size_t num_matches = process_infos.GetSize();
           if (num_matches > 0) {
             for (size_t i = 0; i < num_matches; ++i) {
-              request.GetMatches().AppendString(
+              request.AddCompletion(llvm::StringRef(
                   process_infos.GetProcessNameAtIndex(i),
-                  process_infos.GetProcessNameLengthAtIndex(i));
+                  process_infos.GetProcessNameLengthAtIndex(i)));
             }
           }
         }
Index: source/Commands/CommandObjectPlugin.cpp
===================================================================
--- source/Commands/CommandObjectPlugin.cpp
+++ source/Commands/CommandObjectPlugin.cpp
@@ -48,7 +48,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
Index: source/Commands/CommandObjectPlatform.cpp
===================================================================
--- source/Commands/CommandObjectPlatform.cpp
+++ source/Commands/CommandObjectPlatform.cpp
@@ -181,7 +181,7 @@
   int HandleCompletion(CompletionRequest &request) override {
     CommandCompletions::PlatformPluginNames(GetCommandInterpreter(), request,
                                             nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
   Options *GetOptions() override { return &m_option_group; }
@@ -1583,9 +1583,9 @@
           const uint32_t num_matches = process_infos.GetSize();
           if (num_matches > 0) {
             for (uint32_t i = 0; i < num_matches; ++i) {
-              request.GetMatches().AppendString(
+              request.AddCompletion(llvm::StringRef(
                   process_infos.GetProcessNameAtIndex(i),
-                  process_infos.GetProcessNameLengthAtIndex(i));
+                  process_infos.GetProcessNameLengthAtIndex(i)));
             }
           }
         }
Index: source/Commands/CommandObjectMultiword.cpp
===================================================================
--- source/Commands/CommandObjectMultiword.cpp
+++ source/Commands/CommandObjectMultiword.cpp
@@ -143,7 +143,7 @@
 
   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,36 +190,40 @@
   // 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());
           return cmd_obj->HandleCompletion(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,16 +370,14 @@
   CommandObject *proxy_command = GetProxyCommandObject();
   if (proxy_command)
     return proxy_command->HandleCompletion(request);
-  request.GetMatches().Clear();
   return 0;
 }
 
 int CommandObjectProxy::HandleArgumentCompletion(
     CompletionRequest &request, OptionElementVector &opt_element_vector) {
   CommandObject *proxy_command = GetProxyCommandObject();
   if (proxy_command)
     return proxy_command->HandleArgumentCompletion(request, opt_element_vector);
-  request.GetMatches().Clear();
   return 0;
 }
 
Index: source/Commands/CommandObjectFrame.cpp
===================================================================
--- source/Commands/CommandObjectFrame.cpp
+++ source/Commands/CommandObjectFrame.cpp
@@ -470,7 +470,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eVariablePathCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
 protected:
Index: source/Commands/CommandObjectCommands.cpp
===================================================================
--- source/Commands/CommandObjectCommands.cpp
+++ source/Commands/CommandObjectCommands.cpp
@@ -241,7 +241,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
   Options *GetOptions() override { return &m_options; }
@@ -1429,7 +1429,7 @@
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
         request, nullptr);
-    return request.GetMatches().GetSize();
+    return request.GetNumberOfMatches();
   }
 
   Options *GetOptions() override { return &m_options; }
Index: source/Commands/CommandCompletions.cpp
===================================================================
--- source/Commands/CommandCompletions.cpp
+++ source/Commands/CommandCompletions.cpp
@@ -90,7 +90,7 @@
   } else {
     completer.DoCompletion(searcher);
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
 
 static int DiskFilesOrDirectories(const llvm::Twine &partial_name,
@@ -229,8 +229,14 @@
                                   SearchFilter *searcher) {
   request.SetWordComplete(false);
   StandardTildeExpressionResolver Resolver;
-  return DiskFiles(request.GetCursorArgumentPrefix(), request.GetMatches(),
-                   Resolver);
+  StringList matches;
+  int result = DiskFiles(request.GetCursorArgumentPrefix(), matches, Resolver);
+  request.AddCompletions(matches);
+
+  // Propagate special return values.
+  if (result <= 0)
+    return result;
+  return request.GetNumberOfMatches();
 }
 
 int CommandCompletions::DiskFiles(const llvm::Twine &partial_file_name,
@@ -244,8 +250,15 @@
                                         SearchFilter *searcher) {
   request.SetWordComplete(false);
   StandardTildeExpressionResolver Resolver;
-  return DiskDirectories(request.GetCursorArgumentPrefix(),
-                         request.GetMatches(), Resolver);
+  StringList matches;
+  int result =
+      DiskDirectories(request.GetCursorArgumentPrefix(), matches, Resolver);
+  request.AddCompletions(matches);
+
+  // Propagate special return values.
+  if (result <= 0)
+    return result;
+  return request.GetNumberOfMatches();
 }
 
 int CommandCompletions::DiskDirectories(const llvm::Twine &partial_file_name,
@@ -267,7 +280,7 @@
   } else {
     completer.DoCompletion(searcher);
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
 
 int CommandCompletions::Symbols(CommandInterpreter &interpreter,
@@ -283,7 +296,7 @@
   } else {
     completer.DoCompletion(searcher);
   }
-  return request.GetMatches().GetSize();
+  return request.GetNumberOfMatches();
 }
 
 int CommandCompletions::SettingsNames(CommandInterpreter &interpreter,
@@ -304,20 +317,23 @@
   }
 
   size_t exact_matches_idx = SIZE_MAX;
-  const size_t num_matches =
-      g_property_names.AutoComplete(request.GetCursorArgumentPrefix(),
-                                    request.GetMatches(), exact_matches_idx);
+  StringList matches;
+  g_property_names.AutoComplete(request.GetCursorArgumentPrefix(), matches,
+                                exact_matches_idx);
   request.SetWordComplete(exact_matches_idx != SIZE_MAX);
-  return num_matches;
+  request.AddCompletions(matches);
+  return request.GetNumberOfMatches();
 }
 
 int CommandCompletions::PlatformPluginNames(CommandInterpreter &interpreter,
                                             CompletionRequest &request,
                                             SearchFilter *searcher) {
-  const uint32_t num_matches = PluginManager::AutoCompletePlatformName(
-      request.GetCursorArgumentPrefix(), request.GetMatches());
+  StringList new_matches;
+  std::size_t num_matches = PluginManager::AutoCompletePlatformName(
+      request.GetCursorArgumentPrefix(), new_matches);
   request.SetWordComplete(num_matches == 1);
-  return num_matches;
+  request.AddCompletions(new_matches);
+  return request.GetNumberOfMatches();
 }
 
 int CommandCompletions::ArchitectureNames(CommandInterpreter &interpreter,
@@ -409,10 +425,10 @@
   filter->Search(*this);
   // Now convert the filelist to completions:
   for (size_t i = 0; i < m_matching_files.GetSize(); i++) {
-    m_request.GetMatches().AppendString(
+    m_request.AddCompletion(
         m_matching_files.GetFileSpecAtIndex(i).GetFilename().GetCString());
   }
-  return m_request.GetMatches().GetSize();
+  return m_request.GetNumberOfMatches();
 }
 
 //----------------------------------------------------------------------
@@ -478,9 +494,9 @@
   filter->Search(*this);
   collection::iterator pos = m_match_set.begin(), end = m_match_set.end();
   for (pos = m_match_set.begin(); pos != end; pos++)
-    m_request.GetMatches().AppendString((*pos).GetCString());
+    m_request.AddCompletion((*pos).GetCString());
 
-  return m_request.GetMatches().GetSize();
+  return m_request.GetNumberOfMatches();
 }
 
 //----------------------------------------------------------------------
@@ -517,13 +533,13 @@
       match = false;
 
     if (match) {
-      m_request.GetMatches().AppendString(cur_file_name);
+      m_request.AddCompletion(cur_file_name);
     }
   }
   return Searcher::eCallbackReturnContinue;
 }
 
 size_t CommandCompletions::ModuleCompleter::DoCompletion(SearchFilter *filter) {
   filter->Search(*this);
-  return m_request.GetMatches().GetSize();
+  return m_request.GetNumberOfMatches();
 }
Index: include/lldb/Utility/CompletionRequest.h
===================================================================
--- include/lldb/Utility/CompletionRequest.h
+++ include/lldb/Utility/CompletionRequest.h
@@ -13,6 +13,7 @@
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/StringList.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 
 namespace lldb_private {
 
@@ -77,8 +78,32 @@
 
   void SetWordComplete(bool v) { m_word_complete = v; }
 
-  /// The array of matches returned.
-  StringList &GetMatches() { return *m_matches; }
+  /// Adds a possible completion string. If the completion was already
+  /// suggested before, it will not be added to the list of results. A copy of
+  /// the suggested completion is stored, so the given string can be free'd
+  /// afterwards.
+  ///
+  /// @param match The suggested completion.
+  void AddCompletion(llvm::StringRef completion) {
+    // Filter out duplicates.
+    if (m_match_set.find(completion) != m_match_set.end())
+      return;
+
+    m_matches->AppendString(completion.str());
+    m_match_set.insert(completion);
+  }
+
+  /// Adds multiple possible completion strings.
+  ///
+  /// \param completions The list of completions.
+  ///
+  /// @see AddCompletion
+  void AddCompletions(const StringList &completions) {
+    for (std::size_t i = 0; i < completions.GetSize(); ++i)
+      AddCompletion(completions.GetStringAtIndex(i));
+  }
+
+  std::size_t GetNumberOfMatches() const { return m_matches->GetSize(); }
 
   llvm::StringRef GetCursorArgument() const {
     return GetParsedLine().GetArgumentAtIndex(GetCursorIndex());
@@ -111,8 +136,15 @@
   /// \btrue if this is a complete option value (a space will be inserted
   /// after the completion.)  \bfalse otherwise.
   bool m_word_complete = false;
-  // We don't own the list.
+
+  // Note: This list is kept private. This is by design to prevent that any
+  // completion depends on any already computed completion from another backend.
+  // Note: We don't own the list. It's owned by the creator of the
+  // CompletionRequest object.
   StringList *m_matches;
+
+  /// List of added completions so far. Used to filter out duplicates.
+  llvm::StringSet<> m_match_set;
 };
 
 } // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to