[Lldb-commits] [lldb] c8ef88c - [lldb][NFCI] Remove CommandReturnObject from BreakpointIDList (#77858)

via lldb-commits lldb-commits at lists.llvm.org
Fri Jan 12 13:53:04 PST 2024


Author: Alex Langford
Date: 2024-01-12T13:52:59-08:00
New Revision: c8ef88c446a3ff773c5be2fbf3df84b8b40c0c41

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

LOG: [lldb][NFCI] Remove CommandReturnObject from BreakpointIDList (#77858)

BreakpointIDList does not need to know about CommandReturnObject.
BreakpointIDList::FindAndReplaceIDRanges is the last place that uses it
in BreakpointIDList.

Instead of passing in a CommandReturnObject, it now returns an
llvm::Error. The callsite uses the Error to populate the
CommandReturnObject as needed.

Added: 
    

Modified: 
    lldb/include/lldb/Breakpoint/BreakpointIDList.h
    lldb/source/Breakpoint/BreakpointIDList.cpp
    lldb/source/Commands/CommandObjectBreakpoint.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Breakpoint/BreakpointIDList.h b/lldb/include/lldb/Breakpoint/BreakpointIDList.h
index 6910024695d898..3cda1860dc1672 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointIDList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointIDList.h
@@ -12,12 +12,13 @@
 #include <utility>
 #include <vector>
 
-
-#include "lldb/lldb-enumerations.h"
 #include "lldb/Breakpoint/BreakpointID.h"
 #include "lldb/Breakpoint/BreakpointName.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/Support/Error.h"
+
 namespace lldb_private {
 
 // class BreakpointIDList
@@ -54,12 +55,10 @@ class BreakpointIDList {
   static std::pair<llvm::StringRef, llvm::StringRef>
   SplitIDRangeExpression(llvm::StringRef in_string);
 
-  static void FindAndReplaceIDRanges(Args &old_args, Target *target,
-                                     bool allow_locations,
-                                     BreakpointName::Permissions
-                                       ::PermissionKinds purpose,
-                                     CommandReturnObject &result,
-                                     Args &new_args);
+  static llvm::Error
+  FindAndReplaceIDRanges(Args &old_args, Target *target, bool allow_locations,
+                         BreakpointName::Permissions ::PermissionKinds purpose,
+                         Args &new_args);
 
 private:
   BreakpointIDArray m_breakpoint_ids;

diff  --git a/lldb/source/Breakpoint/BreakpointIDList.cpp b/lldb/source/Breakpoint/BreakpointIDList.cpp
index 05c461827cadd8..51185c30dabad8 100644
--- a/lldb/source/Breakpoint/BreakpointIDList.cpp
+++ b/lldb/source/Breakpoint/BreakpointIDList.cpp
@@ -11,9 +11,9 @@
 
 #include "lldb/Breakpoint/Breakpoint.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
-#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Args.h"
+#include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -93,12 +93,9 @@ bool BreakpointIDList::FindBreakpointID(const char *bp_id_str,
 //  NEW_ARGS should be a copy of OLD_ARGS, with and ID range specifiers replaced
 //  by the members of the range.
 
-void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
-                                              bool allow_locations,
-                                              BreakpointName::Permissions
-                                                  ::PermissionKinds purpose,
-                                              CommandReturnObject &result,
-                                              Args &new_args) {
+llvm::Error BreakpointIDList::FindAndReplaceIDRanges(
+    Args &old_args, Target *target, bool allow_locations,
+    BreakpointName::Permissions ::PermissionKinds purpose, Args &new_args) {
   llvm::StringRef range_from;
   llvm::StringRef range_to;
   llvm::StringRef current_arg;
@@ -109,11 +106,11 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
 
     current_arg = old_args[i].ref();
     if (!allow_locations && current_arg.contains('.')) {
-      result.AppendErrorWithFormat(
+      new_args.Clear();
+      return llvm::createStringError(
+          llvm::inconvertibleErrorCode(),
           "Breakpoint locations not allowed, saw location: %s.",
           current_arg.str().c_str());
-      new_args.Clear();
-      return;
     }
 
     Status error;
@@ -125,8 +122,8 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
     } else if (BreakpointID::StringIsBreakpointName(current_arg, error)) {
       if (!error.Success()) {
         new_args.Clear();
-        result.AppendError(error.AsCString());
-        return;
+        return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                       error.AsCString());
       } else
         names_found.insert(std::string(current_arg));
     } else if ((i + 2 < old_args.size()) &&
@@ -152,9 +149,10 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
             breakpoint_sp = target->GetBreakpointByID(bp_id->GetBreakpointID());
           if (!breakpoint_sp) {
             new_args.Clear();
-            result.AppendErrorWithFormat("'%d' is not a valid breakpoint ID.\n",
-                                         bp_id->GetBreakpointID());
-            return;
+            return llvm::createStringError(
+                llvm::inconvertibleErrorCode(),
+                "'%d' is not a valid breakpoint ID.\n",
+                bp_id->GetBreakpointID());
           }
           const size_t num_locations = breakpoint_sp->GetNumLocations();
           for (size_t j = 0; j < num_locations; ++j) {
@@ -180,17 +178,17 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
     if (!start_bp ||
         !target->GetBreakpointByID(start_bp->GetBreakpointID())) {
       new_args.Clear();
-      result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n",
-                                   range_from.str().c_str());
-      return;
+      return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                     "'%s' is not a valid breakpoint ID.\n",
+                                     range_from.str().c_str());
     }
 
     if (!end_bp ||
         !target->GetBreakpointByID(end_bp->GetBreakpointID())) {
       new_args.Clear();
-      result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n",
-                                   range_to.str().c_str());
-      return;
+      return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                     "'%s' is not a valid breakpoint ID.\n",
+                                     range_to.str().c_str());
     }
     break_id_t start_bp_id = start_bp->GetBreakpointID();
     break_id_t start_loc_id = start_bp->GetLocationID();
@@ -201,11 +199,11 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
         ((start_loc_id != LLDB_INVALID_BREAK_ID) &&
          (end_loc_id == LLDB_INVALID_BREAK_ID))) {
       new_args.Clear();
-      result.AppendError("Invalid breakpoint id range:  Either "
-                         "both ends of range must specify"
-                         " a breakpoint location, or neither can "
-                         "specify a breakpoint location.");
-      return;
+      return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                     "Invalid breakpoint id range:  Either "
+                                     "both ends of range must specify"
+                                     " a breakpoint location, or neither can "
+                                     "specify a breakpoint location.");
     }
 
     // We have valid range starting & ending breakpoint IDs.  Go through all
