[Lldb-commits] [lldb] 5826aa4 - Migrate to llvm::unique_function instead of static member functions for callbacks

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Tue Mar 2 16:14:17 PST 2021


Author: Neal (nealsid)
Date: 2021-03-02T16:13:54-08:00
New Revision: 5826aa48f03fba215b135f3c21ee52662281134d

URL: https://github.com/llvm/llvm-project/commit/5826aa48f03fba215b135f3c21ee52662281134d
DIFF: https://github.com/llvm/llvm-project/commit/5826aa48f03fba215b135f3c21ee52662281134d.diff

LOG: Migrate to llvm::unique_function instead of static member functions for callbacks

A few cleanups suggested in another patch review's comments:

1. Use llvm:unique_function for storing & invoking callbacks from
   Editline to IOHandler
2. Change return type of one of the callback setters from bool to void,
   since it's return value was never used
3. Moved the callback setters inline & made them nonstatic, since that's
   more consistent with other setter definitions
4. Removed the baton parameter since we no longer need it anymore

Differential revision: https://reviews.llvm.org/D50299

Added: 
    

Modified: 
    lldb/include/lldb/Core/IOHandler.h
    lldb/include/lldb/Host/Editline.h
    lldb/source/Core/IOHandler.cpp
    lldb/source/Host/common/Editline.cpp
    lldb/unittests/Editline/EditlineTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/IOHandler.h b/lldb/include/lldb/Core/IOHandler.h
