Author: teemperor Date: Fri Jul 27 11:42:46 2018 New Revision: 338151 URL: http://llvm.org/viewvc/llvm-project?rev=338151&view=rev Log: 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 Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h lldb/trunk/source/Commands/CommandCompletions.cpp lldb/trunk/source/Commands/CommandObjectCommands.cpp lldb/trunk/source/Commands/CommandObjectFrame.cpp lldb/trunk/source/Commands/CommandObjectMultiword.cpp lldb/trunk/source/Commands/CommandObjectPlatform.cpp lldb/trunk/source/Commands/CommandObjectPlugin.cpp lldb/trunk/source/Commands/CommandObjectProcess.cpp lldb/trunk/source/Commands/CommandObjectSettings.cpp lldb/trunk/source/Commands/CommandObjectTarget.cpp lldb/trunk/source/Core/FormatEntity.cpp lldb/trunk/source/Core/IOHandler.cpp lldb/trunk/source/Interpreter/CommandInterpreter.cpp lldb/trunk/source/Interpreter/CommandObject.cpp lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp lldb/trunk/source/Interpreter/OptionValue.cpp lldb/trunk/source/Interpreter/OptionValueArch.cpp lldb/trunk/source/Interpreter/OptionValueBoolean.cpp lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp lldb/trunk/source/Interpreter/OptionValueUUID.cpp lldb/trunk/source/Interpreter/Options.cpp lldb/trunk/source/Symbol/Variable.cpp lldb/trunk/source/Utility/ArchSpec.cpp lldb/trunk/source/Utility/CompletionRequest.cpp lldb/trunk/unittests/Utility/CompletionRequestTest.cpp Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original) +++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Fri Jul 27 11:42:46 2018 @@ -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,29 @@ public: 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) { + // Add the completion if we haven't seen the same value before. + if (m_match_set.insert(completion).second) + m_matches->AppendString(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 +133,15 @@ private: /// \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 Modified: lldb/trunk/source/Commands/CommandCompletions.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandCompletions.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandCompletions.cpp (original) +++ lldb/trunk/source/Commands/CommandCompletions.cpp Fri Jul 27 11:42:46 2018 @@ -90,7 +90,7 @@ int CommandCompletions::SourceFiles(Comm } else { completer.DoCompletion(searcher); } - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } static int DiskFilesOrDirectories(const llvm::Twine &partial_name, @@ -103,7 +103,7 @@ static int DiskFilesOrDirectories(const partial_name.toVector(CompletionBuffer); if (CompletionBuffer.size() >= PATH_MAX) - return 0; + return matches.GetSize(); namespace fs = llvm::sys::fs; namespace path = llvm::sys::path; @@ -145,7 +145,7 @@ static int DiskFilesOrDirectories(const // Make sure it ends with a separator. path::append(CompletionBuffer, path::get_separator()); matches.AppendString(CompletionBuffer); - return 1; + return matches.GetSize(); } // We want to keep the form the user typed, so we special case this to @@ -224,13 +224,21 @@ static int DiskFilesOrDirectories(const return matches.GetSize(); } +static int DiskFilesOrDirectories(CompletionRequest &request, + bool only_directories) { + request.SetWordComplete(false); + StandardTildeExpressionResolver resolver; + StringList matches; + DiskFilesOrDirectories(request.GetCursorArgumentPrefix(), only_directories, + matches, resolver); + request.AddCompletions(matches); + return request.GetNumberOfMatches(); +} + int CommandCompletions::DiskFiles(CommandInterpreter &interpreter, CompletionRequest &request, SearchFilter *searcher) { - request.SetWordComplete(false); - StandardTildeExpressionResolver Resolver; - return DiskFiles(request.GetCursorArgumentPrefix(), request.GetMatches(), - Resolver); + return DiskFilesOrDirectories(request, /*only_dirs*/ false); } int CommandCompletions::DiskFiles(const llvm::Twine &partial_file_name, @@ -242,10 +250,7 @@ int CommandCompletions::DiskFiles(const int CommandCompletions::DiskDirectories(CommandInterpreter &interpreter, CompletionRequest &request, SearchFilter *searcher) { - request.SetWordComplete(false); - StandardTildeExpressionResolver Resolver; - return DiskDirectories(request.GetCursorArgumentPrefix(), - request.GetMatches(), Resolver); + return DiskFilesOrDirectories(request, /*only_dirs*/ true); } int CommandCompletions::DiskDirectories(const llvm::Twine &partial_file_name, @@ -267,7 +272,7 @@ int CommandCompletions::Modules(CommandI } else { completer.DoCompletion(searcher); } - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } int CommandCompletions::Symbols(CommandInterpreter &interpreter, @@ -283,7 +288,7 @@ int CommandCompletions::Symbols(CommandI } else { completer.DoCompletion(searcher); } - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } int CommandCompletions::SettingsNames(CommandInterpreter &interpreter, @@ -304,20 +309,23 @@ int CommandCompletions::SettingsNames(Co } 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 +417,10 @@ CommandCompletions::SourceFileCompleter: 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 +486,9 @@ size_t CommandCompletions::SymbolComplet 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,7 +525,7 @@ Searcher::CallbackReturn CommandCompleti match = false; if (match) { - m_request.GetMatches().AppendString(cur_file_name); + m_request.AddCompletion(cur_file_name); } } return Searcher::eCallbackReturnContinue; @@ -525,5 +533,5 @@ Searcher::CallbackReturn CommandCompleti size_t CommandCompletions::ModuleCompleter::DoCompletion(SearchFilter *filter) { filter->Search(*this); - return m_request.GetMatches().GetSize(); + return m_request.GetNumberOfMatches(); } Modified: lldb/trunk/source/Commands/CommandObjectCommands.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectCommands.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectCommands.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectCommands.cpp Fri Jul 27 11:42:46 2018 @@ -241,7 +241,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } Options *GetOptions() override { return &m_options; } @@ -1429,7 +1429,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } Options *GetOptions() override { return &m_options; } Modified: lldb/trunk/source/Commands/CommandObjectFrame.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectFrame.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectFrame.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectFrame.cpp Fri Jul 27 11:42:46 2018 @@ -470,7 +470,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eVariablePathCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: Modified: lldb/trunk/source/Commands/CommandObjectMultiword.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectMultiword.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectMultiword.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectMultiword.cpp Fri Jul 27 11:42:46 2018 @@ -143,7 +143,7 @@ bool CommandObjectMultiword::Execute(con 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::HandleComple // 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); - - if (matches.GetSize() == 1 && matches.GetStringAtIndex(0) != nullptr && - (arg0 == matches.GetStringAtIndex(0))) { + StringList new_matches; + AddNamesMatchingPartialString(m_subcommand_dict, arg0, new_matches); + request.AddCompletions(new_matches); + + 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::HandleComple } } } - 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 CommandObject *proxy_command = GetProxyCommandObject(); if (proxy_command) return proxy_command->HandleCompletion(request); - request.GetMatches().Clear(); return 0; } @@ -375,7 +378,6 @@ int CommandObjectProxy::HandleArgumentCo CommandObject *proxy_command = GetProxyCommandObject(); if (proxy_command) return proxy_command->HandleArgumentCompletion(request, opt_element_vector); - request.GetMatches().Clear(); return 0; } Modified: lldb/trunk/source/Commands/CommandObjectPlatform.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectPlatform.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectPlatform.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectPlatform.cpp Fri Jul 27 11:42:46 2018 @@ -181,7 +181,7 @@ public: 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 @@ public: 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))); } } } Modified: lldb/trunk/source/Commands/CommandObjectPlugin.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectPlugin.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectPlugin.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectPlugin.cpp Fri Jul 27 11:42:46 2018 @@ -48,7 +48,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: Modified: lldb/trunk/source/Commands/CommandObjectProcess.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectProcess.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectProcess.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectProcess.cpp Fri Jul 27 11:42:46 2018 @@ -141,7 +141,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } Options *GetOptions() override { return &m_options; } @@ -410,9 +410,9 @@ public: 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))); } } } Modified: lldb/trunk/source/Commands/CommandObjectSettings.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectSettings.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectSettings.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectSettings.cpp Fri Jul 27 11:42:46 2018 @@ -172,7 +172,7 @@ insert-before or insert-after."); } } } - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -272,7 +272,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -338,7 +338,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -427,7 +427,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -544,7 +544,7 @@ public: GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -644,7 +644,7 @@ public: GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -749,7 +749,7 @@ public: GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -843,7 +843,7 @@ public: GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -924,7 +924,7 @@ public: GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: Modified: lldb/trunk/source/Commands/CommandObjectTarget.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectTarget.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectTarget.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectTarget.cpp Fri Jul 27 11:42:46 2018 @@ -201,7 +201,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -1810,7 +1810,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eModuleCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } }; @@ -1851,7 +1851,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eSourceFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } }; @@ -2393,7 +2393,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } protected: @@ -3987,7 +3987,7 @@ public: CommandCompletions::InvokeCommonCompletionCallbacks( GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion, request, nullptr); - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } Options *GetOptions() override { return &m_option_group; } Modified: lldb/trunk/source/Core/FormatEntity.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FormatEntity.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Core/FormatEntity.cpp (original) +++ lldb/trunk/source/Core/FormatEntity.cpp Fri Jul 27 11:42:46 2018 @@ -2350,7 +2350,6 @@ size_t FormatEntity::AutoComplete(Comple 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) @@ -2360,7 +2359,7 @@ size_t FormatEntity::AutoComplete(Comple if (dollar_pos == str.size() - 1) { std::string match = str.str(); match.append("{"); - request.GetMatches().AppendString(match); + request.AddCompletion(match); return 1; } @@ -2378,8 +2377,10 @@ size_t FormatEntity::AutoComplete(Comple 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 @@ -2395,19 +2396,23 @@ size_t FormatEntity::AutoComplete(Comple // 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(); } Modified: lldb/trunk/source/Core/IOHandler.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/IOHandler.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Core/IOHandler.cpp (original) +++ lldb/trunk/source/Core/IOHandler.cpp Fri Jul 27 11:42:46 2018 @@ -245,10 +245,10 @@ int IOHandlerDelegate::IOHandlerComplete 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 Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original) +++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Fri Jul 27 11:42:46 2018 @@ -1703,7 +1703,6 @@ bool CommandInterpreter::HandleCommand(c } int CommandInterpreter::HandleCompletionMatches(CompletionRequest &request) { - auto &matches = request.GetMatches(); int num_command_matches = 0; bool look_for_subcommand = false; @@ -1713,30 +1712,34 @@ int CommandInterpreter::HandleCompletion 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) { @@ -1773,8 +1776,7 @@ int CommandInterpreter::HandleCompletion 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; @@ -1812,7 +1814,7 @@ int CommandInterpreter::HandleCompletion 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; } Modified: lldb/trunk/source/Interpreter/CommandObject.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandObject.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/CommandObject.cpp (original) +++ lldb/trunk/source/Interpreter/CommandObject.cpp Fri Jul 27 11:42:46 2018 @@ -267,7 +267,6 @@ int CommandObject::HandleCompletion(Comp 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 @@ int CommandObject::HandleCompletion(Comp 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. Modified: lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp (original) +++ lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp Fri Jul 27 11:42:46 2018 @@ -97,9 +97,8 @@ int CommandObjectRegexCommand::HandleCom 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; Modified: lldb/trunk/source/Interpreter/OptionValue.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValue.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/OptionValue.cpp (original) +++ lldb/trunk/source/Interpreter/OptionValue.cpp Fri Jul 27 11:42:46 2018 @@ -575,8 +575,7 @@ bool OptionValue::DumpQualifiedName(Stre 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, Modified: lldb/trunk/source/Interpreter/OptionValueArch.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueArch.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/OptionValueArch.cpp (original) +++ lldb/trunk/source/Interpreter/OptionValueArch.cpp Fri Jul 27 11:42:46 2018 @@ -76,9 +76,8 @@ lldb::OptionValueSP OptionValueArch::Dee 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(); } Modified: lldb/trunk/source/Interpreter/OptionValueBoolean.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueBoolean.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/OptionValueBoolean.cpp (original) +++ lldb/trunk/source/Interpreter/OptionValueBoolean.cpp Fri Jul 27 11:42:46 2018 @@ -79,7 +79,6 @@ lldb::OptionValueSP OptionValueBoolean:: 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 @@ size_t OptionValueBoolean::AutoComplete( for (auto entry : entries) { if (entry.startswith_lower(request.GetCursorArgumentPrefix())) - request.GetMatches().AppendString(entry); + request.AddCompletion(entry); } - return request.GetMatches().GetSize(); + return request.GetNumberOfMatches(); } Modified: lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp (original) +++ lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp Fri Jul 27 11:42:46 2018 @@ -112,20 +112,18 @@ lldb::OptionValueSP OptionValueEnumerati 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(); } Modified: lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp (original) +++ lldb/trunk/source/Interpreter/OptionValueFileSpec.cpp Fri Jul 27 11:42:46 2018 @@ -102,10 +102,9 @@ lldb::OptionValueSP OptionValueFileSpec: 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() { Modified: lldb/trunk/source/Interpreter/OptionValueUUID.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueUUID.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/OptionValueUUID.cpp (original) +++ lldb/trunk/source/Interpreter/OptionValueUUID.cpp Fri Jul 27 11:42:46 2018 @@ -70,7 +70,6 @@ lldb::OptionValueSP OptionValueUUID::Dee 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 @@ size_t OptionValueUUID::AutoComplete(Com 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(); } Modified: lldb/trunk/source/Interpreter/Options.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Interpreter/Options.cpp (original) +++ lldb/trunk/source/Interpreter/Options.cpp Fri Jul 27 11:42:46 2018 @@ -680,7 +680,7 @@ bool Options::HandleOptionCompletion(Com 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 @@ bool Options::HandleOptionCompletion(Com 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 @@ bool Options::HandleOptionCompletion(Com 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 @@ bool Options::HandleOptionCompletion(Com 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 @@ bool Options::HandleOptionArgumentComple 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; } } Modified: lldb/trunk/source/Symbol/Variable.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Variable.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Symbol/Variable.cpp (original) +++ lldb/trunk/source/Symbol/Variable.cpp Fri Jul 27 11:42:46 2018 @@ -760,9 +760,11 @@ size_t Variable::AutoComplete(const Exec 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(); } Modified: lldb/trunk/source/Utility/ArchSpec.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ArchSpec.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Utility/ArchSpec.cpp (original) +++ lldb/trunk/source/Utility/ArchSpec.cpp Fri Jul 27 11:42:46 2018 @@ -255,12 +255,14 @@ size_t ArchSpec::AutoComplete(Completion 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) Modified: lldb/trunk/source/Utility/CompletionRequest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/CompletionRequest.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/source/Utility/CompletionRequest.cpp (original) +++ lldb/trunk/source/Utility/CompletionRequest.cpp Fri Jul 27 11:42:46 2018 @@ -20,6 +20,7 @@ CompletionRequest::CompletionRequest(llv : 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 Modified: lldb/trunk/unittests/Utility/CompletionRequestTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CompletionRequestTest.cpp?rev=338151&r1=338150&r2=338151&view=diff ============================================================================== --- lldb/trunk/unittests/Utility/CompletionRequestTest.cpp (original) +++ lldb/trunk/unittests/Utility/CompletionRequestTest.cpp Fri Jul 27 11:42:46 2018 @@ -34,7 +34,70 @@ TEST(CompletionRequest, Constructor) { 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)); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
