[Lldb-commits] [lldb] [lldb] Part 1 of 2 - Refactor `CommandObject::Execute(...)` to return `void` instead of ~~`bool`~~ (PR #69989)

Pete Lawrence via lldb-commits lldb-commits at lists.llvm.org
Mon Oct 23 17:38:05 PDT 2023


https://github.com/PortalPete created https://github.com/llvm/llvm-project/pull/69989

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 2 refactors the `CommandObject::DoExecute(...)` method.

rdar://117378957

>From b1b96c7c2da46bb83448829ac807f5e00dd6fa8a Mon Sep 17 00:00:00 2001
From: Pete Lawrence <plawrence at apple.com>
Date: Thu, 19 Oct 2023 18:59:57 -1000
Subject: [PATCH] [lldb] Part 1 of 2 - Refactor `CommandObject::Execute(...)`
 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 2 refactors the `CommandObject::DoExecute(...)` method.

rdar://117378957
---
 lldb/include/lldb/Interpreter/CommandAlias.h  |  2 +-
 lldb/include/lldb/Interpreter/CommandObject.h |  6 +++---
 .../lldb/Interpreter/CommandObjectMultiword.h |  4 ++--
 .../Commands/CommandObjectMultiword.cpp       | 20 +++++++++----------
 lldb/source/Interpreter/CommandAlias.cpp      |  2 +-
 lldb/source/Interpreter/CommandObject.cpp     | 15 +++++++-------
 6 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/CommandAlias.h b/lldb/include/lldb/Interpreter/CommandAlias.h
index 26826db62705d67..7b59ea0a74bb9e5 100644
--- a/lldb/include/lldb/Interpreter/CommandAlias.h
+++ b/lldb/include/lldb/Interpreter/CommandAlias.h
@@ -56,7 +56,7 @@ class CommandAlias : public CommandObject {
 
   void SetHelpLong(llvm::StringRef str) override;
 
-  bool Execute(const char *args_string, CommandReturnObject &result) override;
+  void Execute(const char *args_string, CommandReturnObject &result) override;
 
   lldb::CommandObjectSP GetUnderlyingCommand() {
     return m_underlying_command_sp;
diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h
index d8358435a483bab..004f5d42f1e44ee 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -312,7 +312,7 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
       return false;
   }
 
-  virtual bool Execute(const char *args_string,
+  virtual void Execute(const char *args_string,
                        CommandReturnObject &result) = 0;
 
 protected:
@@ -398,7 +398,7 @@ class CommandObjectParsed : public CommandObject {
 
   ~CommandObjectParsed() override = default;
 
-  bool Execute(const char *args_string, CommandReturnObject &result) override;
+  void Execute(const char *args_string, CommandReturnObject &result) override;
 
 protected:
   virtual bool DoExecute(Args &command, CommandReturnObject &result) = 0;
@@ -415,7 +415,7 @@ class CommandObjectRaw : public CommandObject {
 
   ~CommandObjectRaw() override = default;
 
-  bool Execute(const char *args_string, CommandReturnObject &result) override;
+  void Execute(const char *args_string, CommandReturnObject &result) override;
 
 protected:
   virtual bool DoExecute(llvm::StringRef command,
diff --git a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h
index 1c14b492c8097fe..bceb7f0e51edb6c 100644
--- a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h
+++ b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h
@@ -59,7 +59,7 @@ class CommandObjectMultiword : public CommandObject {
   std::optional<std::string> GetRepeatCommand(Args &current_command_args,
                                               uint32_t index) override;
 
-  bool Execute(const char *args_string, CommandReturnObject &result) override;
+  void Execute(const char *args_string, CommandReturnObject &result) override;
 
   bool IsRemovable() const override { return m_can_be_removed; }
 
@@ -129,7 +129,7 @@ class CommandObjectProxy : public CommandObject {
   ///     Execute is called) and \a GetProxyCommandObject returned null.
   virtual llvm::StringRef GetUnsupportedError();
 
-  bool Execute(const char *args_string, CommandReturnObject &result) override;
+  void Execute(const char *args_string, CommandReturnObject &result) override;
 
 protected:
   // These two want to iterate over the subcommand dictionary.
diff --git a/lldb/source/Commands/CommandObjectMultiword.cpp b/lldb/source/Commands/CommandObjectMultiword.cpp
index 7ef829afaab6e7d..d54be9537531951 100644
--- a/lldb/source/Commands/CommandObjectMultiword.cpp
+++ b/lldb/source/Commands/CommandObjectMultiword.cpp
@@ -159,25 +159,25 @@ llvm::Error CommandObjectMultiword::RemoveUserSubcommand(llvm::StringRef cmd_nam
   return llvm::Error::success();
 }
 
-bool CommandObjectMultiword::Execute(const char *args_string,
+void CommandObjectMultiword::Execute(const char *args_string,
                                      CommandReturnObject &result) {
   Args args(args_string);
   const size_t argc = args.GetArgumentCount();
   if (argc == 0) {
     this->CommandObject::GenerateHelpText(result);
-    return result.Succeeded();
+    return;
   }
 
   auto sub_command = args[0].ref();
   if (sub_command.empty()) {
     result.AppendError("Need to specify a non-empty subcommand.");
-    return result.Succeeded();
+    return;
   }
 
   if (m_subcommand_dict.empty()) {
     result.AppendErrorWithFormat("'%s' does not have any subcommands.\n",
                                  GetCommandName().str().c_str());
-    return false;
+    return;
   }
 
   StringList matches;
@@ -189,7 +189,7 @@ bool CommandObjectMultiword::Execute(const char *args_string,
 
     args.Shift();
     sub_cmd_obj->Execute(args_string, result);
-    return result.Succeeded();
+    return;
   }
 
   std::string error_msg;
@@ -214,7 +214,6 @@ bool CommandObjectMultiword::Execute(const char *args_string,
   }
   error_msg.append("\n");
   result.AppendRawError(error_msg.c_str());
-  return false;
 }
 
 void CommandObjectMultiword::GenerateHelpText(Stream &output_stream) {
@@ -429,11 +428,12 @@ llvm::StringRef CommandObjectProxy::GetUnsupportedError() {
   return "command is not implemented";
 }
 
-bool CommandObjectProxy::Execute(const char *args_string,
+void CommandObjectProxy::Execute(const char *args_string,
                                  CommandReturnObject &result) {
   CommandObject *proxy_command = GetProxyCommandObject();
-  if (proxy_command)
-    return proxy_command->Execute(args_string, result);
+  if (proxy_command) {
+    proxy_command->Execute(args_string, result);
+  }
+
   result.AppendError(GetUnsupportedError());
-  return false;
 }
diff --git a/lldb/source/Interpreter/CommandAlias.cpp b/lldb/source/Interpreter/CommandAlias.cpp
index f6eaeacff81efb6..b95d3c91fcbc2eb 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -135,7 +135,7 @@ Options *CommandAlias::GetOptions() {
   return nullptr;
 }
 
-bool CommandAlias::Execute(const char *args_string,
+void CommandAlias::Execute(const char *args_string,
                            CommandReturnObject &result) {
   llvm_unreachable("CommandAlias::Execute is not to be called");
 }
diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp
index 313d24f0657bea8..f83b977e3b6802c 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/lldb/source/Interpreter/CommandObject.cpp
@@ -715,7 +715,7 @@ Thread *CommandObject::GetDefaultThread() {
     return nullptr;
 }
 
-bool CommandObjectParsed::Execute(const char *args_string,
+void CommandObjectParsed::Execute(const char *args_string,
                                   CommandReturnObject &result) {
   bool handled = false;
   Args cmd_args(args_string);
@@ -746,18 +746,17 @@ bool CommandObjectParsed::Execute(const char *args_string,
           result.AppendErrorWithFormatv("'{0}' doesn't take any arguments.",
                                         GetCommandName());
           Cleanup();
-          return false;
+          return;
         }
-        handled = DoExecute(cmd_args, result);
+        DoExecute(cmd_args, result);
       }
     }
 
     Cleanup();
   }
-  return handled;
 }
 
-bool CommandObjectRaw::Execute(const char *args_string,
+void CommandObjectRaw::Execute(const char *args_string,
                                CommandReturnObject &result) {
   bool handled = false;
   if (HasOverrideCallback()) {
@@ -769,10 +768,10 @@ bool CommandObjectRaw::Execute(const char *args_string,
     handled = InvokeOverrideCallback(argv, result);
   }
   if (!handled) {
-    if (CheckRequirements(result))
-      handled = DoExecute(args_string, result);
+    if (CheckRequirements(result)) {
+      DoExecute(args_string, result);
+    }
 
     Cleanup();
   }
-  return handled;
 }



More information about the lldb-commits mailing list