[lldb] Part 2 of 2 - Refactor `CommandObject::DoExecute(...)` return `void` (not `bool`) (#69991)

[lldb] Part 2 of 2 - Refactor `CommandObject::DoExecute(...)` to return
`void` instead of ~~`bool`~~

Justifications:
- The code doesn't ultimately apply the `true`/`false` return values.
- The methods already pass around a `CommandReturnObject`, typically
with a `result` parameter.
- Each command return object already contains:
	- A more precise status
	- The error code(s) that apply to that status

Part 1 refactors the `CommandObject::Execute(...)` method.
- See
[https://github.com/llvm/llvm-project/pull/69989](https://github.com/llvm/llvm-project/pull/69989)

rdar://117378957

1 files changed, 65 insertions, 86 deletions

diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp
index 656ace2..74d97b0 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -129,12 +129,12 @@ protected:
     OptionValueBoolean m_cmd_relative_to_command_file;
   };
 
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     if (command.GetArgumentCount() != 1) {
       result.AppendErrorWithFormat(
           "'%s' takes exactly one executable filename argument.\n",
           GetCommandName().str().c_str());
-      return false;
+      return;
     }
 
     FileSpec source_dir = {};
@@ -144,7 +144,7 @@ protected:
         result.AppendError("command source -C can only be specified "
                            "from a command file");
         result.SetStatus(eReturnStatusFailed);
-        return false;
+        return;
       }
     }
 
@@ -155,7 +155,7 @@ protected:
         result.AppendError("command source -C can only be used "
                            "with a relative path.");
         result.SetStatus(eReturnStatusFailed);
-        return false;
+        return;
       }
       cmd_file.MakeAbsolute(source_dir);
     }
@@ -186,7 +186,6 @@ protected:
     }
 
     m_interpreter.HandleCommandsFromFile(cmd_file, options, result);
-    return result.Succeeded();
   }
 
   CommandOptions m_options;
@@ -384,11 +383,11 @@ rather than using a positional placeholder:"
   ~CommandObjectCommandsAlias() override = default;
 
 protected:
-  bool DoExecute(llvm::StringRef raw_command_line,
+  void DoExecute(llvm::StringRef raw_command_line,
                  CommandReturnObject &result) override {
     if (raw_command_line.empty()) {
       result.AppendError("'command alias' requires at least two arguments");
-      return false;
+      return;
     }
 
     ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext();
@@ -399,14 +398,14 @@ protected:
     if (args_with_suffix.HasArgs())
       if (!ParseOptionsAndNotify(args_with_suffix.GetArgs(), result,
                                  m_option_group, exe_ctx))
-        return false;
+        return;
 
     llvm::StringRef raw_command_string = args_with_suffix.GetRawPart();
     Args args(raw_command_string);
 
     if (args.GetArgumentCount() < 2) {
       result.AppendError("'command alias' requires at least two arguments");
-      return false;
+      return;
     }
 
     // Get the alias command.
@@ -418,7 +417,7 @@ protected:
         result.AppendWarning("if trying to pass options to 'command alias' add "
                              "a -- at the end of the options");
       }
-      return false;
+      return;
     }
 
     // Strip the new alias name off 'raw_command_string'  (leave it on args,
@@ -431,7 +430,7 @@ protected:
         raw_command_string = raw_command_string.substr(pos);
     } else {
       result.AppendError("Error parsing command string.  No alias created.");
-      return false;
+      return;
     }
 
     // Verify that the command is alias-able.
@@ -439,7 +438,7 @@ protected:
       result.AppendErrorWithFormat(
           "'%s' is a permanent debugger command and cannot be redefined.\n",
           args[0].c_str());
-      return false;
+      return;
     }
 
     if (m_interpreter.UserMultiwordCommandExists(alias_command)) {
@@ -447,7 +446,7 @@ protected:
           "'%s' is a user container command and cannot be overwritten.\n"
           "Delete it first with 'command container delete'\n",
           args[0].c_str());
-      return false;
+      return;
     }
 
     // Get CommandObject that is being aliased. The command name is read from
@@ -462,17 +461,15 @@ protected:
                                    "'%s' does not begin with a valid command."
                                    "  No alias created.",
                                    original_raw_command_string.str().c_str());
-      return false;
     } else if (!cmd_obj->WantsRawCommandString()) {
       // Note that args was initialized with the original command, and has not
       // been updated to this point. Therefore can we pass it to the version of
       // Execute that does not need/expect raw input in the alias.
-      return HandleAliasingNormalCommand(args, result);
+      HandleAliasingNormalCommand(args, result);
     } else {
-      return HandleAliasingRawCommand(alias_command, raw_command_string,
-                                      *cmd_obj, result);
+      HandleAliasingRawCommand(alias_command, raw_command_string, *cmd_obj,
+                               result);
     }
