[lldb] Use CreateOptionParsingError in CommandObjectBreakpoint #83086
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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;
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.