Fix a logic error in "break delete --disabled" when no "protected" breakpoints are specified.

[jingham]

Summary

Fix "break delete --disabled" with no arguments.

The code that figured out which breakpoints to delete was supposed
to set the result status if it found breakpoints, and then the code
that actually deleted them checked that the result's status was set.

The code for "break delete --disabled" failed to set the status if
no "protected" breakpoints were provided. This was a confusing way
to implement this, so I reworked it with early returns so it was less
error prone, and added a test case for the no arguments case.

lldb/source/Commands/CommandObjectBreakpoint.cpp

bool DoExecute(Args &command, CommandReturnObject &result) override {
 
     size_t num_breakpoints = breakpoints.GetSize();
 
     if (num_breakpoints == 0) {
       result.AppendError("No breakpoints exist to be deleted.");
       return false;
     }
 
+    // Handle the delete all breakpoints case:
     if (command.empty() && !m_options.m_delete_disabled) {
       if (!m_options.m_force &&
           !m_interpreter.Confirm(
               "About to delete all breakpoints, do you want to do that?",
               true)) {
         result.AppendMessage("Operation cancelled...");
       } else {
         target.RemoveAllowedBreakpoints();
         result.AppendMessageWithFormat(
             "All breakpoints removed. (%" PRIu64 " breakpoint%s)\n",
             (uint64_t)num_breakpoints, num_breakpoints > 1 ? "s" : "");
       }
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    } else {
-      // Particular breakpoint selected; disable that breakpoint.
-      BreakpointIDList valid_bp_ids;
-      
-      if (m_options.m_delete_disabled) {
-        BreakpointIDList excluded_bp_ids;
+      return result.Succeeded();
+    }
+ 
+    // Either we have some kind of breakpoint specification(s),
+    // or we are handling "break disable --deleted".  Gather the list
+    // of breakpoints to delete here, the we'll delete them below.
 
-        if (!command.empty()) {
-          CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-              command, &target, result, &excluded_bp_ids,
-              BreakpointName::Permissions::PermissionKinds::deletePerm);
-        }
-        for (auto breakpoint_sp : breakpoints.Breakpoints()) {
-          if (!breakpoint_sp->IsEnabled() && breakpoint_sp->AllowDelete()) {
-            BreakpointID bp_id(breakpoint_sp->GetID());
-            size_t pos = 0;
-            if (!excluded_bp_ids.FindBreakpointID(bp_id, &pos))
-              valid_bp_ids.AddBreakpointID(breakpoint_sp->GetID());
-          }
-        }
-        if (valid_bp_ids.GetSize() == 0) {
-          result.AppendError("No disabled breakpoints.");
-          return false;
-        }
-      } else {
-        CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-            command, &target, result, &valid_bp_ids,
-            BreakpointName::Permissions::PermissionKinds::deletePerm);
-      }
-      
-      if (result.Succeeded()) {
-        int delete_count = 0;
-        int disable_count = 0;
-        const size_t count = valid_bp_ids.GetSize();
-        for (size_t i = 0; i < count; ++i) {
-          BreakpointID cur_bp_id = valid_bp_ids.GetBreakpointIDAtIndex(i);
+        if (!result.Succeeded())
+          return false;
+
+      if (!result.Succeeded())
+        return false;
 
-          if (cur_bp_id.GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
-            if (cur_bp_id.GetLocationID() != LLDB_INVALID_BREAK_ID) {
-              Breakpoint *breakpoint =
-                  target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
-              BreakpointLocation *location =
-                  breakpoint->FindLocationByID(cur_bp_id.GetLocationID()).get();
-              // It makes no sense to try to delete individual locations, so we
-              // disable them instead.
-              if (location) {
-                location->SetEnabled(false);
-                ++disable_count;
-              }
-            } else {
-              target.RemoveBreakpointByID(cur_bp_id.GetBreakpointID());
-              ++delete_count;
-            }
-          }
-        }
-        result.AppendMessageWithFormat(
-            "%d breakpoints deleted; %d breakpoint locations disabled.\n",
-            delete_count, disable_count);
-        result.SetStatus(eReturnStatusSuccessFinishNoResult);
-      }
-    }
     return result.Succeeded();
   }
 
 private:
   CommandOptions m_options;
 };
 
 // CommandObjectBreakpointName

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

def test_breakpoint_delete_disabled(self):
         bp_1 = target.FindBreakpointByID(bp_id_1)
         self.assertFalse(bp_1.IsValid(), "Didn't delete disabled breakpoint 1")
 
         bp_2 = target.FindBreakpointByID(bp_id_2)
         self.assertTrue(bp_2.IsValid(), "Deleted enabled breakpoint 2")
 
         bp_3 = target.FindBreakpointByID(bp_id_3)
         self.assertTrue(bp_3.IsValid(), "DeleteMeNot didn't protect disabled breakpoint 3")
+
+        # Reset the first breakpoint, disable it, and do this again with no protected name:
+        bp_1 = target.BreakpointCreateByName("main")
+
+        bp_1.SetEnabled(False)
+
+        bp_id_1 = bp_1.GetID()
+        
+        self.runCmd("breakpoint delete --disabled")
+
+        bp_1 = target.FindBreakpointByID(bp_id_1)
+        self.assertFalse(bp_1.IsValid(), "Didn't delete disabled breakpoint 1")
+
+        bp_2 = target.FindBreakpointByID(bp_id_2)
+        self.assertTrue(bp_2.IsValid(), "Deleted enabled breakpoint 2")
+
+        bp_3 = target.FindBreakpointByID(bp_id_3)
+        self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+