-    return result.Succeeded();
   }
 
   bool HandleAliasingRawCommand(llvm::StringRef alias_command,
@@ -653,13 +650,13 @@ public:
   }
 
 protected:
-  bool DoExecute(Args &args, CommandReturnObject &result) override {
+  void DoExecute(Args &args, CommandReturnObject &result) override {
     CommandObject::CommandMap::iterator pos;
     CommandObject *cmd_obj;
 
     if (args.empty()) {
       result.AppendError("must call 'unalias' with a valid alias");
-      return false;
+      return;
     }
 
     auto command_name = args[0].ref();
@@ -669,7 +666,7 @@ protected:
           "'%s' is not a known command.\nTry 'help' to see a "
           "current list of commands.\n",
           args[0].c_str());
-      return false;
+      return;
     }
 
     if (m_interpreter.CommandExists(command_name)) {
@@ -683,7 +680,7 @@ protected:
             "'%s' is a permanent debugger command and cannot be removed.\n",
             args[0].c_str());
       }
-      return false;
+      return;
     }
 
     if (!m_interpreter.RemoveAlias(command_name)) {
@@ -694,11 +691,10 @@ protected:
       else
         result.AppendErrorWithFormat("'%s' is not an existing alias.\n",
                                      args[0].c_str());
-      return false;
+      return;
     }
 
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    return result.Succeeded();
   }
 };
 
@@ -742,14 +738,14 @@ public:
   }
 
 protected:
-  bool DoExecute(Args &args, CommandReturnObject &result) override {
+  void DoExecute(Args &args, CommandReturnObject &result) override {
     CommandObject::CommandMap::iterator pos;
 
     if (args.empty()) {
       result.AppendErrorWithFormat("must call '%s' with one or more valid user "
                                    "defined regular expression command names",
                                    GetCommandName().str().c_str());
-      return false;
+      return;
     }
 
     auto command_name = args[0].ref();
@@ -761,18 +757,17 @@ protected:
           &error_msg_stream, command_name, llvm::StringRef(), llvm::StringRef(),
           generate_upropos, generate_type_lookup);
       result.AppendError(error_msg_stream.GetString());
-      return false;
+      return;
     }
 
     if (!m_interpreter.RemoveCommand(command_name)) {
       result.AppendErrorWithFormat(
           "'%s' is a permanent debugger command and cannot be removed.\n",
           args[0].c_str());
-      return false;
+      return;
     }
 
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    return true;
   }
 };
 
@@ -868,12 +863,12 @@ protected:
     }
   }
 
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
     if (argc == 0) {
       result.AppendError("usage: 'command regex <command-name> "
                          "[s/<regex1>/<subst1>/ s/<regex2>/<subst2>/ ...]'\n");
-      return false;
+      return;
     }
 
     Status error;
@@ -914,8 +909,6 @@ protected:
     if (error.Fail()) {
       result.AppendError(error.AsCString());
     }
-
-    return result.Succeeded();
   }
 
   Status AppendRegexSubstitution(const llvm::StringRef &regex_sed,
@@ -1126,7 +1119,7 @@ public:
   bool WantsCompletion() override { return true; }
 
 protected:
-  bool DoExecute(llvm::StringRef raw_command_line,
+  void DoExecute(llvm::StringRef raw_command_line,
                  CommandReturnObject &result) override {
     ScriptInterpreter *scripter = GetDebugger().GetScriptInterpreter();
 
@@ -1147,8 +1140,6 @@ protected:
           result.SetStatus(eReturnStatusSuccessFinishResult);
       }
     }
-
-    return result.Succeeded();
   }
 
 private:
@@ -1222,7 +1213,7 @@ public:
   }
 
 protected:
-  bool DoExecute(llvm::StringRef raw_command_line,
+  void DoExecute(llvm::StringRef raw_command_line,
                  CommandReturnObject &result) override {
     ScriptInterpreter *scripter = GetDebugger().GetScriptInterpreter();
 
@@ -1243,8 +1234,6 @@ protected:
           result.SetStatus(eReturnStatusSuccessFinishResult);
       }
     }
-
-    return result.Succeeded();
   }
 
 private:
@@ -1330,10 +1319,10 @@ protected:
     bool silent = false;
   };
 
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     if (command.empty()) {
       result.AppendError("command script import needs one or more arguments");
-      return false;
+      return;
     }
 
     FileSpec source_dir = {};
