Skip to content

Commit 307cd88

Browse files
authored
[lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (#79901)
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).
1 parent 275eeda commit 307cd88

File tree

1 file changed

+59
-67
lines changed

1 file changed

+59
-67
lines changed

lldb/source/Commands/CommandObjectProcess.cpp

Lines changed: 59 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,26 +1591,6 @@ 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;
1596-
bool success = false;
1597-
bool tmp_value = OptionArgParser::ToBoolean(option, false, &success);
1598-
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;
1609-
}
1610-
1611-
return okay;
1612-
}
1613-
16141594
void PrintSignalHeader(Stream &str) {
16151595
str.Printf("NAME PASS STOP NOTIFY\n");
16161596
str.Printf("=========== ===== ===== ======\n");
@@ -1666,33 +1646,52 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
16661646
// the user's options.
16671647
ProcessSP process_sp = target.GetProcessSP();
16681648

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
1649+
std::optional<bool> stop_action = {};
1650+
std::optional<bool> pass_action = {};
1651+
std::optional<bool> notify_action = {};
16721652

1673-
if (!m_options.stop.empty() &&
1674-
!VerifyCommandOptionValue(m_options.stop, stop_action)) {
1675-
result.AppendError("Invalid argument for command option --stop; must be "
1676-
"true or false.\n");
1677-
return;
1653+
if (!m_options.stop.empty()) {
1654+
bool success = false;
1655+
bool value = OptionArgParser::ToBoolean(m_options.stop, false, &success);
1656+
if (!success) {
1657+
result.AppendError(
1658+
"Invalid argument for command option --stop; must be "
1659+
"true or false.\n");
1660+
return;
1661+
}
1662+
1663+
stop_action = value;
16781664
}
16791665

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");
1684-
return;
1666+
if (!m_options.pass.empty()) {
1667+
bool success = false;
1668+
bool value = OptionArgParser::ToBoolean(m_options.pass, false, &success);
1669+
if (!success) {
1670+
result.AppendError(
1671+
"Invalid argument for command option --pass; must be "
1672+
"true or false.\n");
1673+
return;
1674+
}
1675+
pass_action = value;
16851676
}
16861677

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");
1691-
return;
1678+
if (!m_options.notify.empty()) {
1679+
bool success = false;
1680+
bool value =
1681+
OptionArgParser::ToBoolean(m_options.notify, false, &success);
1682+
if (!success) {
1683+
result.AppendError("Invalid argument for command option --notify; must "
1684+
"be true or false.\n");
1685+
return;
1686+
}
1687+
notify_action = value;
1688+
}
1689+
1690+
if (!m_options.notify.empty() && !notify_action.has_value()) {
16921691
}
16931692

1694-
bool no_actions = (stop_action == -1 && pass_action == -1
1695-
&& notify_action == -1);
1693+
bool no_actions = (!stop_action.has_value() && !pass_action.has_value() &&
1694+
!notify_action.has_value());
16961695
if (m_options.only_target_values && !no_actions) {
16971696
result.AppendError("-t is for reporting, not setting, target values.");
16981697
return;
@@ -1730,16 +1729,14 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
17301729
if (signals_sp) {
17311730
int32_t signo = signals_sp->GetSignalNumberFromName(arg.c_str());
17321731
if (signo != LLDB_INVALID_SIGNAL_NUMBER) {
1733-
// Casting the actions as bools here should be okay, because
1734-
// 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;
1732+
if (stop_action.has_value())
1733+
signals_sp->SetShouldStop(signo, *stop_action);
1734+
if (pass_action.has_value()) {
1735+
bool suppress = !*pass_action;
17391736
signals_sp->SetShouldSuppress(signo, suppress);
17401737
}
1741-
if (notify_action != -1)
1742-
signals_sp->SetShouldNotify(signo, notify_action);
1738+
if (notify_action.has_value())
1739+
signals_sp->SetShouldNotify(signo, *notify_action);
17431740
++num_signals_set;
17441741
} else {
17451742
result.AppendErrorWithFormat("Invalid signal name '%s'\n",
@@ -1759,40 +1756,35 @@ class CommandObjectProcessHandle : public CommandObjectParsed {
17591756
}
17601757
num_signals_set = num_args;
17611758
}
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;
1759+
auto set_lazy_bool = [](std::optional<bool> action) -> LazyBool {
1760+
if (!action.has_value())
1761+
return eLazyBoolCalculate;
1762+
return (*action) ? eLazyBoolYes : eLazyBoolNo;
17711763
};
17721764

17731765
// If there were no actions, we're just listing, don't add the dummy:
17741766
if (!no_actions)
1775-
target.AddDummySignal(arg.ref(),
1776-
set_lazy_bool(pass_action),
1767+
target.AddDummySignal(arg.ref(), set_lazy_bool(pass_action),
17771768
set_lazy_bool(notify_action),
17781769
set_lazy_bool(stop_action));
17791770
}
17801771
} else {
17811772
// No signal specified, if any command options were specified, update ALL
17821773
// signals. But we can't do this without a process since we don't know
17831774
// 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) {
1775+
if ((notify_action.has_value() || stop_action.has_value() ||
1776+
pass_action.has_value()) &&
1777+
process_sp) {
17861778
if (m_interpreter.Confirm(
17871779
"Do you really want to update all the signals?", false)) {
17881780
int32_t signo = signals_sp->GetFirstSignalNumber();
17891781
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;
1782+
if (notify_action.has_value())
1783+
signals_sp->SetShouldNotify(signo, *notify_action);
1784+
if (stop_action.has_value())
1785+
signals_sp->SetShouldStop(signo, *stop_action);
1786+
if (pass_action.has_value()) {
1787+
bool suppress = !*pass_action;
17961788
signals_sp->SetShouldSuppress(signo, suppress);
17971789
}
17981790
signo = signals_sp->GetNextSignalNumber(signo);

0 commit comments

Comments
 (0)