llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/83086.diff 2 Files Affected: - (modified) lldb/include/lldb/Interpreter/Options.h (+2) - (modified) lldb/source/Commands/CommandObjectBreakpoint.cpp (+52-49) ``````````diff 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; `````````` </details> https://github.com/llvm/llvm-project/pull/83086 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
