llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Pete Lawrence (PortalPete) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/69989.diff 6 Files Affected: - (modified) lldb/include/lldb/Interpreter/CommandAlias.h (+1-1) - (modified) lldb/include/lldb/Interpreter/CommandObject.h (+3-3) - (modified) lldb/include/lldb/Interpreter/CommandObjectMultiword.h (+2-2) - (modified) lldb/source/Commands/CommandObjectMultiword.cpp (+10-10) - (modified) lldb/source/Interpreter/CommandAlias.cpp (+1-1) - (modified) lldb/source/Interpreter/CommandObject.cpp (+7-8) ``````````diff 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 ¤t_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; } `````````` </details> https://github.com/llvm/llvm-project/pull/69989 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits