[Lldb-commits] [lldb] r367385 - [CompletionRequest] Remove unimplemented members.

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 31 11:35:35 PDT 2019


This wasn't a "weird thing".  

When you have a match with potentially many hits, it would be very handy to say "give me the first 10", then "give me the next 10" so you only do the work to fetch as much as your paginator would show.  That would allow us to avoid unnecessary work when somebody gets a huge number of hits because they accidentally tried to complete "a" or something...  Right now we stop to gather all the matches and then paginate the full list so it can take a long time to get back control so you can then say "I don't want to see any more matches, I hit <TAB> by accident".

Passing in a start index and num_elements is the natural way to express this.

The problem was that none of the searches in lldb that produce long lists of matches were designed so the that you can resume the search midway along, and so it never had any useful clients.  OTOH, we haven't produced any such clients, and it seems like we aren't ever going to be able to use it, so I'm fine with removing the feature.

Jim

> On Jul 31, 2019, at 12:01 AM, Raphael “Teemperor” Isemann via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> Thanks! Wasn’t sure back then if I can just remove that weird thing
> 
>> On Jul 31, 2019, at 5:48 AM, Jonas Devlieghere via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>> 
>> Author: jdevlieghere
>> Date: Tue Jul 30 20:48:29 2019
>> New Revision: 367385
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=367385&view=rev
>> Log:
>> [CompletionRequest] Remove unimplemented members.
>> 
>> Completion requests have two fields that are essentially unimplemented:
>> `m_match_start_point` and `m_max_return_elements`. This would've been
>> okay, if it wasn't for the fact that this caused a bunch of useless
>> parameters to be passed around. Occasionally there would be a comment or
>> assert saying that they are not supported. This patch removes them.
>> 
>> Modified:
>>   lldb/trunk/include/lldb/Core/IOHandler.h
>>   lldb/trunk/include/lldb/Expression/REPL.h
>>   lldb/trunk/include/lldb/Host/Editline.h
>>   lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
>>   lldb/trunk/include/lldb/Utility/CompletionRequest.h
>>   lldb/trunk/source/API/SBCommandInterpreter.cpp
>>   lldb/trunk/source/Core/FormatEntity.cpp
>>   lldb/trunk/source/Core/IOHandler.cpp
>>   lldb/trunk/source/Expression/REPL.cpp
>>   lldb/trunk/source/Host/common/Editline.cpp
>>   lldb/trunk/source/Interpreter/CommandInterpreter.cpp
>>   lldb/trunk/source/Utility/CompletionRequest.cpp
>>   lldb/trunk/unittests/Utility/CompletionRequestTest.cpp
>> 
>> Modified: lldb/trunk/include/lldb/Core/IOHandler.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/IOHandler.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Core/IOHandler.h (original)
>> +++ lldb/trunk/include/lldb/Core/IOHandler.h Tue Jul 30 20:48:29 2019
>> @@ -200,7 +200,6 @@ public:
>> 
>>  virtual int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
>>                                const char *cursor, const char *last_char,
>> -                                int skip_first_n_matches, int max_matches,
>>                                StringList &matches, StringList &descriptions);
>> 
>>  virtual const char *IOHandlerGetFixIndentationCharacters() { return nullptr; }
>> @@ -417,7 +416,6 @@ private:
>> 
>>  static int AutoCompleteCallback(const char *current_line, const char *cursor,
>>                                  const char *last_char,
>> -                                  int skip_first_n_matches, int max_matches,
>>                                  StringList &matches, StringList &descriptions,
>>                                  void *baton);
>> #endif
>> @@ -452,7 +450,6 @@ public:
>> 
>>  int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
>>                        const char *cursor, const char *last_char,
>> -                        int skip_first_n_matches, int max_matches,
>>                        StringList &matches, StringList &descriptions) override;
>> 
>>  void IOHandlerInputComplete(IOHandler &io_handler,
>> 
>> Modified: lldb/trunk/include/lldb/Expression/REPL.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/REPL.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Expression/REPL.h (original)
>> +++ lldb/trunk/include/lldb/Expression/REPL.h Tue Jul 30 20:48:29 2019
>> @@ -105,7 +105,6 @@ public:
>> 
>>  int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
>>                        const char *cursor, const char *last_char,
>> -                        int skip_first_n_matches, int max_matches,
>>                        StringList &matches, StringList &descriptions) override;
>> 
>> protected:
>> 
>> Modified: lldb/trunk/include/lldb/Host/Editline.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Editline.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Host/Editline.h (original)
>> +++ lldb/trunk/include/lldb/Host/Editline.h Tue Jul 30 20:48:29 2019
>> @@ -99,7 +99,6 @@ typedef int (*FixIndentationCallbackType
>> 
>> typedef int (*CompleteCallbackType)(const char *current_line,
>>                                    const char *cursor, const char *last_char,
>> -                                    int skip_first_n_matches, int max_matches,
>>                                    StringList &matches,
>>                                    StringList &descriptions, void *baton);
>> 
>> 
>> Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original)
>> +++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h Tue Jul 30 20:48:29 2019
>> @@ -325,8 +325,7 @@ public:
>>  //
>>  // FIXME: Only max_return_elements == -1 is supported at present.
>>  int HandleCompletion(const char *current_line, const char *cursor,
>> -                       const char *last_char, int match_start_point,
>> -                       int max_return_elements, StringList &matches,
>> +                       const char *last_char, StringList &matches,
>>                       StringList &descriptions);
>> 
>>  // This version just returns matches, and doesn't compute the substring.  It
>> 
>> Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original)
>> +++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Tue Jul 30 20:48:29 2019
>> @@ -68,18 +68,10 @@ public:
>>  ///     the cursor is at the start of the line. The completion starts from
>>  ///     this cursor position.
>>  ///
>> -  /// \param [in] match_start_point
>> -  /// \param [in] max_return_elements
>> -  ///     If there is a match that is expensive to compute, these are here to
>> -  ///     allow you to compute the completions in  batches.  Start the
>> -  ///     completion from match_start_point, and return match_return_elements
>> -  ///     elements.
>> -  ///
>>  /// \param [out] result
>>  ///     The CompletionResult that will be filled with the results after this
>>  ///     request has been handled.
>>  CompletionRequest(llvm::StringRef command_line, unsigned raw_cursor_pos,
>> -                    int match_start_point, int max_return_elements,
>>                    CompletionResult &result);
>> 
>>  llvm::StringRef GetRawLine() const { return m_command; }
>> @@ -98,10 +90,6 @@ public:
>>  void SetCursorCharPosition(int pos) { m_cursor_char_position = pos; }
>>  int GetCursorCharPosition() const { return m_cursor_char_position; }
>> 
>> -  int GetMatchStartPoint() const { return m_match_start_point; }
>> -
>> -  int GetMaxReturnElements() const { return m_max_return_elements; }
>> -
>>  bool GetWordComplete() { return m_word_complete; }
>> 
>>  void SetWordComplete(bool v) { m_word_complete = v; }
>> @@ -170,13 +158,6 @@ private:
>>  int m_cursor_index;
>>  /// The cursor position in the argument indexed by m_cursor_index.
>>  int m_cursor_char_position;
>> -  /// If there is a match that is expensive
>> -  /// to compute, these are here to allow you to compute the completions in
>> -  /// batches.  Start the completion from \amatch_start_point, and return
>> -  /// \amatch_return_elements elements.
>> -  // FIXME: These two values are not implemented.
>> -  int m_match_start_point;
>> -  int m_max_return_elements;
>>  /// \btrue if this is a complete option value (a space will be inserted
>>  /// after the completion.)  \bfalse otherwise.
>>  bool m_word_complete = false;
>> 
>> Modified: lldb/trunk/source/API/SBCommandInterpreter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBCommandInterpreter.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/API/SBCommandInterpreter.cpp (original)
>> +++ lldb/trunk/source/API/SBCommandInterpreter.cpp Tue Jul 30 20:48:29 2019
>> @@ -372,8 +372,7 @@ int SBCommandInterpreter::HandleCompleti
>>  if (IsValid()) {
>>    lldb_private::StringList lldb_matches, lldb_descriptions;
>>    num_completions = m_opaque_ptr->HandleCompletion(
>> -        current_line, cursor, last_char, match_start_point, max_return_elements,
>> -        lldb_matches, lldb_descriptions);
>> +        current_line, cursor, last_char, lldb_matches, lldb_descriptions);
>> 
>>    SBStringList temp_matches_list(&lldb_matches);
>>    matches.AppendList(temp_matches_list);
>> 
>> Modified: lldb/trunk/source/Core/FormatEntity.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FormatEntity.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Core/FormatEntity.cpp (original)
>> +++ lldb/trunk/source/Core/FormatEntity.cpp Tue Jul 30 20:48:29 2019
>> @@ -2350,7 +2350,6 @@ size_t FormatEntity::AutoComplete(Comple
>>  llvm::StringRef str = request.GetCursorArgumentPrefix().str();
>> 
>>  request.SetWordComplete(false);
>> -  str = str.drop_front(request.GetMatchStartPoint());
>> 
>>  const size_t dollar_pos = str.rfind('$');
>>  if (dollar_pos == llvm::StringRef::npos)
>> 
>> Modified: lldb/trunk/source/Core/IOHandler.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/IOHandler.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Core/IOHandler.cpp (original)
>> +++ lldb/trunk/source/Core/IOHandler.cpp Tue Jul 30 20:48:29 2019
>> @@ -172,8 +172,7 @@ IOHandlerConfirm::~IOHandlerConfirm() =
>> 
>> int IOHandlerConfirm::IOHandlerComplete(
>>    IOHandler &io_handler, const char *current_line, const char *cursor,
>> -    const char *last_char, int skip_first_n_matches, int max_matches,
>> -    StringList &matches, StringList &descriptions) {
>> +    const char *last_char, StringList &matches, StringList &descriptions) {
>>  if (current_line == cursor) {
>>    if (m_default_response) {
>>      matches.AppendString("y");
>> @@ -221,20 +220,17 @@ void IOHandlerConfirm::IOHandlerInputCom
>> 
>> int IOHandlerDelegate::IOHandlerComplete(
>>    IOHandler &io_handler, const char *current_line, const char *cursor,
>> -    const char *last_char, int skip_first_n_matches, int max_matches,
>> -    StringList &matches, StringList &descriptions) {
>> +    const char *last_char, StringList &matches, StringList &descriptions) {
>>  switch (m_completion) {
>>  case Completion::None:
>>    break;
>> 
>>  case Completion::LLDBCommand:
>>    return io_handler.GetDebugger().GetCommandInterpreter().HandleCompletion(
>> -        current_line, cursor, last_char, skip_first_n_matches, max_matches,
>> -        matches, descriptions);
>> +        current_line, cursor, last_char, matches, descriptions);
>>  case Completion::Expression: {
>>    CompletionResult result;
>> -    CompletionRequest request(current_line, cursor - current_line,
>> -                              skip_first_n_matches, max_matches, result);
>> +    CompletionRequest request(current_line, cursor - current_line, result);
>>    CommandCompletions::InvokeCommonCompletionCallbacks(
>>        io_handler.GetDebugger().GetCommandInterpreter(),
>>        CommandCompletions::eVariablePathCompletion, request, nullptr);
>> @@ -449,13 +445,12 @@ int IOHandlerEditline::FixIndentationCal
>> 
>> int IOHandlerEditline::AutoCompleteCallback(
>>    const char *current_line, const char *cursor, const char *last_char,
>> -    int skip_first_n_matches, int max_matches, StringList &matches,
>> -    StringList &descriptions, void *baton) {
>> +    StringList &matches, StringList &descriptions, void *baton) {
>>  IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
>>  if (editline_reader)
>>    return editline_reader->m_delegate.IOHandlerComplete(
>> -        *editline_reader, current_line, cursor, last_char, skip_first_n_matches,
>> -        max_matches, matches, descriptions);
>> +        *editline_reader, current_line, cursor, last_char, matches,
>> +        descriptions);
>>  return 0;
>> }
>> #endif
>> 
>> Modified: lldb/trunk/source/Expression/REPL.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/REPL.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Expression/REPL.cpp (original)
>> +++ lldb/trunk/source/Expression/REPL.cpp Tue Jul 30 20:48:29 2019
>> @@ -435,7 +435,6 @@ void REPL::IOHandlerInputComplete(IOHand
>> 
>> int REPL::IOHandlerComplete(IOHandler &io_handler, const char *current_line,
>>                            const char *cursor, const char *last_char,
>> -                            int skip_first_n_matches, int max_matches,
>>                            StringList &matches, StringList &descriptions) {
>>  matches.Clear();
>> 
>> @@ -448,8 +447,7 @@ int REPL::IOHandlerComplete(IOHandler &i
>>    // auto complete LLDB commands
>>    const char *lldb_current_line = line.substr(1).data();
>>    return debugger.GetCommandInterpreter().HandleCompletion(
>> -        lldb_current_line, cursor, last_char, skip_first_n_matches, max_matches,
>> -        matches, descriptions);
>> +        lldb_current_line, cursor, last_char, matches, descriptions);
>>  }
>> 
>>  // Strip spaces from the line and see if we had only spaces
>> 
>> Modified: lldb/trunk/source/Host/common/Editline.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Editline.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Host/common/Editline.cpp (original)
>> +++ lldb/trunk/source/Host/common/Editline.cpp Tue Jul 30 20:48:29 2019
>> @@ -896,8 +896,6 @@ unsigned char Editline::TabCommand(int c
>> 
>>  const int num_completions = m_completion_callback(
>>      line_info->buffer, line_info->cursor, line_info->lastchar,
>> -      0,  // Don't skip any matches (start at match zero)
>> -      -1, // Get all the matches
>>      completions, descriptions, m_completion_callback_baton);
>> 
>>  if (num_completions == 0)
>> 
>> Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
>> +++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Tue Jul 30 20:48:29 2019
>> @@ -1813,15 +1813,15 @@ int CommandInterpreter::HandleCompletion
>>  return num_command_matches;
>> }
>> 
>> -int CommandInterpreter::HandleCompletion(
>> -    const char *current_line, const char *cursor, const char *last_char,
>> -    int match_start_point, int max_return_elements, StringList &matches,
>> -    StringList &descriptions) {
>> +int CommandInterpreter::HandleCompletion(const char *current_line,
>> +                                         const char *cursor,
>> +                                         const char *last_char,
>> +                                         StringList &matches,
>> +                                         StringList &descriptions) {
>> 
>>  llvm::StringRef command_line(current_line, last_char - current_line);
>>  CompletionResult result;
>> -  CompletionRequest request(command_line, cursor - current_line,
>> -                            match_start_point, max_return_elements, result);
>> +  CompletionRequest request(command_line, cursor - current_line, result);
>>  // Don't complete comments, and if the line we are completing is just the
>>  // history repeat character, substitute the appropriate history line.
>>  const char *first_arg = request.GetParsedLine().GetArgumentAtIndex(0);
>> @@ -1838,9 +1838,6 @@ int CommandInterpreter::HandleCompletion
>>    }
>>  }
>> 
>> -  // Only max_return_elements == -1 is supported at present:
>> -  lldbassert(max_return_elements == -1);
>> -
>>  int num_command_matches = HandleCompletionMatches(request);
>>  result.GetMatches(matches);
>>  result.GetDescriptions(descriptions);
>> 
>> Modified: lldb/trunk/source/Utility/CompletionRequest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/CompletionRequest.cpp?rev=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Utility/CompletionRequest.cpp (original)
>> +++ lldb/trunk/source/Utility/CompletionRequest.cpp Tue Jul 30 20:48:29 2019
>> @@ -13,12 +13,9 @@ using namespace lldb_private;
>> 
>> CompletionRequest::CompletionRequest(llvm::StringRef command_line,
>>                                     unsigned raw_cursor_pos,
>> -                                     int match_start_point,
>> -                                     int max_return_elements,
>>                                     CompletionResult &result)
>>    : 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_result(result) {
>> +      m_result(result) {
>> 
>>  // 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=367385&r1=367384&r2=367385&view=diff
>> ==============================================================================
>> --- lldb/trunk/unittests/Utility/CompletionRequestTest.cpp (original)
>> +++ lldb/trunk/unittests/Utility/CompletionRequestTest.cpp Tue Jul 30 20:48:29 2019
>> @@ -16,21 +16,16 @@ TEST(CompletionRequest, Constructor) {
>>  const unsigned cursor_pos = 3;
>>  const int arg_index = 1;
>>  const int arg_cursor_pos = 1;
>> -  const int match_start = 2345;
>> -  const int match_max_return = 12345;
>>  StringList matches;
>>  CompletionResult result;
>> 
>> -  CompletionRequest request(command, cursor_pos, match_start, match_max_return,
>> -                            result);
>> +  CompletionRequest request(command, cursor_pos, result);
>>  result.GetMatches(matches);
>> 
>>  EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str());
>>  EXPECT_EQ(request.GetRawCursorPos(), cursor_pos);
>>  EXPECT_EQ(request.GetCursorIndex(), arg_index);
>>  EXPECT_EQ(request.GetCursorCharPosition(), arg_cursor_pos);
>> -  EXPECT_EQ(request.GetMatchStartPoint(), match_start);
>> -  EXPECT_EQ(request.GetMaxReturnElements(), match_max_return);
>>  EXPECT_EQ(request.GetWordComplete(), false);
>> 
>>  EXPECT_EQ(request.GetPartialParsedLine().GetArgumentCount(), 2u);
>> @@ -43,7 +38,7 @@ TEST(CompletionRequest, DuplicateFilteri
>>  StringList matches;
>> 
>>  CompletionResult result;
>> -  CompletionRequest request(command, cursor_pos, 0, 0, result);
>> +  CompletionRequest request(command, cursor_pos, result);
>>  result.GetMatches(matches);
>> 
>>  EXPECT_EQ(0U, request.GetNumberOfMatches());
>> @@ -106,7 +101,7 @@ TEST(CompletionRequest, DuplicateFilteri
>>  StringList matches, descriptions;
>> 
>>  CompletionResult result;
>> -  CompletionRequest request(command, cursor_pos, 0, 0, result);
>> +  CompletionRequest request(command, cursor_pos, result);
>>  result.GetMatches(matches);
>>  result.GetDescriptions(descriptions);
>> 
>> @@ -182,7 +177,7 @@ TEST(CompletionRequest, TestCompletionOw
>>  StringList matches;
>> 
>>  CompletionResult result;
>> -  CompletionRequest request(command, cursor_pos, 0, 0, result);
>> +  CompletionRequest request(command, cursor_pos, result);
>> 
>>  std::string Temporary = "bar";
>>  request.AddCompletion(Temporary);
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits



More information about the lldb-commits mailing list