Skip to content

Commit

Permalink
[lldb] Part 1 of 2 - Refactor CommandObject::Execute(...) return `v…
Browse files Browse the repository at this point in the history
…oid` (not `bool`) (#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
[#69991

rdar://117378957
  • Loading branch information
PortalPete committed Oct 25, 2023
1 parent c9ca2fe commit 463a02b
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 26 deletions.
2 changes: 1 addition & 1 deletion lldb/include/lldb/Interpreter/CommandAlias.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions lldb/include/lldb/Interpreter/CommandObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions lldb/include/lldb/Interpreter/CommandObjectMultiword.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down Expand Up @@ -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.
Expand Down
22 changes: 10 additions & 12 deletions lldb/source/Commands/CommandObjectMultiword.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -429,11 +428,10 @@ 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);
result.AppendError(GetUnsupportedError());
return false;
if (CommandObject *proxy_command = GetProxyCommandObject())
proxy_command->Execute(args_string, result);
else
result.AppendError(GetUnsupportedError());
}
2 changes: 1 addition & 1 deletion lldb/source/Interpreter/CommandAlias.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
12 changes: 5 additions & 7 deletions lldb/source/Interpreter/CommandObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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()) {
Expand All @@ -770,9 +769,8 @@ bool CommandObjectRaw::Execute(const char *args_string,
}
if (!handled) {
if (CheckRequirements(result))
handled = DoExecute(args_string, result);
DoExecute(args_string, result);

Cleanup();
}
return handled;
}

0 comments on commit 463a02b

Please sign in to comment.