@@ -221,13 +219,13 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
         (end_loc_id != LLDB_INVALID_BREAK_ID)) {
       if (start_bp_id != end_bp_id) {
         new_args.Clear();
-        result.AppendErrorWithFormat(
+        return llvm::createStringError(
+            llvm::inconvertibleErrorCode(),
             "Invalid range: Ranges that specify particular breakpoint "
             "locations"
             " must be within the same major breakpoint; you specified two"
             " 
diff erent major breakpoints, %d and %d.\n",
             start_bp_id, end_bp_id);
-        return;
       }
     }
 
@@ -302,8 +300,7 @@ void BreakpointIDList::FindAndReplaceIDRanges(Args &old_args, Target *target,
       }
     }
   }
-
-  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+  return llvm::Error::success();
 }
 
 std::pair<llvm::StringRef, llvm::StringRef>

diff  --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index f9ba68eda3ff1f..1661d5d9b743e2 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -2488,8 +2488,12 @@ void CommandObjectMultiwordBreakpoint::VerifyIDs(
   // breakpoint ids in the range, and shove all of those breakpoint id strings
   // into TEMP_ARGS.
 
-  BreakpointIDList::FindAndReplaceIDRanges(args, target, allow_locations,
-                                           purpose, result, temp_args);
+  if (llvm::Error err = BreakpointIDList::FindAndReplaceIDRanges(
+          args, target, allow_locations, purpose, temp_args)) {
+    result.SetError(std::move(err));
+    return;
+  }
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
 
   // NOW, convert the list of breakpoint id strings in TEMP_ARGS into an actual
   // BreakpointIDList:
@@ -2501,33 +2505,31 @@ void CommandObjectMultiwordBreakpoint::VerifyIDs(
   // At this point,  all of the breakpoint ids that the user passed in have
   // been converted to breakpoint IDs and put into valid_ids.
 
-  if (result.Succeeded()) {
-    // Now that we've converted everything from args into a list of breakpoint
-    // ids, go through our tentative list of breakpoint id's and verify that
-    // they correspond to valid/currently set breakpoints.
-
-    const size_t count = valid_ids->GetSize();
-    for (size_t i = 0; i < count; ++i) {
-      BreakpointID cur_bp_id = valid_ids->GetBreakpointIDAtIndex(i);
-      Breakpoint *breakpoint =
-          target->GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
-      if (breakpoint != nullptr) {
-        const size_t num_locations = breakpoint->GetNumLocations();
-        if (static_cast<size_t>(cur_bp_id.GetLocationID()) > num_locations) {
-          StreamString id_str;
-          BreakpointID::GetCanonicalReference(
-              &id_str, cur_bp_id.GetBreakpointID(), cur_bp_id.GetLocationID());
-          i = valid_ids->GetSize() + 1;
-          result.AppendErrorWithFormat(
-              "'%s' is not a currently valid breakpoint/location id.\n",
-              id_str.GetData());
-        }
-      } else {
+  // Now that we've converted everything from args into a list of breakpoint
+  // ids, go through our tentative list of breakpoint id's and verify that
+  // they correspond to valid/currently set breakpoints.
+
+  const size_t count = valid_ids->GetSize();
+  for (size_t i = 0; i < count; ++i) {
+    BreakpointID cur_bp_id = valid_ids->GetBreakpointIDAtIndex(i);
+    Breakpoint *breakpoint =
+        target->GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
+    if (breakpoint != nullptr) {
+      const size_t num_locations = breakpoint->GetNumLocations();
+      if (static_cast<size_t>(cur_bp_id.GetLocationID()) > num_locations) {
+        StreamString id_str;
+        BreakpointID::GetCanonicalReference(
+            &id_str, cur_bp_id.GetBreakpointID(), cur_bp_id.GetLocationID());
         i = valid_ids->GetSize() + 1;
         result.AppendErrorWithFormat(
-            "'%d' is not a currently valid breakpoint ID.\n",
-            cur_bp_id.GetBreakpointID());
+            "'%s' is not a currently valid breakpoint/location id.\n",
+            id_str.GetData());
       }
+    } else {
+      i = valid_ids->GetSize() + 1;
+      result.AppendErrorWithFormat(
+          "'%d' is not a currently valid breakpoint ID.\n",
+          cur_bp_id.GetBreakpointID());
     }
   }
 }


        


More information about the lldb-commits mailing list