Skip to content

Commit 3cd6afa

Browse files
committed
[lldb][NFCI] Minor refactor to CommandObjectProcessHandle::VerifyCommandOptionValue
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, it seems like the interface is better served with an optional<bool> -- Either it parses into true or false, or you get back nothing (nullopt).
1 parent 9a1ca24 commit 3cd6afa

File tree

1 file changed

+48
-55
lines changed

1 file changed

+48
-55
lines changed

lldb/source/Commands/CommandObjectProcess.cpp

Lines changed: 48 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,24 +1591,24 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
15911591

15921592
Options *GetOptions() override { return &m_options; }
15931593

1594-
bool VerifyCommandOptionValue(const std::string &option, int &real_value) {
1595-
bool okay = true;
1594+
std::optional<bool> VerifyCommandOptionValue(const std::string &option) {
1595+
if (option.empty())
1596+
return std::nullopt;
1597+
15961598
bool success = false;
15971599
bool tmp_value = OptionArgParser::ToBoolean(option, false, &success);
1600+
if (success)
1601+
return tmp_value;
1602+
1603+
int parsed_value = -1;
1604+
if (llvm::to_integer(option, parsed_value)) {
1605+
if (parsed_value != 0 && parsed_value != 1)
1606+
return std::nullopt;
15981607

1599-
if (success && tmp_value)
1600-
real_value = 1;
1601-
else if (success && !tmp_value)
1602-
real_value = 0;
1603-
else {
1604-
// If the value isn't 'true' or 'false', it had better be 0 or 1.
1605-
if (!llvm::to_integer(option, real_value))
1606-
real_value = 3;
1607-
if (real_value != 0 && real_value != 1)
1608-
okay = false;
1608+
return parsed_value == 0 ? false : true;
16091609
}
16101610

1611-
return okay;
1611+
return std::nullopt;
16121612
}
16131613

16141614
void PrintSignalHeader(Stream &str) {
@@ -1666,33 +1666,31 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
16661666
// the user's options.
16671667
ProcessSP process_sp = target.GetProcessSP();
16681668

1669-
int stop_action = -1; // -1 means leave the current setting alone
1670-
int pass_action = -1; // -1 means leave the current setting alone
1671-
int notify_action = -1; // -1 means leave the current setting alone
1669+
std::optional<bool> stop_action = VerifyCommandOptionValue(m_options.stop);
1670+
std::optional<bool> pass_action = VerifyCommandOptionValue(m_options.pass);
1671+
std::optional<bool> notify_action =
1672+
VerifyCommandOptionValue(m_options.notify);
16721673

1673-
if (!m_options.stop.empty() &&
1674-
!VerifyCommandOptionValue(m_options.stop, stop_action)) {
1674+
if (!m_options.stop.empty() && !stop_action.has_value()) {
16751675
result.AppendError("Invalid argument for command option --stop; must be "
16761676
"true or false.\n");
16771677
return;
16781678
}
16791679

1680-
if (!m_options.notify.empty() &&
1681-
!VerifyCommandOptionValue(m_options.notify, notify_action)) {
1682-
result.AppendError("Invalid argument for command option --notify; must "
1683-
"be true or false.\n");
1680+
if (!m_options.pass.empty() && !pass_action.has_value()) {
1681+
result.AppendError("Invalid argument for command option --pass; must be "
1682+
"true or false.\n");
16841683
return;
16851684
}
16861685

1687-
if (!m_options.pass.empty() &&
1688-
!VerifyCommandOptionValue(m_options.pass, pass_action)) {
1689-
result.AppendError("Invalid argument for command option --pass; must be "
1690-
"true or false.\n");
1686+
if (!m_options.notify.empty() && !notify_action.has_value()) {
1687+
result.AppendError("Invalid argument for command option --notify; must "
1688+
"be true or false.\n");
16911689
return;
16921690
}
16931691

1694-
bool no_actions = (stop_action == -1 && pass_action == -1
1695-
&& notify_action == -1);
1692+
bool no_actions = (!stop_action.has_value() && !pass_action.has_value() &&
1693+
!notify_action.has_value());
16961694
if (m_options.only_target_values && !no_actions) {
16971695
result.AppendError("-t is for reporting, not setting, target values.");
16981696
return;
@@ -1732,14 +1730,14 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
17321730
if (signo != LLDB_INVALID_SIGNAL_NUMBER) {
17331731
// Casting the actions as bools here should be okay, because
17341732
// VerifyCommandOptionValue guarantees the value is either 0 or 1.
1735-
if (stop_action != -1)
1736-
signals_sp->SetShouldStop(signo, stop_action);
1737-
if (pass_action != -1) {
1738-
bool suppress = !pass_action;
1733+
if (stop_action.has_value())
1734+
signals_sp->SetShouldStop(signo, *stop_action);
1735+
if (pass_action.has_value()) {
1736+
bool suppress = !*pass_action;
17391737
signals_sp->SetShouldSuppress(signo, suppress);
17401738
}
1741-
if (notify_action != -1)
1742-
signals_sp->SetShouldNotify(signo, notify_action);
1739+
if (notify_action.has_value())
1740+
signals_sp->SetShouldNotify(signo, *notify_action);
17431741
++num_signals_set;
17441742
} else {
17451743
result.AppendErrorWithFormat("Invalid signal name '%s'\n",
@@ -1759,40 +1757,35 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
17591757
}
17601758
num_signals_set = num_args;
17611759
}
1762-
auto set_lazy_bool = [] (int action) -> LazyBool {
1763-
LazyBool lazy;
1764-
if (action == -1)
1765-
lazy = eLazyBoolCalculate;
1766-
else if (action)
1767-
lazy = eLazyBoolYes;
1768-
else
1769-
lazy = eLazyBoolNo;
1770-
return lazy;
1760+
auto set_lazy_bool = [](std::optional<bool> action) -> LazyBool {
1761+
if (!action.has_value())
1762+
return eLazyBoolCalculate;
1763+
return (*action) ? eLazyBoolYes : eLazyBoolNo;
17711764
};
17721765

17731766
// If there were no actions, we're just listing, don't add the dummy:
17741767
if (!no_actions)
1775-
target.AddDummySignal(arg.ref(),
1776-
set_lazy_bool(pass_action),
1777-
set_lazy_bool(notify_action),
1778-
set_lazy_bool(stop_action));
1768+
target.AddDummySignal(arg.ref(), set_lazy_bool(pass_action),
1769+
set_lazy_bool(notify_action),
1770+
set_lazy_bool(stop_action));
17791771
}
17801772
} else {
17811773
// No signal specified, if any command options were specified, update ALL
17821774
// signals. But we can't do this without a process since we don't know
17831775
// all the possible signals that might be valid for this target.
1784-
if (((notify_action != -1) || (stop_action != -1) || (pass_action != -1))
1785-
&& process_sp) {
1776+
if ((notify_action.has_value() || stop_action.has_value() ||
1777+
pass_action.has_value()) &&
1778+
process_sp) {
17861779
if (m_interpreter.Confirm(
17871780
"Do you really want to update all the signals?", false)) {
17881781
int32_t signo = signals_sp->GetFirstSignalNumber();
17891782
while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
1790-
if (notify_action != -1)
1791-
signals_sp->SetShouldNotify(signo, notify_action);
1792-
if (stop_action != -1)
1793-
signals_sp->SetShouldStop(signo, stop_action);
1794-
if (pass_action != -1) {
1795-
bool suppress = !pass_action;
1783+
if (notify_action.has_value())
1784+
signals_sp->SetShouldNotify(signo, *notify_action);
1785+
if (stop_action.has_value())
1786+
signals_sp->SetShouldStop(signo, *stop_action);
1787+
if (pass_action.has_value()) {
1788+
bool suppress = !*pass_action;
17961789
signals_sp->SetShouldSuppress(signo, suppress);
17971790
}
17981791
signo = signals_sp->GetNextSignalNumber(signo);

0 commit comments

Comments
 (0)