[lldb] Part 1 of 2 - Refactor CommandObject::Execute(...) return void (not bool)
#69989
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PortalPete
changed the title
CommandObject::Execute(...) to return void instead of ~~bool~~CommandObject::Execute(...) return void (not bool)
|
@llvm/pr-subscribers-lldb Author: Pete Lawrence (PortalPete) ChangesJustifications:
Part 2 refactors the rdar://117378957 Full diff: https://github.com/llvm/llvm-project/pull/69989.diff 6 Files Affected:
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;
}
|
kastiglione
reviewed
Oct 24, 2023
kastiglione
reviewed
Oct 24, 2023
PortalPete
force-pushed
the
refactor/CommandObject-Execute
branch
from
b1b96c7
to
fe1bc41
Compare
JDevlieghere
approved these changes
Oct 24, 2023
bulbazord
approved these changes
Oct 24, 2023
… `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
PortalPete
force-pushed
the
refactor/CommandObject-Execute
branch
from
fe1bc41
to
d18c6e2
Compare
|
LGTM! |
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Oct 26, 2023
…oid` (not `bool`) (llvm#69989) [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. - See [llvm#69991 rdar://117378957
adrian-prantl
pushed a commit
that referenced
this pull request
Oct 30, 2023
…`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 [#69989 rdar://117378957
PortalPete
added a commit
to PortalPete/llvm-project
that referenced
this pull request
Feb 7, 2024
…oid` (not `bool`) (llvm#69989) [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. - See [llvm#69991 rdar://117378957
PortalPete
added a commit
to PortalPete/llvm-project
that referenced
this pull request
Feb 7, 2024
…`void` (not `bool`) (llvm#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 [llvm#69989 rdar://117378957
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[lldb] Part 1 of 2 - Refactor
CommandObject::Execute(...)to returnvoidinstead ofboolJustifications:
true/falsereturn values.CommandReturnObject, typically with aresultparameter.Part 2 refactors the
CommandObject::DoExecute(...)method.CommandObject::DoExecute(...)returnvoid(notbool) #69991rdar://117378957