[Lldb-commits] [lldb] [lldb][NFCI] Minor refactor to CommandObjectProcessHandle::VerifyCommandOptionValue (PR #79901)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Jan 29 13:24:33 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Alex Langford (bulbazord)
<details>
<summary>Changes</summary>
I was refactoring something else but ran into this function. It was somewhat confusing to read through and understand, but it boils down to two steps:
- First we try `OptionArgParser::ToBoolean`. If that works, then we're good to go.
- Second, we try `llvm::to_integer` to see if it's an integer. If it parses to 0 or 1, we're good.
- Failing either of the steps above means we cannot parse it into a bool.
Instead of having an integer out param and a bool return value, the interface is better served with an optional<bool> -- Either it parses into true or false, or you get back nothing (nullopt).
---
Full diff: https://github.com/llvm/llvm-project/pull/79901.diff
1 Files Affected:
- (modified) lldb/source/Commands/CommandObjectProcess.cpp (+48-55)
``````````diff
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index c7b874d1979377..f089a86275dc69 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1591,24 +1591,24 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
Options *GetOptions() override { return &m_options; }
- bool VerifyCommandOptionValue(const std::string &option, int &real_value) {
- bool okay = true;
+ std::optional<bool> VerifyCommandOptionValue(const std::string &option) {
+ if (option.empty())
+ return std::nullopt;
+
bool success = false;
bool tmp_value = OptionArgParser::ToBoolean(option, false, &success);
+ if (success)
+ return tmp_value;
+
+ int parsed_value = -1;
+ if (llvm::to_integer(option, parsed_value)) {
+ if (parsed_value != 0 && parsed_value != 1)
+ return std::nullopt;
- if (success && tmp_value)
- real_value = 1;
- else if (success && !tmp_value)
- real_value = 0;
- else {
- // If the value isn't 'true' or 'false', it had better be 0 or 1.
- if (!llvm::to_integer(option, real_value))
- real_value = 3;
- if (real_value != 0 && real_value != 1)
- okay = false;
+ return parsed_value == 0 ? false : true;
}
- return okay;
+ return std::nullopt;
}
void PrintSignalHeader(Stream &str) {
@@ -1666,33 +1666,31 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
// the user's options.
ProcessSP process_sp = target.GetProcessSP();
- int stop_action = -1; // -1 means leave the current setting alone
- int pass_action = -1; // -1 means leave the current setting alone
- int notify_action = -1; // -1 means leave the current setting alone
+ std::optional<bool> stop_action = VerifyCommandOptionValue(m_options.stop);
+ std::optional<bool> pass_action = VerifyCommandOptionValue(m_options.pass);
+ std::optional<bool> notify_action =
+ VerifyCommandOptionValue(m_options.notify);
- if (!m_options.stop.empty() &&
- !VerifyCommandOptionValue(m_options.stop, stop_action)) {
+ if (!m_options.stop.empty() && !stop_action.has_value()) {
result.AppendError("Invalid argument for command option --stop; must be "
"true or false.\n");
return;
}
- if (!m_options.notify.empty() &&
- !VerifyCommandOptionValue(m_options.notify, notify_action)) {
- result.AppendError("Invalid argument for command option --notify; must "
- "be true or false.\n");
+ if (!m_options.pass.empty() && !pass_action.has_value()) {
+ result.AppendError("Invalid argument for command option --pass; must be "
+ "true or false.\n");
return;
}
- if (!m_options.pass.empty() &&
- !VerifyCommandOptionValue(m_options.pass, pass_action)) {
- result.AppendError("Invalid argument for command option --pass; must be "
- "true or false.\n");
+ if (!m_options.notify.empty() && !notify_action.has_value()) {
+ result.AppendError("Invalid argument for command option --notify; must "
+ "be true or false.\n");
return;
}
- bool no_actions = (stop_action == -1 && pass_action == -1
- && notify_action == -1);
+ bool no_actions = (!stop_action.has_value() && !pass_action.has_value() &&
+ !notify_action.has_value());
if (m_options.only_target_values && !no_actions) {
result.AppendError("-t is for reporting, not setting, target values.");
return;
@@ -1732,14 +1730,14 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
if (signo != LLDB_INVALID_SIGNAL_NUMBER) {
// Casting the actions as bools here should be okay, because
// VerifyCommandOptionValue guarantees the value is either 0 or 1.
- if (stop_action != -1)
- signals_sp->SetShouldStop(signo, stop_action);
- if (pass_action != -1) {
- bool suppress = !pass_action;
+ if (stop_action.has_value())
+ signals_sp->SetShouldStop(signo, *stop_action);
+ if (pass_action.has_value()) {
+ bool suppress = !*pass_action;
signals_sp->SetShouldSuppress(signo, suppress);
}
- if (notify_action != -1)
- signals_sp->SetShouldNotify(signo, notify_action);
+ if (notify_action.has_value())
+ signals_sp->SetShouldNotify(signo, *notify_action);
++num_signals_set;
} else {
result.AppendErrorWithFormat("Invalid signal name '%s'\n",
@@ -1759,40 +1757,35 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
}
num_signals_set = num_args;
}
- auto set_lazy_bool = [] (int action) -> LazyBool {
- LazyBool lazy;
- if (action == -1)
- lazy = eLazyBoolCalculate;
- else if (action)
- lazy = eLazyBoolYes;
- else
- lazy = eLazyBoolNo;
- return lazy;
+ auto set_lazy_bool = [](std::optional<bool> action) -> LazyBool {
+ if (!action.has_value())
+ return eLazyBoolCalculate;
+ return (*action) ? eLazyBoolYes : eLazyBoolNo;
};
// If there were no actions, we're just listing, don't add the dummy:
if (!no_actions)
- target.AddDummySignal(arg.ref(),
- set_lazy_bool(pass_action),
- set_lazy_bool(notify_action),
- set_lazy_bool(stop_action));
+ target.AddDummySignal(arg.ref(), set_lazy_bool(pass_action),
+ set_lazy_bool(notify_action),
+ set_lazy_bool(stop_action));
}
} else {
// No signal specified, if any command options were specified, update ALL
// signals. But we can't do this without a process since we don't know
// all the possible signals that might be valid for this target.
- if (((notify_action != -1) || (stop_action != -1) || (pass_action != -1))
- && process_sp) {
+ if ((notify_action.has_value() || stop_action.has_value() ||
+ pass_action.has_value()) &&
+ process_sp) {
if (m_interpreter.Confirm(
"Do you really want to update all the signals?", false)) {
int32_t signo = signals_sp->GetFirstSignalNumber();
while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
- if (notify_action != -1)
- signals_sp->SetShouldNotify(signo, notify_action);
- if (stop_action != -1)
- signals_sp->SetShouldStop(signo, stop_action);
- if (pass_action != -1) {
- bool suppress = !pass_action;
+ if (notify_action.has_value())
+ signals_sp->SetShouldNotify(signo, *notify_action);
+ if (stop_action.has_value())
+ signals_sp->SetShouldStop(signo, *stop_action);
+ if (pass_action.has_value()) {
+ bool suppress = !*pass_action;
signals_sp->SetShouldSuppress(signo, suppress);
}
signo = signals_sp->GetNextSignalNumber(signo);
``````````
</details>
https://github.com/llvm/llvm-project/pull/79901
More information about the lldb-commits
mailing list