@@ -1342,7 +1331,7 @@ protected:
       if (!source_dir) {
         result.AppendError("command script import -c can only be specified "
                            "from a command file");
-        return false;
+        return;
       }
     }
 
@@ -1371,8 +1360,6 @@ protected:
                                      error.AsCString());
       }
     }
-
-    return result.Succeeded();
   }
 
   CommandOptions m_options;
@@ -1567,16 +1554,16 @@ protected:
     io_handler.SetIsDone(true);
   }
 
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     if (GetDebugger().GetScriptLanguage() != lldb::eScriptLanguagePython) {
       result.AppendError("only scripting language supported for scripted "
                          "commands is currently Python");
-      return false;
+      return;
     }
 
     if (command.GetArgumentCount() == 0) {
       result.AppendError("'command script add' requires at least one argument");
-      return false;
+      return;
     }
     // Store the options in case we get multi-line input, also figure out the
     // default if not user supplied:
@@ -1598,7 +1585,7 @@ protected:
     if (path_error.Fail()) {
       result.AppendErrorWithFormat("error in command path: %s",
                                    path_error.AsCString());
-      return false;
+      return;
     }
 
     if (!m_container) {
@@ -1617,7 +1604,7 @@ protected:
     if (m_options.m_class_name.empty() && m_options.m_funct_name.empty()) {
       m_interpreter.GetPythonCommandsFromIOHandler("     ", // Prompt
                                                    *this);  // IOHandlerDelegate
-      return result.Succeeded();
+      return;
     }
 
     CommandObjectSP new_cmd_sp;
@@ -1629,7 +1616,7 @@ protected:
       ScriptInterpreter *interpreter = GetDebugger().GetScriptInterpreter();
       if (!interpreter) {
         result.AppendError("cannot find ScriptInterpreter");
-        return false;
+        return;
       }
 
       auto cmd_obj_sp = interpreter->CreateScriptCommandObject(
@@ -1637,7 +1624,7 @@ protected:
       if (!cmd_obj_sp) {
         result.AppendErrorWithFormatv("cannot create helper object for: "
                                       "'{0}'", m_options.m_class_name);
-        return false;
+        return;
       }
 
       new_cmd_sp.reset(new CommandObjectScriptingObject(
@@ -1660,7 +1647,6 @@ protected:
         result.AppendErrorWithFormat("cannot add command: %s", 
                                      llvm::toString(std::move(llvm_error)).c_str());
     }
-    return result.Succeeded();
   }
 
   CommandOptions m_options;
@@ -1684,12 +1670,10 @@ public:
 
   ~CommandObjectCommandsScriptList() override = default;
 
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     m_interpreter.GetHelp(result, CommandInterpreter::eCommandTypesUserDef);
 
     result.SetStatus(eReturnStatusSuccessFinishResult);
-
-    return true;
   }
 };
 
@@ -1704,12 +1688,10 @@ public:
   ~CommandObjectCommandsScriptClear() override = default;
 
 protected:
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     m_interpreter.RemoveAllUser();
 
     result.SetStatus(eReturnStatusSuccessFinishResult);
-
-    return true;
   }
 };
 
@@ -1748,44 +1730,44 @@ public:
   }
 
 protected:
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
 
     llvm::StringRef root_cmd = command[0].ref();
     size_t num_args = command.GetArgumentCount();
 
     if (root_cmd.empty()) {
       result.AppendErrorWithFormat("empty root command name");
-      return false;
+      return;
     }
     if (!m_interpreter.HasUserCommands() &&
         !m_interpreter.HasUserMultiwordCommands()) {
       result.AppendErrorWithFormat("can only delete user defined commands, "
                                    "but no user defined commands found");
-      return false;
+      return;
     }
 
     CommandObjectSP cmd_sp = m_interpreter.GetCommandSPExact(root_cmd);
     if (!cmd_sp) {
       result.AppendErrorWithFormat("command '%s' not found.",
                                    command[0].c_str());
-      return false;
+      return;
     }
     if (!cmd_sp->IsUserCommand()) {
       result.AppendErrorWithFormat("command '%s' is not a user command.",
                                    command[0].c_str());
-      return false;
+      return;
     }
     if (cmd_sp->GetAsMultiwordCommand() && num_args == 1) {
       result.AppendErrorWithFormat("command '%s' is a multi-word command.\n "
                                    "Delete with \"command container delete\"",
                                    command[0].c_str());
-      return false;
+      return;
     }
 
     if (command.GetArgumentCount() == 1) {
       m_interpreter.RemoveUser(root_cmd);
       result.SetStatus(eReturnStatusSuccessFinishResult);
-      return true;
+      return;
     }
     // We're deleting a command from a multiword command.  Verify the command
     // path:
