[lldb] Standardize command option parsing error messages (#82273)

I have been looking to simplify parsing logic and improve the interfaces
so that they are both easier to use and harder to abuse. To be specific,
I am referring to functions such as `OptionArgParser::ToBoolean`: I
would like to go from its current interface to something more like
`llvm::Error<bool> ToBoolean(llvm::StringRef option_arg)`.

Through working on that, I encountered 2 inconveniences:
1. Option parsing code is not uniform. Every function writes a slightly
different error message, so incorporating an error message from the
`ToBoolean` implementation is going to be laborious as I figure out what
exactly needs to change or stay the same.
2. Changing the interface of `ToBoolean` would require a global atomic
change across all of the Command code. This would be quite frustrating
to do because of the non-uniformity of our existing code.

To address these frustrations, I think it would be easiest to first
standardize the error reporting mechanism when parsing options in
commands. I do so by introducing `CreateOptionParsingError` which will
create an error message of the shape:
Invalid value ('${option_arg}') for -${short_value} ('${long_value}'):
${additional_context}

Concretely, it would look something like this:
(lldb) breakpoint set -n main -G yay
error: Invalid value ('yay') for -G (auto-continue): Failed to parse as
boolean

After this, updating the interfaces for parsing the values themselves
should become simpler. Because this can be adopted incrementally, this
should be able to done over the course of time instead of all at once as
a giant difficult-to-review change. I've changed exactly one function
where this function would be used as an illustration of what I am
proposing.

1 files changed, 20 insertions, 17 deletions

diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 3fdf5cd..fc22176 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -64,6 +64,8 @@ public:
     Status error;
     const int short_option =
         g_breakpoint_modify_options[option_idx].short_option;
+    const char *long_option =
+        g_breakpoint_modify_options[option_idx].long_option;
 
     switch (short_option) {
     case 'c':
@@ -84,18 +86,17 @@ public:
     case 'G': {
       bool value, success;
       value = OptionArgParser::ToBoolean(option_arg, false, &success);
-      if (success) {
+      if (success)
         m_bp_opts.SetAutoContinue(value);
-      } else
-        error.SetErrorStringWithFormat(
-            "invalid boolean value '%s' passed for -G option",
-            option_arg.str().c_str());
+      else
+        error = CreateOptionParsingError(option_arg, short_option, long_option,
+                                         g_bool_parsing_error_message);
     } break;
     case 'i': {
       uint32_t ignore_count;
       if (option_arg.getAsInteger(0, ignore_count))
-        error.SetErrorStringWithFormat("invalid ignore count '%s'",
-                                       option_arg.str().c_str());
+        error = CreateOptionParsingError(option_arg, short_option, long_option,
+                                         g_int_parsing_error_message);
       else
         m_bp_opts.SetIgnoreCount(ignore_count);
     } break;
@@ -105,27 +106,29 @@ public:
       if (success) {
         m_bp_opts.SetOneShot(value);
       } else
-        error.SetErrorStringWithFormat(
-            "invalid boolean value '%s' passed for -o option",
-            option_arg.str().c_str());
+        error = CreateOptionParsingError(option_arg, short_option, long_option,
+                                         g_bool_parsing_error_message);
     } break;
     case 't': {
       lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID;
       if (option_arg == "current") {
         if (!execution_context) {
-          error.SetErrorStringWithFormat("No context to determine current "
-                                         "thread");
+          error = CreateOptionParsingError(
+              option_arg, short_option, long_option,
+              "No context to determine current thread");
         } else {
           ThreadSP ctx_thread_sp = execution_context->GetThreadSP();
           if (!ctx_thread_sp || !ctx_thread_sp->IsValid()) {
-            error.SetErrorStringWithFormat("No currently selected thread");
+            error =
+                CreateOptionParsingError(option_arg, short_option, long_option,
+                                         "No currently selected thread");
           } else {
             thread_id = ctx_thread_sp->GetID();
           }
         }
       } else if (option_arg.getAsInteger(0, thread_id)) {
-        error.SetErrorStringWithFormat("invalid thread id string '%s'",
-                                       option_arg.str().c_str());
+        error = CreateOptionParsingError(option_arg, short_option, long_option,
+                                         g_int_parsing_error_message);
       }
       if (thread_id != LLDB_INVALID_THREAD_ID)
         m_bp_opts.SetThreadID(thread_id);
@@ -139,8 +142,8 @@ public:
     case 'x': {
       uint32_t thread_index = UINT32_MAX;
       if (option_arg.getAsInteger(0, thread_index)) {
-        error.SetErrorStringWithFormat("invalid thread index string '%s'",
-                                       option_arg.str().c_str());
+        error = CreateOptionParsingError(option_arg, short_option, long_option,
+                                         g_int_parsing_error_message);
       } else {
         m_bp_opts.GetThreadSpec()->SetIndex(thread_index);
       }