[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)
Alex Langford via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 26 16:08:45 PST 2024
https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/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.
>From feb4588cd12a5f513061dffa99f6370067f91463 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Mon, 26 Feb 2024 16:01:24 -0800
Subject: [PATCH] [lldb] Use CreateOptionParsingError in
CommandObjectBreakpoint
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.
---
lldb/include/lldb/Interpreter/Options.h | 2 +
.../Commands/CommandObjectBreakpoint.cpp | 101 +++++++++---------
2 files changed, 54 insertions(+), 49 deletions(-)
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;
More information about the lldb-commits
mailing list