Skip to content

Commit f6ed35e

Browse files
firewavefirewave
committed
ProgramMemory: avoid duplicated lookups / removed at() [skip ci]
1 parent 3529cd6 commit f6ed35e

File tree

3 files changed

+59
-67
lines changed

3 files changed

+59
-67
lines changed

‎lib/programmemory.cpp‎

Lines changed: 58 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,15 @@ const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossib
8585
return nullptr;
8686
}
8787

88+
ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid)
89+
{
90+
const auto it = find(exprid);
91+
const bool found = it != mValues->cend();
92+
if (found)
93+
return &it->second;
94+
return nullptr;
95+
}
96+
8897
// cppcheck-suppress unusedFunction
8998
bool ProgramMemory::getIntValue(nonneg int exprid, MathLib::bigint& result) const
9099
{
@@ -161,24 +170,6 @@ bool ProgramMemory::hasValue(nonneg int exprid) const
161170
return it != mValues->cend();
162171
}
163172

164-
const ValueFlow::Value& ProgramMemory::at(nonneg int exprid) const {
165-
const auto it = find(exprid);
166-
if (it == mValues->cend()) {
167-
throw std::out_of_range("ProgramMemory::at");
168-
}
169-
return it->second;
170-
}
171-
172-
ValueFlow::Value& ProgramMemory::at(nonneg int exprid) {
173-
copyOnWrite();
174-
175-
const auto it = find(exprid);
176-
if (it == mValues->end()) {
177-
throw std::out_of_range("ProgramMemory::at");
178-
}
179-
return it->second;
180-
}
181-
182173
void ProgramMemory::erase_if(const std::function<bool(const ExprIdToken&)>& pred)
183174
{
184175
if (mValues->empty())
@@ -1343,10 +1334,9 @@ namespace {
13431334

13441335
ValueFlow::Value executeMultiCondition(bool b, const Token* expr)
13451336
{
1346-
if (pm->hasValue(expr->exprId())) {
1347-
const ValueFlow::Value& v = utils::as_const(*pm).at(expr->exprId());
1348-
if (v.isIntValue())
1349-
return v;
1337+
if (const ValueFlow::Value* v = utils::as_const(*pm).getValue(expr->exprId())) {
1338+
if (v->isIntValue())
1339+
return *v;
13501340
}
13511341

13521342
// Evaluate recursively if there are no exprids
@@ -1475,18 +1465,18 @@ namespace {
14751465
if (rhs.isUninitValue())
14761466
return unknown();
14771467
if (expr->str() != "=") {
1478-
if (!pm->hasValue(expr->astOperand1()->exprId()))
1468+
ValueFlow::Value* lhs = pm->getValue(expr->astOperand1()->exprId());
1469+
if (!lhs)
14791470
return unknown();
1480-
ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId());
1481-
rhs = evaluate(removeAssign(expr->str()), lhs, rhs);
1482-
if (lhs.isIntValue())
1483-
ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1));
1484-
else if (lhs.isFloatValue())
1471+
rhs = evaluate(removeAssign(expr->str()), *lhs, rhs);
1472+
if (lhs->isIntValue())
1473+
ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs->intvalue), std::placeholders::_1));
1474+
else if (lhs->isFloatValue())
14851475
ValueFlow::Value::visitValue(rhs,
1486-
std::bind(assign{}, std::ref(lhs.floatValue), std::placeholders::_1));
1476+
std::bind(assign{}, std::ref(lhs->floatValue), std::placeholders::_1));
14871477
else
14881478
return unknown();
1489-
return lhs;
1479+
return *lhs;
14901480
}
14911481
pm->setValue(expr->astOperand1(), rhs);
14921482
return rhs;
@@ -1498,20 +1488,20 @@ namespace {
14981488
execute(expr->astOperand1());
14991489
return execute(expr->astOperand2());
15001490
} else if (expr->tokType() == Token::eIncDecOp && expr->astOperand1() && expr->astOperand1()->exprId() != 0) {
1501-
if (!pm->hasValue(expr->astOperand1()->exprId()))
1491+
ValueFlow::Value* lhs = pm->getValue(expr->astOperand1()->exprId());
1492+
if (!lhs)
15021493
return ValueFlow::Value::unknown();
1503-
ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId());
1504-
if (!lhs.isIntValue())
1494+
if (!lhs->isIntValue())
15051495
return unknown();
15061496
// overflow
1507-
if (!lhs.isImpossible() && lhs.intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1()))
1497+
if (!lhs->isImpossible() && lhs->intvalue == 0 && expr->str() == "--" && astIsUnsigned(expr->astOperand1()))
15081498
return unknown();
15091499

15101500
if (expr->str() == "++")
1511-
lhs.intvalue++;
1501+
lhs->intvalue++;
15121502
else
1513-
lhs.intvalue--;
1514-
return lhs;
1503+
lhs->intvalue--;
1504+
return *lhs;
15151505
} else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) {
15161506
const Token* tokvalue = nullptr;
15171507
if (!pm->getTokValue(expr->astOperand1()->exprId(), tokvalue)) {
@@ -1602,13 +1592,16 @@ namespace {
16021592
}
16031593
return execute(expr->astOperand1());
16041594
}
1605-
if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) {
1606-
ValueFlow::Value result = utils::as_const(*pm).at(expr->exprId());
1607-
if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) {
1608-
result.intvalue = !result.intvalue;
1609-
result.setKnown();
1595+
if (expr->exprId() > 0) {
1596+
if (const ValueFlow::Value* v = utils::as_const(*pm).getValue(expr->exprId()))
1597+
{
1598+
ValueFlow::Value result = *v;
1599+
if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) {
1600+
result.intvalue = !result.intvalue;
1601+
result.setKnown();
1602+
}
1603+
return result;
16101604
}
1611-
return result;
16121605
}
16131606

