Skip to content

Commit 4fc115d

Browse files
Handle reverse conditions in isOppositeCond function
Signed-off-by: Francois Berder <fberder@outlook.fr>
1 parent b9e6ace commit 4fc115d

2 files changed

Lines changed: 55 additions & 14 deletions

File tree

lib/astutils.cpp

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,21 +1810,32 @@ bool isSameExpression(bool macro, const Token *tok1, const Token *tok2, const Se
18101810
return commutativeEquals;
18111811
}
18121812

1813-
static bool isZeroBoundCond(const Token * const cond)
1813+
static bool isZeroBoundCond(const Token * const cond, bool reverse)
18141814
{
18151815
if (cond == nullptr)
18161816
return false;
18171817
// Assume unsigned
1818-
// TODO: Handle reverse conditions
1819-
const bool isZero = cond->astOperand2()->getValue(0);
1820-
if (cond->str() == "==" || cond->str() == ">=")
1821-
return isZero;
1822-
if (cond->str() == "<=")
1823-
return true;
1824-
if (cond->str() == "<")
1825-
return !isZero;
1826-
if (cond->str() == ">")
1827-
return false;
1818+
const bool isZero = reverse ? cond->astOperand1()->getValue(0) : cond->astOperand2()->getValue(0);
1819+
if (reverse) {
1820+
if (cond->str() == "==" || cond->str() == "<=")
1821+
return isZero;
1822+
if (cond->str() == ">=")
1823+
return true;
1824+
if (cond->str() == ">")
1825+
return !isZero;
1826+
if (cond->str() == "<")
1827+
return false;
1828+
} else {
1829+
if (cond->str() == "==" || cond->str() == ">=")
1830+
return isZero;
1831+
if (cond->str() == "<=")
1832+
return true;
1833+
if (cond->str() == "<")
1834+
return !isZero;
1835+
if (cond->str() == ">")
1836+
return false;
1837+
}
1838+
18281839
return false;
18291840
}
18301841

@@ -1888,7 +1899,7 @@ bool isOppositeCond(bool isNot, const Token * const cond1, const Token * const c
18881899
if (isSameExpression(true, cond1->astOperand2(), cond2->astOperand2(), settings, pure, followVar, errors))
18891900
return isDifferentKnownValues(cond1->astOperand1(), cond2->astOperand1());
18901901
}
1891-
// TODO: Handle reverse conditions
1902+
18921903
if (Library::isContainerYield(cond1, Library::Container::Yield::EMPTY, "empty") &&
18931904
Library::isContainerYield(cond2->astOperand1(), Library::Container::Yield::SIZE, "size") &&
18941905
isSameExpression(true,
@@ -1898,7 +1909,19 @@ bool isOppositeCond(bool isNot, const Token * const cond1, const Token * const c
18981909
pure,
18991910
followVar,
19001911
errors)) {
1901-
return !isZeroBoundCond(cond2);
1912+
return !isZeroBoundCond(cond2, false);
1913+
}
1914+
1915+
if (Library::isContainerYield(cond1, Library::Container::Yield::EMPTY, "empty") &&
1916+
Library::isContainerYield(cond2->astOperand2(), Library::Container::Yield::SIZE, "size") &&
1917+
isSameExpression(true,
1918+
cond1->astOperand1()->astOperand1(),
1919+
cond2->astOperand2()->astOperand1()->astOperand1(),
1920+
settings,
1921+
pure,
1922+
followVar,
1923+
errors)) {
1924+
return !isZeroBoundCond(cond2, true);
19021925
}
19031926

19041927
if (Library::isContainerYield(cond2, Library::Container::Yield::EMPTY, "empty") &&
@@ -1910,7 +1933,19 @@ bool isOppositeCond(bool isNot, const Token * const cond1, const Token * const c
19101933
pure,
19111934
followVar,
19121935
errors)) {
1913-
return !isZeroBoundCond(cond1);
1936+
return !isZeroBoundCond(cond1, false);
1937+
}
1938+
1939+
if (Library::isContainerYield(cond2, Library::Container::Yield::EMPTY, "empty") &&
1940+
Library::isContainerYield(cond1->astOperand2(), Library::Container::Yield::SIZE, "size") &&
1941+
isSameExpression(true,
1942+
cond2->astOperand1()->astOperand1(),
1943+
cond1->astOperand2()->astOperand1()->astOperand1(),
1944+
settings,
1945+
pure,
1946+
followVar,
1947+
errors)) {
1948+
return !isZeroBoundCond(cond1, true);
19141949
}
19151950
}
19161951

test/testcondition.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2674,6 +2674,12 @@ class TestCondition : public TestFixture {
26742674
check("void f1(const std::string &s) { if(s.size() >= 0) if(s.empty()) {}} "); // CheckOther says: Unsigned expression 's.size()' can't be negative so it is unnecessary to test it. [unsignedPositive]
26752675
ASSERT_EQUALS("", errout_str());
26762676

2677+
check("void f1(const std::string &s) { if(42 < s.size()) if(s.empty()) {}}");
2678+
ASSERT_EQUALS("[test.cpp:1:39] -> [test.cpp:1:61]: (warning) Opposite inner 'if' condition leads to a dead code block. [oppositeInnerCondition]\n", errout_str());
2679+
2680+
check("void f1(const std::string &s) { if(s.empty()) if(42 < s.size()) {}}");
2681+
ASSERT_EQUALS("[test.cpp:1:43] -> [test.cpp:1:53]: (warning) Opposite inner 'if' condition leads to a dead code block. [oppositeInnerCondition]\n", errout_str());
2682+
26772683
// TODO: These are identical condition since size cannot be negative
26782684
check("void f1(const std::string &s) { if(s.size() <= 0) if(s.empty()) {}}");
26792685
ASSERT_EQUALS("", errout_str());

0 commit comments

Comments
 (0)