Complete breakpoint enable/disable/delete/modify with a list of breakpoint IDs

[MrHate]

Summary

1. A new common completion CommandCompletions::Breakpoints to provide a list of the breakpoints of the current context;
2. Apply the completion above to the commands breakpoint enable/disable/delete/modify;
3. Unit test.

[JDevlieghere]

A few small comments but this is looking pretty good to me.

lldb/source/Commands/CommandCompletions.cpp

558

GetSelectedTarget() returns a TargetSP. It seems like all the operations below can be performed on the shared pointer.

I think if you change this line you don't even need to change anything below.

TargetSP target = interpreter.GetDebugger().GetSelectedTarget();

lldb/test/API/functionalities/completion/TestCompletion.py

486

nit: missing period

497

It might be nice to split this up into self.assertTrue(bp) and self.assertEqual(bp.GetNumLocations(), 1). That way the error message will be a bit more informative if either fails. The same applies to bp2 below.

[MrHate]

Thank you for your advice Jonas.
Btw, I also modified the raw Breakpoint pointer to be a BreakpointSP.

But I don't really understand your meaning of the comment "nit: missing period".

[teemperor]

LGTM, thanks! I'll just add the two missing periods when committing.

[jingham]

This is great, but you can also specify breakpoints by name. Should be possible to also complete on the list of breakpoint names.

lldb/include/lldb/Interpreter/CommandCompletions.h

enum CommonCompletionTypes {
     eDiskDirectoryCompletion = (1u << 2),
     eSymbolCompletion = (1u << 3),
     eModuleCompletion = (1u << 4),
     eSettingsNameCompletion = (1u << 5),
     ePlatformPluginCompletion = (1u << 6),
     eArchitectureCompletion = (1u << 7),
     eVariablePathCompletion = (1u << 8),
     eRegisterCompletion = (1u << 9),
+    eBreakpointCompletion = (1u << 10),
     // This item serves two purposes.  It is the last element in the enum, so
     // you can add custom enums starting from here in your Option class. Also
     // if you & in this bit the base code will not process the option.
-    eCustomCompletion = (1u << 10)
+    eCustomCompletion = (1u << 11)
   };
 
   static bool InvokeCommonCompletionCallbacks(
       CommandInterpreter &interpreter, uint32_t completion_mask,
       lldb_private::CompletionRequest &request, SearchFilter *searcher);
 
   // These are the generic completer functions:
   static void DiskFiles(CommandInterpreter &interpreter,
static void ArchitectureNames(CommandInterpreter &interpreter,
                                 CompletionRequest &request,
                                 SearchFilter *searcher);
 
   static void VariablePath(CommandInterpreter &interpreter,
                            CompletionRequest &request, SearchFilter *searcher);
 
   static void Registers(CommandInterpreter &interpreter,
                         CompletionRequest &request, SearchFilter *searcher);
+
+  static void Breakpoints(CommandInterpreter &interpreter,
+                          CompletionRequest &request, SearchFilter *searcher);
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_COMMANDCOMPLETIONS_H

lldb/source/Commands/CommandCompletions.cpp

const CommonCompletionElement common_completions[] = {
       {eDiskDirectoryCompletion, CommandCompletions::DiskDirectories},
       {eSymbolCompletion, CommandCompletions::Symbols},
       {eModuleCompletion, CommandCompletions::Modules},
       {eSettingsNameCompletion, CommandCompletions::SettingsNames},
       {ePlatformPluginCompletion, CommandCompletions::PlatformPluginNames},
       {eArchitectureCompletion, CommandCompletions::ArchitectureNames},
       {eVariablePathCompletion, CommandCompletions::VariablePath},
       {eRegisterCompletion, CommandCompletions::Registers},
+      {eBreakpointCompletion, CommandCompletions::Breakpoints},
       {eNoCompletion, nullptr} // This one has to be last in the list.
   };
 
   for (int i = 0;; i++) {
     if (common_completions[i].type == eNoCompletion)
       break;
     else if ((common_completions[i].type & completion_mask) ==
                  common_completions[i].type &&
void CommandCompletions::Registers(CommandInterpreter &interpreter,
   RegisterContext *reg_ctx =
       interpreter.GetExecutionContext().GetRegisterContext();
   const size_t reg_num = reg_ctx->GetRegisterCount();
   for (size_t reg_idx = 0; reg_idx < reg_num; ++reg_idx) {
     const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(reg_idx);
     request.TryCompleteCurrentArg(reg_prefix + reg_info->name,
                                   reg_info->alt_name);
   }
-}
+}
+
+void CommandCompletions::Breakpoints(CommandInterpreter &interpreter,
+                                     CompletionRequest &request,
+                                     SearchFilter *searcher) {
+  lldb::TargetSP target = interpreter.GetDebugger().GetSelectedTarget();
+  if (!target)
+    return;
+
+  const BreakpointList &breakpoints = target->GetBreakpointList();
+
+  std::unique_lock<std::recursive_mutex> lock;
+  target->GetBreakpointList().GetListMutex(lock);
+
+  size_t num_breakpoints = breakpoints.GetSize();
+  if (num_breakpoints == 0)
+    return;
+
+  for (size_t i = 0; i < num_breakpoints; ++i) {
+    lldb::BreakpointSP bp = breakpoints.GetBreakpointAtIndex(i);
+
+    StreamString s;
+    bp->GetDescription(&s, lldb::eDescriptionLevelBrief);
+    llvm::StringRef bp_info = s.GetString();
+
+    const size_t colon_pos = bp_info.find_first_of(':');
+    if (colon_pos != llvm::StringRef::npos)
+      bp_info = bp_info.drop_front(colon_pos + 2);
+
+    request.TryCompleteCurrentArg(std::to_string(bp->GetID()), bp_info);
+  }
+}

lldb/source/Commands/CommandObjectBreakpoint.cpp

m_options.Append(&m_bp_opts,
                      LLDB_OPT_SET_1 | LLDB_OPT_SET_2 | LLDB_OPT_SET_3,
                      LLDB_OPT_SET_ALL);
     m_options.Append(&m_dummy_opts, LLDB_OPT_SET_1, LLDB_OPT_SET_ALL);
     m_options.Finalize();
   }
 
   ~CommandObjectBreakpointModify() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), CommandCompletions::eBreakpointCompletion,