16141607
if (Token::Match(expr->previous(), ">|%name% {|(")) {
@@ -1658,14 +1651,16 @@ namespace {
16581651
}
16591652
// Check if function modifies argument
16601653
visitAstNodes(expr->astOperand2(), [&](const Token* child) {
1661-
if (child->exprId() > 0 && pm->hasValue(child->exprId())) {
1662-
ValueFlow::Value& v = pm->at(child->exprId());
1663-
if (v.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) {
1664-
if (ValueFlow::isContainerSizeChanged(child, v.indirect, settings))
1665-
v = unknown();
1666-
} else if (v.valueType != ValueFlow::Value::ValueType::UNINIT) {
1667-
if (isVariableChanged(child, v.indirect, settings))
1668-
v = unknown();
1654+
if (child->exprId() > 0) {
1655+
if (ValueFlow::Value* v = pm->getValue(child->exprId()))
1656+
{
1657+
if (v->valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE) {
1658+
if (ValueFlow::isContainerSizeChanged(child, v->indirect, settings))
1659+
*v = unknown();
1660+
} else if (v->valueType != ValueFlow::Value::ValueType::UNINIT) {
1661+
if (isVariableChanged(child, v->indirect, settings))
1662+
*v = unknown();
1663+
}
16691664
}
16701665
}
16711666
return ChildrenToVisit::op1_and_op2;
@@ -1717,22 +1712,27 @@ namespace {
17171712
return v;
17181713
if (!expr)
17191714
return v;
1720-
if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) {
1721-
if (updateValue(v, utils::as_const(*pm).at(expr->exprId())))
1722-
return v;
1715+
if (expr->exprId() > 0) {
1716+
if (const ValueFlow::Value* val = utils::as_const(*pm).getValue(expr->exprId()))
1717+
{
1718+
if (updateValue(v, *val))
1719+
return v;
1720+
}
17231721
}
17241722
// Find symbolic values
17251723
for (const ValueFlow::Value& value : expr->values()) {
17261724
if (!value.isSymbolicValue())
17271725
continue;
17281726
if (!value.isKnown())
17291727
continue;
1730-
if (value.tokvalue->exprId() > 0 && !pm->hasValue(value.tokvalue->exprId()))
1728+
if (value.tokvalue->exprId() == 0)
1729+
continue;
1730+
const ValueFlow::Value* v_p = utils::as_const(*pm).getValue(value.tokvalue->exprId());
1731+
if (!v_p)
17311732
continue;
1732-
const ValueFlow::Value& v_ref = utils::as_const(*pm).at(value.tokvalue->exprId());
1733-
if (!v_ref.isIntValue() && value.intvalue != 0)
1733+
if (!v_p->isIntValue() && value.intvalue != 0)
17341734
continue;
1735-
ValueFlow::Value v2 = v_ref;
1735+
ValueFlow::Value v2 = *v_p;
17361736
v2.intvalue += value.intvalue;
17371737
return v2;
17381738
}

‎lib/programmemory.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ struct CPPCHECKLIB ProgramMemory {
101101

102102
void setValue(const Token* expr, const ValueFlow::Value& value);
103103
const ValueFlow::Value* getValue(nonneg int exprid, bool impossible = false) const;
104+
ValueFlow::Value* getValue(nonneg int exprid);
104105

105106
bool getIntValue(nonneg int exprid, MathLib::bigint& result) const;
106107
void setIntValue(const Token* expr, MathLib::bigint value, bool impossible = false);
@@ -114,9 +115,6 @@ struct CPPCHECKLIB ProgramMemory {
114115
bool getTokValue(nonneg int exprid, const Token*& result) const;
115116
bool hasValue(nonneg int exprid) const;
116117

117-
const ValueFlow::Value& at(nonneg int exprid) const;
118-
ValueFlow::Value& at(nonneg int exprid);
119-
120118
void erase_if(const std::function<bool(const ExprIdToken&)>& pred);
121119

122120
void swap(ProgramMemory &pm) NOEXCEPT;

‎test/testprogrammemory.cpp‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,6 @@ class TestProgramMemory : public TestFixture {
9292
ProgramMemory pm;
9393
ASSERT(!pm.getValue(123));
9494
}
95-
96-
void at() const {
97-
ProgramMemory pm;
98-
ASSERT_THROW_EQUALS_2(pm.at(123), std::out_of_range, "ProgramMemory::at");
99-
ASSERT_THROW_EQUALS_2(utils::as_const(pm).at(123), std::out_of_range, "ProgramMemory::at");
100-
}
10195
};
10296

10397
REGISTER_TEST(TestProgramMemory)

0 commit comments

Comments
 (0)