Skip to content

Commit 0ef66fc

Browse files
authored
[lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (#83086)
This updates the remaining SetOptionValue methods in CommandObjectBreakpoint to use CreateOptionParsingError. I found a few minor bugs that were fixed during this refactor (e.g. using the wrong flag in an error message). That is one of the benefits of centralizing error message creation. I also found some option parsing code that is written incorrectly. I do not make an attempt to update those here because this PR is primarily about changing existing error handling code, not adding new error handling code.
1 parent 12df1cf commit 0ef66fc

File tree

2 files changed

+54
-49
lines changed

2 files changed

+54
-49
lines changed

lldb/include/lldb/Interpreter/Options.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,8 @@ static constexpr llvm::StringLiteral g_bool_parsing_error_message =
368368
"Failed to parse as boolean";
369369
static constexpr llvm::StringLiteral g_int_parsing_error_message =
370370
"Failed to parse as integer";
371+
static constexpr llvm::StringLiteral g_language_parsing_error_message =
372+
"Unknown language";
371373

372374
} // namespace lldb_private
373375

lldb/source/Commands/CommandObjectBreakpoint.cpp

Lines changed: 52 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
266266
Status error;
267267
const int short_option =
268268
g_breakpoint_set_options[option_idx].short_option;
269+
const char *long_option =
270+
g_breakpoint_set_options[option_idx].long_option;
269271

270272
switch (short_option) {
271273
case 'a': {
@@ -284,13 +286,15 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
284286

285287
case 'u':
286288
if (option_arg.getAsInteger(0, m_column))
287-
error.SetErrorStringWithFormat("invalid column number: %s",
288-
option_arg.str().c_str());
289+
error =
290+
CreateOptionParsingError(option_arg, short_option, long_option,
291+
g_int_parsing_error_message);
289292
break;
290293

291294
case 'E': {
292295
LanguageType language = Language::GetLanguageTypeFromString(option_arg);
293296

297+
llvm::StringRef error_context;
294298
switch (language) {
295299
case eLanguageTypeC89:
296300
case eLanguageTypeC:
@@ -308,19 +312,18 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
308312
m_exception_language = eLanguageTypeObjC;
309313
break;
310314
case eLanguageTypeObjC_plus_plus:
311-
error.SetErrorStringWithFormat(
312-
"Set exception breakpoints separately for c++ and objective-c");
315+
error_context =
316+
"Set exception breakpoints separately for c++ and objective-c";
313317
break;
314318
case eLanguageTypeUnknown:
315-
error.SetErrorStringWithFormat(
316-
"Unknown language type: '%s' for exception breakpoint",
317-
option_arg.str().c_str());
319+
error_context = "Unknown language type for exception breakpoint";
318320
break;
319321
default:
320-
error.SetErrorStringWithFormat(
321-
"Unsupported language type: '%s' for exception breakpoint",
322-
option_arg.str().c_str());
322+
error_context = "Unsupported language type for exception breakpoint";
323323
}
324+
if (!error_context.empty())
325+
error = CreateOptionParsingError(option_arg, short_option,
326+
long_option, error_context);
324327
} break;
325328

326329
case 'f':
@@ -336,9 +339,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
336339
bool success;
337340
m_catch_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
338341
if (!success)
339-
error.SetErrorStringWithFormat(
340-
"Invalid boolean value for on-catch option: '%s'",
341-
option_arg.str().c_str());
342+
error =
343+
CreateOptionParsingError(option_arg, short_option, long_option,
344+
g_bool_parsing_error_message);
342345
} break;
343346

344347
case 'H':
@@ -355,23 +358,24 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
355358
m_skip_prologue = eLazyBoolNo;
356359

357360
if (!success)
358-
error.SetErrorStringWithFormat(
359-
"Invalid boolean value for skip prologue option: '%s'",
360-
option_arg.str().c_str());
361+
error =
362+
CreateOptionParsingError(option_arg, short_option, long_option,
363+
g_bool_parsing_error_message);
361364
} break;
362365

363366
case 'l':
364367
if (option_arg.getAsInteger(0, m_line_num))
365-
error.SetErrorStringWithFormat("invalid line number: %s.",
366-
option_arg.str().c_str());
368+
error =
369+
CreateOptionParsingError(option_arg, short_option, long_option,
370+
g_int_parsing_error_message);
367371
break;
368372

369373
case 'L':
370374
m_language = Language::GetLanguageTypeFromString(option_arg);
371375
if (m_language == eLanguageTypeUnknown)
372-
error.SetErrorStringWithFormat(
373-
"Unknown language type: '%s' for breakpoint",
374-
option_arg.str().c_str());
376+
error =
377+
CreateOptionParsingError(option_arg, short_option, long_option,
378+
g_language_parsing_error_message);
375379
break;
376380

377381
case 'm': {
@@ -384,9 +388,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
384388
m_move_to_nearest_code = eLazyBoolNo;
385389

386390
if (!success)
387-
error.SetErrorStringWithFormat(
388-
"Invalid boolean value for move-to-nearest-code option: '%s'",
389-
option_arg.str().c_str());
391+
error =
392+
CreateOptionParsingError(option_arg, short_option, long_option,
393+
g_bool_parsing_error_message);
390394
break;
391395
}
392396

@@ -404,8 +408,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
404408
if (BreakpointID::StringIsBreakpointName(option_arg, error))
405409
m_breakpoint_names.push_back(std::string(option_arg));
406410
else
407-
error.SetErrorStringWithFormat("Invalid breakpoint name: %s",
408-
option_arg.str().c_str());
411+
error = CreateOptionParsingError(
412+
option_arg, short_option, long_option, "Invalid breakpoint name");
409413
break;
410414
}
411415