+        request, nullptr);
+  }
+
   Options *GetOptions() override { return &m_options; }
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target &target = GetSelectedOrDummyTarget(m_dummy_opts.m_use_dummy);
 
     std::unique_lock<std::recursive_mutex> lock;
     target.GetBreakpointList().GetListMutex(lock);
CommandObject::AddIDsArgumentData(arg, eArgTypeBreakpointID,
                                       eArgTypeBreakpointIDRange);
     // Add the entry for the first argument for this command to the object's
     // arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectBreakpointEnable() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), CommandCompletions::eBreakpointCompletion,
+        request, nullptr);
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target &target = GetSelectedOrDummyTarget();
 
     std::unique_lock<std::recursive_mutex> lock;
     target.GetBreakpointList().GetListMutex(lock);
 
     const BreakpointList &breakpoints = target.GetBreakpointList();
CommandObject::AddIDsArgumentData(arg, eArgTypeBreakpointID,
                                       eArgTypeBreakpointIDRange);
     // Add the entry for the first argument for this command to the object's
     // arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectBreakpointDisable() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), CommandCompletions::eBreakpointCompletion,
+        request, nullptr);
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target &target = GetSelectedOrDummyTarget();
     std::unique_lock<std::recursive_mutex> lock;
     target.GetBreakpointList().GetListMutex(lock);
 
     const BreakpointList &breakpoints = target.GetBreakpointList();
     size_t num_breakpoints = breakpoints.GetSize();
CommandObject::AddIDsArgumentData(arg, eArgTypeBreakpointID,
                                       eArgTypeBreakpointIDRange);
     // Add the entry for the first argument for this command to the object's
     // arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectBreakpointDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), CommandCompletions::eBreakpointCompletion,
+        request, nullptr);
+  }
+
   Options *GetOptions() override { return &m_options; }
 
   class CommandOptions : public Options {
   public:
     CommandOptions() : Options(), m_use_dummy(false), m_force(false) {}
 
     ~CommandOptions() override = default;
 

lldb/test/API/functionalities/completion/TestCompletion.py

def test_register_read_and_write_on_x86(self):
         self.complete_from_to('register write ra',
                               'register write rax')
         self.complete_from_to('register write rb',
                               ['rbx',
                                'rbp'])
         # register write can only take exact one register name as argument
         self.complete_from_to('register write rbx ',
                               [])
+
+    def test_breakpoint_enable_disable_delete_modify_with_ids(self):
+        """These four breakpoint subcommands should be completed with a list of breakpoint ids"""
+
+        subcommands = ['enable', 'disable', 'delete', 'modify']
+
+        # The tab completion here is unavailable without a target
+        for subcommand in subcommands:
+            self.complete_from_to('breakpoint ' + subcommand + ' ',
+                                  'breakpoint ' + subcommand + ' ')
+
+        self.build()
+        target = self.dbg.CreateTarget(self.getBuildArtifact('a.out'))
+        self.assertTrue(target, VALID_TARGET)
+
+        bp = target.BreakpointCreateByName('main', 'a.out')
+        self.assertTrue(bp)
+        self.assertEqual(bp.GetNumLocations(), 1)
+
+        for subcommand in subcommands:
+            self.complete_from_to('breakpoint ' + subcommand + ' ',
+                                  ['1'])
+        
+        bp2 = target.BreakpointCreateByName('Bar', 'a.out')
+        self.assertTrue(bp2)
+        self.assertEqual(bp2.GetNumLocations(), 1)
+
+        for subcommand in subcommands:
+            self.complete_from_to('breakpoint ' + subcommand + ' ',
+                                  ['1',
+                                   '2'])
+        
+        for subcommand in subcommands:
+            self.complete_from_to('breakpoint ' + subcommand + ' 1 ',
+                                  ['1',
+                                   '2'])
+