index 2e8f3225fd5f..6cb0e56681e0 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -419,16 +419,14 @@ class IOHandlerEditline : public IOHandler {
 
 private:
 #if LLDB_ENABLE_LIBEDIT
-  static bool IsInputCompleteCallback(Editline *editline, StringList &lines,
-                                      void *baton);
+  bool IsInputCompleteCallback(Editline *editline, StringList &lines);
 
-  static int FixIndentationCallback(Editline *editline, const StringList &lines,
-                                    int cursor_position, void *baton);
+  int FixIndentationCallback(Editline *editline, const StringList &lines,
+                             int cursor_position);
 
-  static llvm::Optional<std::string> SuggestionCallback(llvm::StringRef line,
-                                                        void *baton);
+  llvm::Optional<std::string> SuggestionCallback(llvm::StringRef line);
 
-  static void AutoCompleteCallback(CompletionRequest &request, void *baton);
+  void AutoCompleteCallback(CompletionRequest &request);
 #endif
 
 protected:

diff  --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index a37ad1b9d106..3295c9a2080a 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -38,7 +38,6 @@
 #include <sstream>
 #include <vector>
 
-#include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/lldb-private.h"
 
 #if defined(_WIN32)
@@ -56,6 +55,9 @@
 #include "lldb/Utility/CompletionRequest.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Predicate.h"
+#include "lldb/Utility/StringList.h"
+
+#include "llvm/ADT/FunctionExtras.h"
 
 namespace lldb_private {
 namespace line_editor {
@@ -81,27 +83,26 @@ using EditLineGetCharType = wchar_t;
 using EditLineGetCharType = char;
 #endif
 
-typedef int (*EditlineGetCharCallbackType)(::EditLine *editline,
-                                           EditLineGetCharType *c);
-typedef unsigned char (*EditlineCommandCallbackType)(::EditLine *editline,
-                                                     int ch);
-typedef const char *(*EditlinePromptCallbackType)(::EditLine *editline);
+using EditlineGetCharCallbackType = int (*)(::EditLine *editline,
+                                            EditLineGetCharType *c);
+using EditlineCommandCallbackType = unsigned char (*)(::EditLine *editline,
+                                                      int ch);
+using EditlinePromptCallbackType = const char *(*)(::EditLine *editline);
 
 class EditlineHistory;
 
-typedef std::shared_ptr<EditlineHistory> EditlineHistorySP;
+using EditlineHistorySP = std::shared_ptr<EditlineHistory>;
 
-typedef bool (*IsInputCompleteCallbackType)(Editline *editline,
-                                            StringList &lines, void *baton);
+using IsInputCompleteCallbackType =
+    llvm::unique_function<bool(Editline *, StringList &)>;
 
-typedef int (*FixIndentationCallbackType)(Editline *editline,
-                                          const StringList &lines,
-                                          int cursor_position, void *baton);
+using FixIndentationCallbackType =
+    llvm::unique_function<int(Editline *, StringList &, int)>;
 
-typedef llvm::Optional<std::string> (*SuggestionCallbackType)(
-    llvm::StringRef line, void *baton);
+using SuggestionCallbackType =
+    llvm::unique_function<llvm::Optional<std::string>(llvm::StringRef)>;
 
-typedef void (*CompleteCallbackType)(CompletionRequest &request, void *baton);
+using CompleteCallbackType = llvm::unique_function<void(CompletionRequest &)>;
 
 /// Status used to decide when and how to start editing another line in
 /// multi-line sessions
@@ -188,21 +189,28 @@ class Editline {
   bool Cancel();
 
   /// Register a callback for autosuggestion.
-  void SetSuggestionCallback(SuggestionCallbackType callback, void *baton);
+  void SetSuggestionCallback(SuggestionCallbackType callback) {
+    m_suggestion_callback = std::move(callback);
+  }
 
   /// Register a callback for the tab key
-  void SetAutoCompleteCallback(CompleteCallbackType callback, void *baton);
+  void SetAutoCompleteCallback(CompleteCallbackType callback) {
+    m_completion_callback = std::move(callback);
+  }
 
   /// Register a callback for testing whether multi-line input is complete
-  void SetIsInputCompleteCallback(IsInputCompleteCallbackType callback,
-                                  void *baton);
+  void SetIsInputCompleteCallback(IsInputCompleteCallbackType callback) {
+    m_is_input_complete_callback = std::move(callback);
+  }
 
   /// Register a callback for determining the appropriate indentation for a line
   /// when creating a newline.  An optional set of insertable characters can
-  /// also
-  /// trigger the callback.
-  bool SetFixIndentationCallback(FixIndentationCallbackType callback,
-                                 void *baton, const char *indent_chars);
+  /// also trigger the callback.
+  void SetFixIndentationCallback(FixIndentationCallbackType callback,
+                                 const char *indent_chars) {
+    m_fix_indentation_callback = std::move(callback);
+    m_fix_indentation_callback_chars = indent_chars;
+  }
 
   /// Prompts for and reads a single line of user input.
   bool GetLine(std::string &line, bool &interrupted);
@@ -365,15 +373,16 @@ class Editline {
   FILE *m_output_file;
   FILE *m_error_file;
   ConnectionFileDescriptor m_input_connection;
-  IsInputCompleteCallbackType m_is_input_complete_callback = nullptr;
-  void *m_is_input_complete_callback_baton = nullptr;
-  FixIndentationCallbackType m_fix_indentation_callback = nullptr;
-  void *m_fix_indentation_callback_baton = nullptr;
+
+  IsInputCompleteCallbackType m_is_input_complete_callback;
+
+  FixIndentationCallbackType m_fix_indentation_callback;
   const char *m_fix_indentation_callback_chars = nullptr;
-  CompleteCallbackType m_completion_callback = nullptr;
-  void *m_completion_callback_baton = nullptr;
-  SuggestionCallbackType m_suggestion_callback = nullptr;
-  void *m_suggestion_callback_baton = nullptr;
+
+  CompleteCallbackType m_completion_callback;
+
+  SuggestionCallbackType m_suggestion_callback;
+
   std::size_t m_previous_autosuggestion_size = 0;
   std::mutex m_output_mutex;
 };

diff  --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 8c654d9d8a98..22c39effce71 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -265,17 +265,31 @@ IOHandlerEditline::IOHandlerEditline(
     m_editline_up = std::make_unique<Editline>(editline_name, GetInputFILE(),
                                                GetOutputFILE(), GetErrorFILE(),
                                                m_color_prompts);
-    m_editline_up->SetIsInputCompleteCallback(IsInputCompleteCallback, this);
-    m_editline_up->SetAutoCompleteCallback(AutoCompleteCallback, this);
-    if (debugger.GetUseAutosuggestion() && debugger.GetUseColor())
-      m_editline_up->SetSuggestionCallback(SuggestionCallback, this);
+    m_editline_up->SetIsInputCompleteCallback(
+        [this](Editline *editline, StringList &lines) {
+          return this->IsInputCompleteCallback(editline, lines);
+        });
+
+    m_editline_up->SetAutoCompleteCallback([this](CompletionRequest &request) {
+      this->AutoCompleteCallback(request);
+    });
+
+    if (debugger.GetUseAutosuggestion() && debugger.GetUseColor()) {
+      m_editline_up->SetSuggestionCallback([this](llvm::StringRef line) {
+        return this->SuggestionCallback(line);
+      });
+    }
     // See if the delegate supports fixing indentation
     const char *indent_chars = delegate.IOHandlerGetFixIndentationCharacters();
     if (indent_chars) {
       // The delegate does support indentation, hook it up so when any
       // indentation character is typed, the delegate gets a chance to fix it
-      m_editline_up->SetFixIndentationCallback(FixIndentationCallback, this,
-                                               indent_chars);
+      FixIndentationCallbackType f = [this](Editline *editline,
+                                            const StringList &lines,
+                                            int cursor_position) {
+        return this->FixIndentationCallback(editline, lines, cursor_position);
+      };
+      m_editline_up->SetFixIndentationCallback(std::move(f), indent_chars);
     }
   }
 #endif
@@ -425,37 +439,23 @@ bool IOHandlerEditline::GetLine(std::string &line, bool &interrupted) {
 
 #if LLDB_ENABLE_LIBEDIT
 bool IOHandlerEditline::IsInputCompleteCallback(Editline *editline,
-                                                StringList &lines,
-                                                void *baton) {
-  IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
-  return editline_reader->m_delegate.IOHandlerIsInputComplete(*editline_reader,
-                                                              lines);
+                                                StringList &lines) {
+  return m_delegate.IOHandlerIsInputComplete(*this, lines);
 }
 
 int IOHandlerEditline::FixIndentationCallback(Editline *editline,
                                               const StringList &lines,
-                                              int cursor_position,
-                                              void *baton) {
-  IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
-  return editline_reader->m_delegate.IOHandlerFixIndentation(
-      *editline_reader, lines, cursor_position);
+                                              int cursor_position) {
+  return m_delegate.IOHandlerFixIndentation(*this, lines, cursor_position);
 }
 
 llvm::Optional<std::string>
-IOHandlerEditline::SuggestionCallback(llvm::StringRef line, void *baton) {
-  IOHandlerEditline *editline_reader = static_cast<IOHandlerEditline *>(baton);
-  if (editline_reader)
-    return editline_reader->m_delegate.IOHandlerSuggestion(*editline_reader,
-                                                           line);
-
-  return llvm::None;
+IOHandlerEditline::SuggestionCallback(llvm::StringRef line) {
+  return m_delegate.IOHandlerSuggestion(*this, line);
 }
 
-void IOHandlerEditline::AutoCompleteCallback(CompletionRequest &request,
-                                             void *baton) {
-  IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
-  if (editline_reader)
-    editline_reader->m_delegate.IOHandlerComplete(*editline_reader, request);
+void IOHandlerEditline::AutoCompleteCallback(CompletionRequest &request) {
+  m_delegate.IOHandlerComplete(*this, request);
 }
 #endif
 

diff  --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 026a05da45b2..fa06bfb26583 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -9,8 +9,9 @@
 #include <iomanip>
 #include <limits.h>
 
-#include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/Editline.h"
+
+#include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/CompletionRequest.h"
@@ -641,8 +642,7 @@ unsigned char Editline::BreakLineCommand(int ch) {
       lines.AppendString(new_line_fragment);
 #endif
 
-      int indent_correction = m_fix_indentation_callback(
-          this, lines, 0, m_fix_indentation_callback_baton);
+      int indent_correction = m_fix_indentation_callback(this, lines, 0);
       new_line_fragment = FixIndentation(new_line_fragment, indent_correction);
       m_revert_cursor_index = GetIndentation(new_line_fragment);
     }
@@ -677,8 +677,7 @@ unsigned char Editline::EndOrAddLineCommand(int ch) {
       info->cursor == info->lastchar) {
     if (m_is_input_complete_callback) {
       auto lines = GetInputAsStringList();
-      if (!m_is_input_complete_callback(this, lines,
-                                        m_is_input_complete_callback_baton)) {
+      if (!m_is_input_complete_callback(this, lines)) {
         return BreakLineCommand(ch);
       }
 
@@ -811,8 +810,7 @@ unsigned char Editline::NextLineCommand(int ch) {
     if (m_fix_indentation_callback) {
       StringList lines = GetInputAsStringList();
       lines.AppendString("");
-      indentation = m_fix_indentation_callback(
-          this, lines, 0, m_fix_indentation_callback_baton);
+      indentation = m_fix_indentation_callback(this, lines, 0);
     }
     m_input_lines.insert(
         m_input_lines.end(),
@@ -857,8 +855,8 @@ unsigned char Editline::FixIndentationCommand(int ch) {
   // Save the edits and determine the correct indentation level
   SaveEditedLine();
   StringList lines = GetInputAsStringList(m_current_line_index + 1);
-  int indent_correction = m_fix_indentation_callback(
-      this, lines, cursor_position, m_fix_indentation_callback_baton);
+  int indent_correction =
+      m_fix_indentation_callback(this, lines, cursor_position);
 
   // If it is already correct no special work is needed
   if (indent_correction == 0)
@@ -977,7 +975,7 @@ DisplayCompletions(::EditLine *editline, FILE *output_file,
 }
 
 unsigned char Editline::TabCommand(int ch) {
-  if (m_completion_callback == nullptr)
+  if (!m_completion_callback)
     return CC_ERROR;
 
   const LineInfo *line_info = el_line(m_editline);
@@ -988,7 +986,7 @@ unsigned char Editline::TabCommand(int ch) {
   CompletionResult result;
   CompletionRequest request(line, cursor_index, result);
 
-  m_completion_callback(request, m_completion_callback_baton);
+  m_completion_callback(request);
 
   llvm::ArrayRef<CompletionResult::Completion> results = result.GetResults();
 
@@ -1047,12 +1045,15 @@ unsigned char Editline::TabCommand(int ch) {
 }
 
 unsigned char Editline::ApplyAutosuggestCommand(int ch) {
+  if (!m_suggestion_callback) {
+    return CC_REDISPLAY;
+  }
+
   const LineInfo *line_info = el_line(m_editline);
   llvm::StringRef line(line_info->buffer,
                        line_info->lastchar - line_info->buffer);
 
-  if (llvm::Optional<std::string> to_add =
-          m_suggestion_callback(line, m_suggestion_callback_baton))
+  if (llvm::Optional<std::string> to_add = m_suggestion_callback(line))
     el_insertstr(m_editline, to_add->c_str());
 
   return CC_REDISPLAY;
@@ -1061,12 +1062,16 @@ unsigned char Editline::ApplyAutosuggestCommand(int ch) {
 unsigned char Editline::TypedCharacter(int ch) {
   std::string typed = std::string(1, ch);
   el_insertstr(m_editline, typed.c_str());
+
+  if (!m_suggestion_callback) {
+    return CC_REDISPLAY;
+  }
+
   const LineInfo *line_info = el_line(m_editline);
   llvm::StringRef line(line_info->buffer,
                        line_info->lastchar - line_info->buffer);
 
-  if (llvm::Optional<std::string> to_add =
-          m_suggestion_callback(line, m_suggestion_callback_baton)) {
+  if (llvm::Optional<std::string> to_add = m_suggestion_callback(line)) {
     std::string to_add_color = ANSI_FAINT + to_add.getValue() + ANSI_UNFAINT;
     fputs(typed.c_str(), m_output_file);
     fputs(to_add_color.c_str(), m_output_file);
@@ -1447,33 +1452,6 @@ bool Editline::Cancel() {
   return result;
 }
 
-void Editline::SetSuggestionCallback(SuggestionCallbackType callback,
-                                     void *baton) {
-  m_suggestion_callback = callback;
-  m_suggestion_callback_baton = baton;
-}
-
-void Editline::SetAutoCompleteCallback(CompleteCallbackType callback,
-                                       void *baton) {
-  m_completion_callback = callback;
-  m_completion_callback_baton = baton;
-}
-
-void Editline::SetIsInputCompleteCallback(IsInputCompleteCallbackType callback,
-                                          void *baton) {
-  m_is_input_complete_callback = callback;
-  m_is_input_complete_callback_baton = baton;
-}
-
-bool Editline::SetFixIndentationCallback(FixIndentationCallbackType callback,
-                                         void *baton,
-                                         const char *indent_chars) {
-  m_fix_indentation_callback = callback;
-  m_fix_indentation_callback_baton = baton;
-  m_fix_indentation_callback_chars = indent_chars;
-  return false;
-}
-
 bool Editline::GetLine(std::string &line, bool &interrupted) {
   ConfigureEditor(false);
   m_input_lines = std::vector<EditLineStringType>();

diff  --git a/lldb/unittests/Editline/EditlineTest.cpp b/lldb/unittests/Editline/EditlineTest.cpp
index 8dbe30e97878..531848114504 100644
--- a/lldb/unittests/Editline/EditlineTest.cpp
+++ b/lldb/unittests/Editline/EditlineTest.cpp
@@ -81,8 +81,8 @@ class EditlineAdapter {
   void ConsumeAllOutput();
 
 private:
-  static bool IsInputComplete(lldb_private::Editline *editline,
-                              lldb_private::StringList &lines, void *baton);
+  bool IsInputComplete(lldb_private::Editline *editline,
+                       lldb_private::StringList &lines);
 
   std::unique_ptr<lldb_private::Editline> _editline_sp;
 
@@ -122,7 +122,10 @@ EditlineAdapter::EditlineAdapter()
   _editline_sp->SetPrompt("> ");
 
   // Hookup our input complete callback.
-  _editline_sp->SetIsInputCompleteCallback(IsInputComplete, this);
+  auto input_complete_cb = [this](Editline *editline, StringList &lines) {
+    return this->IsInputComplete(editline, lines);
+  };
+  _editline_sp->SetIsInputCompleteCallback(input_complete_cb);
 }
 
 void EditlineAdapter::CloseInput() {
@@ -183,8 +186,7 @@ bool EditlineAdapter::GetLines(lldb_private::StringList &lines,
 }
 
 bool EditlineAdapter::IsInputComplete(lldb_private::Editline *editline,
-                                      lldb_private::StringList &lines,
-                                      void *baton) {
+                                      lldb_private::StringList &lines) {
   // We'll call ourselves complete if we've received a balanced set of braces.
   int start_block_count = 0;
   int brace_balance = 0;


        


More information about the lldb-commits mailing list