@@ -443,9 +447,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
443447
bool success;
444448
m_throw_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
445449
if (!success)
446-
error.SetErrorStringWithFormat(
447-
"Invalid boolean value for on-throw option: '%s'",
448-
option_arg.str().c_str());
450+
error =
451+
CreateOptionParsingError(option_arg, short_option, long_option,
452+
g_bool_parsing_error_message);
449453
} break;
450454

451455
case 'X':
@@ -457,9 +461,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
457461
OptionValueFileColonLine value;
458462
Status fcl_err = value.SetValueFromString(option_arg);
459463
if (!fcl_err.Success()) {
460-
error.SetErrorStringWithFormat(
461-
"Invalid value for file:line specifier: %s",
462-
fcl_err.AsCString());
464+
error = CreateOptionParsingError(option_arg, short_option,
465+
long_option, fcl_err.AsCString());
463466
} else {
464467
m_filenames.AppendIfUnique(value.GetFileSpec());
465468
m_line_num = value.GetLineNumber();
@@ -1557,6 +1560,7 @@ class BreakpointNameOptionGroup : public OptionGroup {
15571560
ExecutionContext *execution_context) override {
15581561
Status error;
15591562
const int short_option = g_breakpoint_name_options[option_idx].short_option;
1563+
const char *long_option = g_breakpoint_name_options[option_idx].long_option;
15601564

15611565
switch (short_option) {
15621566
case 'N':
@@ -1566,15 +1570,13 @@ class BreakpointNameOptionGroup : public OptionGroup {
15661570
break;
15671571
case 'B':
15681572
if (m_breakpoint.SetValueFromString(option_arg).Fail())
1569-
error.SetErrorStringWithFormat(
1570-
"unrecognized value \"%s\" for breakpoint",
1571-
option_arg.str().c_str());
1573+
error = CreateOptionParsingError(option_arg, short_option, long_option,
1574+
g_int_parsing_error_message);
15721575
break;
15731576
case 'D':
15741577
if (m_use_dummy.SetValueFromString(option_arg).Fail())
1575-
error.SetErrorStringWithFormat(
1576-
"unrecognized value \"%s\" for use-dummy",
1577-
option_arg.str().c_str());
1578+
error = CreateOptionParsingError(option_arg, short_option, long_option,
1579+
g_bool_parsing_error_message);
15781580
break;
15791581
case 'H':
15801582
m_help_string.SetValueFromString(option_arg);
@@ -1617,6 +1619,8 @@ class BreakpointAccessOptionGroup : public OptionGroup {
16171619
Status error;
16181620
const int short_option =
16191621
g_breakpoint_access_options[option_idx].short_option;
1622+
const char *long_option =
1623+
g_breakpoint_access_options[option_idx].long_option;
16201624

16211625
switch (short_option) {
16221626
case 'L': {
@@ -1625,29 +1629,26 @@ class BreakpointAccessOptionGroup : public OptionGroup {
16251629
if (success) {
16261630
m_permissions.SetAllowList(value);
16271631
} else
1628-
error.SetErrorStringWithFormat(
1629-
"invalid boolean value '%s' passed for -L option",
1630-
option_arg.str().c_str());
1632+
error = CreateOptionParsingError(option_arg, short_option, long_option,
1633+
g_bool_parsing_error_message);
16311634
} break;
16321635
case 'A': {
16331636
bool value, success;
16341637
value = OptionArgParser::ToBoolean(option_arg, false, &success);
16351638
if (success) {
16361639
m_permissions.SetAllowDisable(value);
16371640
} else
1638-
error.SetErrorStringWithFormat(
1639-
"invalid boolean value '%s' passed for -L option",
1640-
option_arg.str().c_str());
1641+
error = CreateOptionParsingError(option_arg, short_option, long_option,
1642+
g_bool_parsing_error_message);
16411643
} break;
16421644
case 'D': {
16431645
bool value, success;
16441646
value = OptionArgParser::ToBoolean(option_arg, false, &success);
16451647
if (success) {
16461648
m_permissions.SetAllowDelete(value);
16471649
} else
1648-
error.SetErrorStringWithFormat(
1649-
"invalid boolean value '%s' passed for -L option",
1650-
option_arg.str().c_str());
1650+
error = CreateOptionParsingError(option_arg, short_option, long_option,
1651+
g_bool_parsing_error_message);
16511652
} break;
16521653
default:
16531654
llvm_unreachable("Unimplemented option");
@@ -2141,6 +2142,8 @@ class CommandObjectBreakpointRead : public CommandObjectParsed {
21412142
ExecutionContext *execution_context) override {
21422143
Status error;
21432144
const int short_option = m_getopt_table[option_idx].val;
2145+
const char *long_option =
2146+
m_getopt_table[option_idx].definition->long_option;
21442147

21452148
switch (short_option) {
21462149
case 'f':
@@ -2150,8 +2153,8 @@ class CommandObjectBreakpointRead : public CommandObjectParsed {
21502153
Status name_error;
21512154
if (!BreakpointID::StringIsBreakpointName(llvm::StringRef(option_arg),
21522155
name_error)) {
2153-
error.SetErrorStringWithFormat("Invalid breakpoint name: %s",
2154-
name_error.AsCString());
2156+
error = CreateOptionParsingError(option_arg, short_option,
2157+
long_option, name_error.AsCString());
21552158
}
21562159
m_names.push_back(std::string(option_arg));
21572160
break;

0 commit comments

Comments
 (0)