[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.
This commit is contained in:
Alex Langford 2024-01-12 13:52:59 -08:00 committed by GitHub
parent 4618ef8cf5
commit c8ef88c446
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 61 additions and 63 deletions

View File

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

View File

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

View File

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