[lldb] returning command completions up to a maximum #135565

eronnen · 2025-04-13T21:34:55Z

Adding max_return_elements field to CompletionRequest.
adding maximum checks to SymbolCompleter and SourceFileCompleter.

github-actions · 2025-04-13T21:35:15Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-04-13T21:35:45Z

@llvm/pr-subscribers-lldb

Author: Ely Ronnen (eronnen)

Changes

Adding max_return_elements field to CompletionRequest.
adding maximum checks to SymbolCompleter and SourceFileCompleter.

Fixes #135553

Full diff: https://github.com/llvm/llvm-project/pull/135565.diff

4 Files Affected:

(modified) lldb/include/lldb/Utility/CompletionRequest.h (+38)
(modified) lldb/source/API/SBCommandInterpreter.cpp (+5-1)
(modified) lldb/source/Commands/CommandCompletions.cpp (+11-3)
(modified) lldb/source/Utility/CompletionRequest.cpp (+8-1)

diff --git a/lldb/include/lldb/Utility/CompletionRequest.h b/lldb/include/lldb/Utility/CompletionRequest.h
index 865d6db576298..ae35c6a015649 100644
--- a/lldb/include/lldb/Utility/CompletionRequest.h
+++ b/lldb/include/lldb/Utility/CompletionRequest.h
@@ -115,6 +115,25 @@ class CompletionRequest {
   CompletionRequest(llvm::StringRef command_line, unsigned raw_cursor_pos,
                     CompletionResult &result);
 
+  /// Constructs a completion request.
+  ///
+  /// \param [in] command_line
+  ///     The command line the user has typed at this point.
+  ///
+  /// \param [in] raw_cursor_pos
+  ///     The position of the cursor in the command line string. Index 0 means
+  ///     the cursor is at the start of the line. The completion starts from
+  ///     this cursor position.
+  ///
+  /// \param [in] max_return_elements
+  ///     The maximum number of completions that should be returned.
+  ///
+  /// \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,
+                    size_t max_return_elements, CompletionResult &result);
+
   /// Returns the raw user input used to create this CompletionRequest cut off
   /// at the cursor position. The cursor will be at the end of the raw line.
   llvm::StringRef GetRawLine() const {
@@ -157,6 +176,23 @@ class CompletionRequest {
 
   size_t GetCursorIndex() const { return m_cursor_index; }
 
+  size_t GetMaxReturnElements() const { return m_max_return_elements; }
+
+  /// Returns true if the maximum number of completions has been reached
+  /// already.
+  bool ShouldStopAddingResults() const {
+    return m_result.GetNumberOfResults() >= m_max_return_elements;
+  }
+
+  /// Returns the maximum number of completions that need to be added
+  /// until reaching the maximum
+  size_t GetMaxNumberOfResultsToAdd() const {
+    const size_t number_of_results = m_result.GetNumberOfResults();
+    if (number_of_results >= m_max_return_elements)
+      return 0;
+    return m_max_return_elements - number_of_results;
+  }
+
   /// 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
@@ -231,6 +267,8 @@ class CompletionRequest {
   size_t m_cursor_index;
   /// The cursor position in the argument indexed by m_cursor_index.
   size_t m_cursor_char_position;
+  /// The maximum number of completions that should be returned.
+  size_t m_max_return_elements;
 
   /// The result this request is supposed to fill out.
   /// We keep this object private to ensure that no backend can in any way
diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp
index de22a9dd96bd8..6baf214f75b2d 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -263,9 +263,13 @@ int SBCommandInterpreter::HandleCompletionWithDescriptions(
   if (!IsValid())
     return 0;
 
+  if (0 >= max_return_elements)
+    return 0;
+
   lldb_private::StringList lldb_matches, lldb_descriptions;
   CompletionResult result;
-  CompletionRequest request(current_line, cursor - current_line, result);
+  CompletionRequest request(current_line, cursor - current_line,
+                            static_cast<size_t>(max_return_elements), result);
   m_opaque_ptr->HandleCompletion(request);
   result.GetMatches(lldb_matches);
   result.GetDescriptions(lldb_descriptions);
diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp
index 216aaf9abce6c..11cb94d4eda15 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -91,7 +91,7 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
        nullptr} // This one has to be last in the list.
   };
 
-  for (int i = 0;; i++) {
+  for (int i = 0; !request.ShouldStopAddingResults(); i++) {
     if (common_completions[i].type == lldb::eTerminatorCompletion)
       break;
     else if ((common_completions[i].type & completion_mask) ==
@@ -167,7 +167,9 @@ class SourceFileCompleter : public Completer {
         m_matching_files.AppendIfUnique(context.comp_unit->GetPrimaryFile());
       }
     }
-    return Searcher::eCallbackReturnContinue;
+    return m_matching_files.GetSize() >= m_request.GetMaxNumberOfResultsToAdd()
+               ? Searcher::eCallbackReturnStop
+               : Searcher::eCallbackReturnContinue;
   }
 
   void DoCompletion(SearchFilter *filter) override {
@@ -230,6 +232,10 @@ class SymbolCompleter : public Completer {
 
       // Now add the functions & symbols to the list - only add if unique:
       for (const SymbolContext &sc : sc_list) {
+        if (m_match_set.size() >= m_request.GetMaxNumberOfResultsToAdd()) {
+          break;
+        }
+
         ConstString func_name = sc.GetFunctionName(Mangled::ePreferDemangled);
         // Ensure that the function name matches the regex. This is more than
         // a sanity check. It is possible that the demangled function name
@@ -239,7 +245,9 @@ class SymbolCompleter : public Completer {
           m_match_set.insert(func_name);
       }
     }
-    return Searcher::eCallbackReturnContinue;
+    return m_match_set.size() >= m_request.GetMaxNumberOfResultsToAdd()
+               ? Searcher::eCallbackReturnStop
+               : Searcher::eCallbackReturnContinue;
   }
 
   void DoCompletion(SearchFilter *filter) override {
diff --git a/lldb/source/Utility/CompletionRequest.cpp b/lldb/source/Utility/CompletionRequest.cpp
index e12609ca75e7d..7dc20e9460bb3 100644
--- a/lldb/source/Utility/CompletionRequest.cpp
+++ b/lldb/source/Utility/CompletionRequest.cpp
@@ -14,8 +14,15 @@ using namespace lldb_private;
 CompletionRequest::CompletionRequest(llvm::StringRef command_line,
                                      unsigned raw_cursor_pos,
                                      CompletionResult &result)
+    : CompletionRequest(std::move(command_line), raw_cursor_pos, SIZE_MAX,
+                        result) {}
+
+CompletionRequest::CompletionRequest(llvm::StringRef command_line,
+                                     unsigned raw_cursor_pos,
+                                     size_t max_return_elements,
+                                     CompletionResult &result)
     : m_command(command_line), m_raw_cursor_pos(raw_cursor_pos),
-      m_result(result) {
+      m_max_return_elements(max_return_elements), m_result(result) {
   assert(raw_cursor_pos <= command_line.size() && "Out of bounds cursor?");
 
   // We parse the argument up to the cursor, so the last argument in

JDevlieghere

This definitely needs a test.

lldb/include/lldb/Utility/CompletionRequest.h

lldb/source/Commands/CommandCompletions.cpp

lldb/include/lldb/Utility/CompletionRequest.h

eronnen · 2025-04-15T21:57:31Z

@JDevlieghere Revoled comments and added the test_expr_completion_max_results test, do you think more tests are needed?

Jlalond · 2025-04-17T18:37:48Z

lldb/include/lldb/Utility/CompletionRequest.h

+  /// until reaching the maximum
+  size_t GetMaxNumberOfResultsToAdd() const {
+    const size_t number_of_results = m_result.GetNumberOfResults();
+    if (number_of_results >= m_max_return_elements)


Shouldn't this just call ShouldAddCompletions() and return 0 if false?

But still it needs to return the number of completions if ShouldAddCompletions is true

I Changed ShouldAddCompletions to return GetMaxNumberOfCompletionsToAdd > 0 instead in order to reuse code

Jlalond · 2025-04-17T18:38:28Z

lldb/source/Commands/CommandCompletions.cpp

@@ -230,6 +232,9 @@ class SymbolCompleter : public Completer {

      // Now add the functions & symbols to the list - only add if unique:
      for (const SymbolContext &sc : sc_list) {
+        if (m_match_set.size() >= m_request.GetMaxNumberOfResultsToAdd())


>? If I have 255 completions, and the max number of completions is 255, shouldn't that be valid?

In this case at the last iteration m_match_set.size() == 254 before adding the last completion, so it will still be added...
if m_match_set.size() > m_request.GetMaxNumberOfResultsToAdd() it means that the number of completions added at the end will be bigger than the max number

- Adding `max_return_elements` field to `CompletionRequest`. - adding maximum checks to `SymbolCompleter` and `SourceFileCompleter`.

Jlalond

Not a code-owner but LGTM. Please also get sign off from @JDevlieghere

Jlalond · 2025-04-22T17:39:04Z

lldb/include/lldb/Utility/CompletionRequest.h

@@ -231,6 +253,8 @@ class CompletionRequest {
  size_t m_cursor_index;
  /// The cursor position in the argument indexed by m_cursor_index.
  size_t m_cursor_char_position;
+  /// The maximum number of completions that should be returned.
+  size_t m_max_return_elements = std::numeric_limits<size_t>::max();


Should we have a sensible (non size_t max) default?

github-actions · 2025-04-23T22:01:22Z

@eronnen Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

- Adding `max_return_elements` field to `CompletionRequest`. - adding maximum checks to `SymbolCompleter` and `SourceFileCompleter`. Fixes llvm#135553

eronnen requested a review from JDevlieghere as a code owner April 13, 2025 21:34

llvmbot added the lldb label Apr 13, 2025

eronnen force-pushed the lldb-completions-return-up-to-maximum branch from 1b45ab6 to 66333e2 Compare April 13, 2025 23:52

JDevlieghere reviewed Apr 14, 2025

View reviewed changes

lldb/include/lldb/Utility/CompletionRequest.h Outdated Show resolved Hide resolved

lldb/source/Commands/CommandCompletions.cpp Outdated Show resolved Hide resolved

lldb/include/lldb/Utility/CompletionRequest.h Outdated Show resolved Hide resolved

eronnen force-pushed the lldb-completions-return-up-to-maximum branch from 66333e2 to 9f60323 Compare April 15, 2025 21:48

Jlalond reviewed Apr 17, 2025

View reviewed changes

eronnen added 3 commits April 18, 2025 00:23

[lldb] returning command completions up to a maximum

04e503a

- Adding `max_return_elements` field to `CompletionRequest`. - adding maximum checks to `SymbolCompleter` and `SourceFileCompleter`.

limiting completion results to max_return_elements, adding api tests

8dc824d

reuse GetMaxNumberOfCompletionsToAdd

1922ad8

eronnen force-pushed the lldb-completions-return-up-to-maximum branch from 9f60323 to 1922ad8 Compare April 17, 2025 22:53

eronnen requested review from JDevlieghere and Jlalond April 17, 2025 23:03

Jlalond approved these changes Apr 22, 2025

View reviewed changes

JDevlieghere approved these changes Apr 23, 2025

View reviewed changes

JDevlieghere merged commit 6f5b98b into llvm:main Apr 23, 2025
6 checks passed

eronnen mentioned this pull request Apr 25, 2025

Request Commit Access For eronnen #137382

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb] returning command completions up to a maximum #135565

[lldb] returning command completions up to a maximum #135565

Uh oh!

eronnen commented Apr 13, 2025

Uh oh!

github-actions bot commented Apr 13, 2025

Uh oh!

llvmbot commented Apr 13, 2025

Uh oh!

JDevlieghere left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eronnen commented Apr 15, 2025 •

edited

Loading

Uh oh!

Jlalond Apr 17, 2025

Uh oh!

eronnen Apr 17, 2025

Uh oh!

Jlalond Apr 17, 2025

Uh oh!

eronnen Apr 17, 2025

Uh oh!

Jlalond left a comment

Uh oh!

Jlalond Apr 22, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 23, 2025

Uh oh!

Uh oh!

[lldb] returning command completions up to a maximum #135565

[lldb] returning command completions up to a maximum #135565

Uh oh!

Conversation

eronnen commented Apr 13, 2025

Uh oh!

github-actions bot commented Apr 13, 2025

Uh oh!

llvmbot commented Apr 13, 2025

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eronnen commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jlalond Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

eronnen Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

Jlalond Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

eronnen Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

Jlalond left a comment

Choose a reason for hiding this comment

Uh oh!

Jlalond Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Apr 23, 2025

Uh oh!

Uh oh!

eronnen commented Apr 15, 2025 •

edited

Loading