-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb] Use CreateOptionParsingError in CommandObjectBreakpoint #83086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb] Use CreateOptionParsingError in CommandObjectBreakpoint #83086
Conversation
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.
@llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) ChangesThis 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. Full diff: https://github.com/llvm/llvm-project/pull/83086.diff 2 Files Affected:
diff --git a/lldb/include/lldb/Interpreter/Options.h b/lldb/include/lldb/Interpreter/Options.h
index 18a87e49deee5c..9a6a17c2793fa4 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -368,6 +368,8 @@ static constexpr llvm::StringLiteral g_bool_parsing_error_message =
"Failed to parse as boolean";
static constexpr llvm::StringLiteral g_int_parsing_error_message =
"Failed to parse as integer";
+static constexpr llvm::StringLiteral g_language_parsing_error_message =
+ "Unknown language";
} // namespace lldb_private
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index fc2217608a0bb9..cfb0b87a59e640 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -266,6 +266,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
Status error;
const int short_option =
g_breakpoint_set_options[option_idx].short_option;
+ const char *long_option =
+ g_breakpoint_set_options[option_idx].long_option;
switch (short_option) {
case 'a': {
@@ -284,13 +286,15 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
case 'u':
if (option_arg.getAsInteger(0, m_column))
- error.SetErrorStringWithFormat("invalid column number: %s",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_int_parsing_error_message);
break;
case 'E': {
LanguageType language = Language::GetLanguageTypeFromString(option_arg);
+ llvm::StringRef error_context;
switch (language) {
case eLanguageTypeC89:
case eLanguageTypeC:
@@ -308,19 +312,18 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
m_exception_language = eLanguageTypeObjC;
break;
case eLanguageTypeObjC_plus_plus:
- error.SetErrorStringWithFormat(
- "Set exception breakpoints separately for c++ and objective-c");
+ error_context =
+ "Set exception breakpoints separately for c++ and objective-c";
break;
case eLanguageTypeUnknown:
- error.SetErrorStringWithFormat(
- "Unknown language type: '%s' for exception breakpoint",
- option_arg.str().c_str());
+ error_context = "Unknown language type for exception breakpoint";
break;
default:
- error.SetErrorStringWithFormat(
- "Unsupported language type: '%s' for exception breakpoint",
- option_arg.str().c_str());
+ error_context = "Unsupported language type for exception breakpoint";
}
+ if (!error_context.empty())
+ error = CreateOptionParsingError(option_arg, short_option,
+ long_option, error_context);
} break;
case 'f':
@@ -336,9 +339,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
bool success;
m_catch_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
if (!success)
- error.SetErrorStringWithFormat(
- "Invalid boolean value for on-catch option: '%s'",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'H':
@@ -355,23 +358,24 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
m_skip_prologue = eLazyBoolNo;
if (!success)
- error.SetErrorStringWithFormat(
- "Invalid boolean value for skip prologue option: '%s'",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'l':
if (option_arg.getAsInteger(0, m_line_num))
- error.SetErrorStringWithFormat("invalid line number: %s.",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_int_parsing_error_message);
break;
case 'L':
m_language = Language::GetLanguageTypeFromString(option_arg);
if (m_language == eLanguageTypeUnknown)
- error.SetErrorStringWithFormat(
- "Unknown language type: '%s' for breakpoint",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_language_parsing_error_message);
break;
case 'm': {
@@ -384,9 +388,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
m_move_to_nearest_code = eLazyBoolNo;
if (!success)
- error.SetErrorStringWithFormat(
- "Invalid boolean value for move-to-nearest-code option: '%s'",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
break;
}
@@ -404,8 +408,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
if (BreakpointID::StringIsBreakpointName(option_arg, error))
m_breakpoint_names.push_back(std::string(option_arg));
else
- error.SetErrorStringWithFormat("Invalid breakpoint name: %s",
- option_arg.str().c_str());
+ error = CreateOptionParsingError(
+ option_arg, short_option, long_option, "Invalid breakpoint name");
break;
}
@@ -443,9 +447,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
bool success;
m_throw_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
if (!success)
- error.SetErrorStringWithFormat(
- "Invalid boolean value for on-throw option: '%s'",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'X':
@@ -457,9 +461,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
OptionValueFileColonLine value;
Status fcl_err = value.SetValueFromString(option_arg);
if (!fcl_err.Success()) {
- error.SetErrorStringWithFormat(
- "Invalid value for file:line specifier: %s",
- fcl_err.AsCString());
+ error = CreateOptionParsingError(option_arg, short_option,
+ long_option, fcl_err.AsCString());
} else {
m_filenames.AppendIfUnique(value.GetFileSpec());
m_line_num = value.GetLineNumber();
@@ -1557,6 +1560,7 @@ class BreakpointNameOptionGroup : public OptionGroup {
ExecutionContext *execution_context) override {
Status error;
const int short_option = g_breakpoint_name_options[option_idx].short_option;
+ const char *long_option = g_breakpoint_name_options[option_idx].long_option;
switch (short_option) {
case 'N':
@@ -1566,15 +1570,13 @@ class BreakpointNameOptionGroup : public OptionGroup {
break;
case 'B':
if (m_breakpoint.SetValueFromString(option_arg).Fail())
- error.SetErrorStringWithFormat(
- "unrecognized value \"%s\" for breakpoint",
- option_arg.str().c_str());
+ error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_int_parsing_error_message);
break;
case 'D':
if (m_use_dummy.SetValueFromString(option_arg).Fail())
- error.SetErrorStringWithFormat(
- "unrecognized value \"%s\" for use-dummy",
- option_arg.str().c_str());
+ error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
break;
case 'H':
m_help_string.SetValueFromString(option_arg);
@@ -1617,6 +1619,8 @@ class BreakpointAccessOptionGroup : public OptionGroup {
Status error;
const int short_option =
g_breakpoint_access_options[option_idx].short_option;
+ const char *long_option =
+ g_breakpoint_access_options[option_idx].long_option;
switch (short_option) {
case 'L': {
@@ -1625,9 +1629,8 @@ class BreakpointAccessOptionGroup : public OptionGroup {
if (success) {
m_permissions.SetAllowList(value);
} else
- error.SetErrorStringWithFormat(
- "invalid boolean value '%s' passed for -L option",
- option_arg.str().c_str());
+ error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'A': {
bool value, success;
@@ -1635,9 +1638,8 @@ class BreakpointAccessOptionGroup : public OptionGroup {
if (success) {
m_permissions.SetAllowDisable(value);
} else
- error.SetErrorStringWithFormat(
- "invalid boolean value '%s' passed for -L option",
- option_arg.str().c_str());
+ error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'D': {
bool value, success;
@@ -1645,9 +1647,8 @@ class BreakpointAccessOptionGroup : public OptionGroup {
if (success) {
m_permissions.SetAllowDelete(value);
} else
- error.SetErrorStringWithFormat(
- "invalid boolean value '%s' passed for -L option",
- option_arg.str().c_str());
+ error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
default:
llvm_unreachable("Unimplemented option");
@@ -2141,6 +2142,8 @@ class CommandObjectBreakpointRead : public CommandObjectParsed {
ExecutionContext *execution_context) override {
Status error;
const int short_option = m_getopt_table[option_idx].val;
+ const char *long_option =
+ m_getopt_table[option_idx].definition->long_option;
switch (short_option) {
case 'f':
@@ -2150,8 +2153,8 @@ class CommandObjectBreakpointRead : public CommandObjectParsed {
Status name_error;
if (!BreakpointID::StringIsBreakpointName(llvm::StringRef(option_arg),
name_error)) {
- error.SetErrorStringWithFormat("Invalid breakpoint name: %s",
- name_error.AsCString());
+ error = CreateOptionParsingError(option_arg, short_option,
+ long_option, name_error.AsCString());
}
m_names.push_back(std::string(option_arg));
break;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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.