@@ -1796,14 +1778,14 @@ protected:
     if (error.Fail()) {
       result.AppendErrorWithFormat("could not resolve command path: %s",
                                    error.AsCString());
-      return false;
+      return;
     }
     if (!container) {
       // This means that command only had a leaf command, so the container is
       // the root.  That should have been handled above.
       result.AppendErrorWithFormat("could not find a container for '%s'",
                                    command[0].c_str());
-      return false;
+      return;
     }
     const char *leaf_cmd = command[num_args - 1].c_str();
     llvm::Error llvm_error = container->RemoveUserSubcommand(leaf_cmd,
@@ -1812,7 +1794,7 @@ protected:
       result.AppendErrorWithFormat("could not delete command '%s': %s",
                                    leaf_cmd, 
                                    llvm::toString(std::move(llvm_error)).c_str());
-      return false;
+      return;
     }
 
     Stream &out_stream = result.GetOutputStream();
@@ -1824,7 +1806,6 @@ protected:
     }
     out_stream << '\n';
     result.SetStatus(eReturnStatusSuccessFinishResult);
-    return true;
   }
 };
 
@@ -1945,12 +1926,12 @@ protected:
     std::string m_long_help;
     bool m_overwrite = false;
   };
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     size_t num_args = command.GetArgumentCount();
 
     if (num_args == 0) {
       result.AppendError("no command was specified");
-      return false;
+      return;
     }
 
     if (num_args == 1) {
@@ -1965,10 +1946,10 @@ protected:
       if (add_error.Fail()) {
         result.AppendErrorWithFormat("error adding command: %s",
                                      add_error.AsCString());
-        return false;
+        return;
       }
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
-      return true;
+      return;
     }
 
     // We're adding this to a subcommand, first find the subcommand:
@@ -1980,7 +1961,7 @@ protected:
     if (!add_to_me) {
       result.AppendErrorWithFormat("error adding command: %s",
                                    path_error.AsCString());
-      return false;
+      return;
     }
 
     const char *cmd_name = command.GetArgumentAtIndex(num_args - 1);
@@ -1992,11 +1973,10 @@ protected:
     if (llvm_error) {
       result.AppendErrorWithFormat("error adding subcommand: %s",
                                    llvm::toString(std::move(llvm_error)).c_str());
-      return false;
+      return;
     }
 
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    return true;
   }
 
 private:
@@ -2039,12 +2019,12 @@ public:
   }
 
 protected:
-  bool DoExecute(Args &command, CommandReturnObject &result) override {
+  void DoExecute(Args &command, CommandReturnObject &result) override {
     size_t num_args = command.GetArgumentCount();
 
     if (num_args == 0) {
       result.AppendError("No command was specified.");
-      return false;
+      return;
     }
 
     if (num_args == 1) {
@@ -2057,27 +2037,27 @@ protected:
       if (!cmd_sp) {
         result.AppendErrorWithFormat("container command %s doesn't exist.",
                                      cmd_name);
-        return false;
+        return;
       }
       if (!cmd_sp->IsUserCommand()) {
         result.AppendErrorWithFormat(
             "container command %s is not a user command", cmd_name);
-        return false;
+        return;
       }
       if (!cmd_sp->GetAsMultiwordCommand()) {
         result.AppendErrorWithFormat("command %s is not a container command",
                                      cmd_name);
-        return false;
+        return;
       }
 
       bool did_remove = GetCommandInterpreter().RemoveUserMultiword(cmd_name);
       if (!did_remove) {
         result.AppendErrorWithFormat("error removing command %s.", cmd_name);
-        return false;
+        return;
       }
 
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
-      return true;
+      return;
     }
 
     // We're removing a subcommand, first find the subcommand's owner:
@@ -2089,7 +2069,7 @@ protected:
     if (!container) {
       result.AppendErrorWithFormat("error removing container command: %s",
                                    path_error.AsCString());
-      return false;
+      return;
     }
     const char *leaf = command.GetArgumentAtIndex(num_args - 1);
     llvm::Error llvm_error =
@@ -2097,10 +2077,9 @@ protected:
     if (llvm_error) {
       result.AppendErrorWithFormat("error removing container command: %s",
                                    llvm::toString(std::move(llvm_error)).c_str());
-      return false;
+      return;
     }
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    return true;
   }
 };